tberghammer updated this revision to Diff 48908. tberghammer added a comment.
I added the memory optimization to represent the case when no DW_AT_start_scope is specified as an empty range map. This way the memory impact for C/C++ will be only an empty std::vector per variable. Regarding the location of the code I don't want to add it to DWARFExpression because dwarf expressions are used for much more thing then variable locations on their own. If an other DIE needs this information in the future then I would suggest to introduce a new class (don't have any good name for it) containing a DWARFExpression representing the location and a RangeMap representing the scope information. In long term (don't have time from for it) I would also want to split DWARFExpression to 2 class where one of them know how to evaluate a dwarf expression and can represent only a single dwarf expression. Next to it I want to introduce a new DWARFLocationList class what knows about the location list part with no knowledge about the DWARF opcodes. The 2 class can be closely integrated so we don't need to parse anything upfront while we are keeping the 2 concept separated. http://reviews.llvm.org/D17449 Files: include/lldb/Core/RangeMap.h include/lldb/Symbol/Block.h include/lldb/Symbol/Variable.h source/API/SBBlock.cpp source/API/SBFrame.cpp source/Core/Address.cpp source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp source/Symbol/Block.cpp source/Symbol/Variable.cpp source/Target/StackFrame.cpp
Index: source/Target/StackFrame.cpp =================================================================== --- source/Target/StackFrame.cpp +++ source/Target/StackFrame.cpp @@ -540,7 +540,11 @@ const bool can_create = true; const bool stop_if_child_block_is_inlined_function = true; m_variable_list_sp.reset(new VariableList()); - frame_block->AppendBlockVariables(can_create, get_child_variables, stop_if_child_block_is_inlined_function, m_variable_list_sp.get()); + frame_block->AppendBlockVariables(can_create, + get_child_variables, + stop_if_child_block_is_inlined_function, + [this](Variable* v) { return v->IsInScope(this); }, + m_variable_list_sp.get()); } } @@ -584,6 +588,7 @@ m_sc.block->AppendVariables (can_create, get_parent_variables, stop_if_block_is_inlined_function, + [this](Variable* v) { return v->IsInScope(this); }, var_list_sp.get()); } Index: source/Symbol/Variable.cpp =================================================================== --- source/Symbol/Variable.cpp +++ source/Symbol/Variable.cpp @@ -43,6 +43,7 @@ const lldb::SymbolFileTypeSP &symfile_type_sp, ValueType scope, SymbolContextScope *context, + const RangeList& scope_range, Declaration* decl_ptr, const DWARFExpression& location, bool external, @@ -54,6 +55,7 @@ m_symfile_type_sp(symfile_type_sp), m_scope(scope), m_owner_scope(context), + m_scope_range(scope_range), m_declaration(decl_ptr), m_location(location), m_external(external), @@ -356,14 +358,24 @@ { SymbolContext variable_sc; CalculateSymbolContext (&variable_sc); + // Check for static or global variable defined at the compile unit // level that wasn't defined in a block if (variable_sc.block == nullptr) - return true; + return true; - if (variable_sc.block == deepest_frame_block) + // Check if the variable is valid in the current block + if (variable_sc.block != deepest_frame_block && + !variable_sc.block->Contains(deepest_frame_block)) + return false; + + // If no scope range is specified then it means that the scope is the same as the + // scope of the enclosing lexical block. + if (m_scope_range.IsEmpty()) return true; - return variable_sc.block->Contains (deepest_frame_block); + + addr_t file_address = frame->GetFrameCodeAddress().GetFileAddress(); + return m_scope_range.FindEntryThatContains(file_address) != nullptr; } } break; Index: source/Symbol/Block.cpp =================================================================== --- source/Symbol/Block.cpp +++ source/Symbol/Block.cpp @@ -486,16 +486,24 @@ Block::AppendBlockVariables (bool can_create, bool get_child_block_variables, bool stop_if_child_block_is_inlined_function, + const std::function<bool(Variable*)>& filter, VariableList *variable_list) { uint32_t num_variables_added = 0; VariableList *block_var_list = GetBlockVariableList (can_create).get(); if (block_var_list) { - num_variables_added += block_var_list->GetSize(); - variable_list->AddVariables (block_var_list); + for (size_t i = 0; i < block_var_list->GetSize(); ++i) + { + VariableSP variable = block_var_list->GetVariableAtIndex(i); + if (filter(variable.get())) + { + num_variables_added++; + variable_list->AddVariable(variable); + } + } } - + if (get_child_block_variables) { collection::const_iterator pos, end = m_children.end(); @@ -508,6 +516,7 @@ num_variables_added += child_block->AppendBlockVariables (can_create, get_child_block_variables, stop_if_child_block_is_inlined_function, + filter, variable_list); } } @@ -521,27 +530,39 @@ bool can_create, bool get_parent_variables, bool stop_if_block_is_inlined_function, + const std::function<bool(Variable*)>& filter, VariableList *variable_list ) { uint32_t num_variables_added = 0; VariableListSP variable_list_sp(GetBlockVariableList(can_create)); bool is_inlined_function = GetInlinedFunctionInfo() != nullptr; - if (variable_list_sp.get()) + if (variable_list_sp) { - num_variables_added = variable_list_sp->GetSize(); - variable_list->AddVariables(variable_list_sp.get()); + for (size_t i = 0; i < variable_list_sp->GetSize(); ++i) + { + VariableSP variable = variable_list_sp->GetVariableAtIndex(i); + if (filter(variable.get())) + { + num_variables_added++; + variable_list->AddVariable(variable); + } + } } - + if (get_parent_variables) { if (stop_if_block_is_inlined_function && is_inlined_function) return num_variables_added; Block* parent_block = GetParent(); if (parent_block) - num_variables_added += parent_block->AppendVariables (can_create, get_parent_variables, stop_if_block_is_inlined_function, variable_list); + num_variables_added += parent_block->AppendVariables(can_create, + get_parent_variables, + stop_if_block_is_inlined_function, + filter, + variable_list); } return num_variables_added; } Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp =================================================================== --- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -4091,6 +4091,7 @@ bool location_is_const_value_data = false; bool has_explicit_location = false; DWARFFormValue const_value; + Variable::RangeList scope_ranges; //AccessType accessibility = eAccessNone; for (i=0; i<num_attributes; ++i) @@ -4206,13 +4207,44 @@ spec_die = debug_info->GetDIE(DIERef(form_value)); break; } + case DW_AT_start_scope: + { + if (form_value.Form() == DW_FORM_sec_offset) + { + DWARFRangeList dwarf_scope_ranges; + const DWARFDebugRanges* debug_ranges = DebugRanges(); + debug_ranges->FindRanges(form_value.Unsigned(), dwarf_scope_ranges); + + // All DW_AT_start_scope are relative to the base address of the + // compile unit. We add the compile unit base address to make + // sure all the addresses are properly fixed up. + for (size_t i = 0, count = dwarf_scope_ranges.GetSize(); i < count; ++i) + { + const DWARFRangeList::Entry& range = dwarf_scope_ranges.GetEntryRef(i); + scope_ranges.Append(range.GetRangeBase() + die.GetCU()->GetBaseAddress(), + range.GetByteSize()); + } + } + else + { + // TODO: Handle the case when DW_AT_start_scope have form constant. The + // dwarf spec is a bit ambiguous about what is the expected behavior in + // case the enclosing block have a non coninious address range and the + // DW_AT_start_scope entry have a form constant. + GetObjectFile()->GetModule()->ReportWarning ("0x%8.8" PRIx64 ": DW_AT_start_scope has unsupported form type (0x%x)\n", + die.GetID(), + form_value.Form()); + } + + scope_ranges.Sort(); + scope_ranges.CombineConsecutiveRanges(); + } case DW_AT_artificial: is_artificial = form_value.Boolean(); break; case DW_AT_accessibility: break; //accessibility = DW_ACCESS_to_AccessType(form_value.Unsigned()); break; case DW_AT_declaration: case DW_AT_description: case DW_AT_endianity: case DW_AT_segment: - case DW_AT_start_scope: case DW_AT_visibility: default: case DW_AT_abstract_origin: @@ -4392,19 +4424,20 @@ if (symbol_context_scope) { SymbolFileTypeSP type_sp(new SymbolFileType(*this, DIERef(type_die_form).GetUID())); - + if (const_value.Form() && type_sp && type_sp->GetType()) location.CopyOpcodeData(const_value.Unsigned(), type_sp->GetType()->GetByteSize(), die.GetCU()->GetAddressByteSize()); - + var_sp.reset (new Variable (die.GetID(), - name, + name, mangled, type_sp, - scope, - symbol_context_scope, - &decl, - location, - is_external, + scope, + symbol_context_scope, + scope_ranges, + &decl, + location, + is_external, is_artificial, is_static_member)); Index: source/Core/Address.cpp =================================================================== --- source/Core/Address.cpp +++ source/Core/Address.cpp @@ -760,10 +760,11 @@ bool stop_if_block_is_inlined_function = false; VariableList variable_list; sc.block->AppendVariables (can_create, - get_parent_variables, - stop_if_block_is_inlined_function, + get_parent_variables, + stop_if_block_is_inlined_function, + [](Variable*) { return true; }, &variable_list); - + const size_t num_variables = variable_list.GetSize(); for (size_t var_idx = 0; var_idx < num_variables; ++var_idx) { Index: source/API/SBFrame.cpp =================================================================== --- source/API/SBFrame.cpp +++ source/API/SBFrame.cpp @@ -789,9 +789,10 @@ const bool get_parent_variables = true; const bool stop_if_block_is_inlined_function = true; - if (sc.block->AppendVariables (can_create, + if (sc.block->AppendVariables (can_create, get_parent_variables, stop_if_block_is_inlined_function, + [frame](Variable* v) { return v->IsInScope(frame); }, &variable_list)) { var_sp = variable_list.FindVariable (ConstString(name)); @@ -887,6 +888,7 @@ sc.block->AppendVariables(can_create, get_parent_variables, stop_if_block_is_inlined_function, + [frame](Variable* v) { return v->IsInScope(frame); }, &variable_list); if (value_type == eValueTypeVariableGlobal) { Index: source/API/SBBlock.cpp =================================================================== --- source/API/SBBlock.cpp +++ source/API/SBBlock.cpp @@ -131,7 +131,11 @@ if (IsValid()) { bool show_inline = true; - m_opaque_ptr->AppendVariables (can_create, get_parent_variables, show_inline, var_list); + m_opaque_ptr->AppendVariables (can_create, + get_parent_variables, + show_inline, + [](Variable*) { return true; }, + var_list); } } Index: include/lldb/Symbol/Variable.h =================================================================== --- include/lldb/Symbol/Variable.h +++ include/lldb/Symbol/Variable.h @@ -17,6 +17,7 @@ #include "lldb/lldb-private.h" #include "lldb/lldb-enumerations.h" #include "lldb/Core/Mangled.h" +#include "lldb/Core/RangeMap.h" #include "lldb/Core/UserID.h" #include "lldb/Expression/DWARFExpression.h" #include "lldb/Symbol/Declaration.h" @@ -27,6 +28,8 @@ public std::enable_shared_from_this<Variable> { public: + typedef RangeVector<lldb::addr_t, lldb::addr_t> RangeList; + //------------------------------------------------------------------ // Constructors and Destructors //------------------------------------------------------------------ @@ -36,6 +39,7 @@ const lldb::SymbolFileTypeSP &symfile_type_sp, lldb::ValueType scope, SymbolContextScope *owner_scope, + const RangeList& scope_range, Declaration* decl, const DWARFExpression& location, bool external, @@ -178,12 +182,14 @@ CompilerDecl GetDecl (); + protected: ConstString m_name; // The basename of the variable (no namespaces) Mangled m_mangled; // The mangled name of the variable lldb::SymbolFileTypeSP m_symfile_type_sp; // The type pointer of the variable (int, struct, class, etc) lldb::ValueType m_scope; // global, parameter, local SymbolContextScope *m_owner_scope; // The symbol file scope that this variable was defined in + RangeList m_scope_range; // The list of ranges inside the owner's scope where this variable is valid Declaration m_declaration; // Declaration location for this item. DWARFExpression m_location; // The location of this variable that can be fed to DWARFExpression::Evaluate() uint8_t m_external:1, // Visible outside the containing compile unit? Index: include/lldb/Symbol/Block.h =================================================================== --- include/lldb/Symbol/Block.h +++ include/lldb/Symbol/Block.h @@ -310,8 +310,9 @@ AppendBlockVariables (bool can_create, bool get_child_block_variables, bool stop_if_child_block_is_inlined_function, + const std::function<bool(Variable*)>& filter, VariableList *variable_list); - + //------------------------------------------------------------------ /// Appends the variables from this block, and optionally from all /// parent blocks, to \a variable_list. @@ -341,9 +342,10 @@ /// variable_list. //------------------------------------------------------------------ uint32_t - AppendVariables (bool can_create, - bool get_parent_variables, + AppendVariables (bool can_create, + bool get_parent_variables, bool stop_if_block_is_inlined_function, + const std::function<bool(Variable*)>& filter, VariableList *variable_list); //------------------------------------------------------------------ Index: include/lldb/Core/RangeMap.h =================================================================== --- include/lldb/Core/RangeMap.h +++ include/lldb/Core/RangeMap.h @@ -202,7 +202,13 @@ { m_entries.push_back (entry); } - + + void + Append (B base, S size) + { + m_entries.emplace_back(base, size); + } + bool RemoveEntrtAtIndex (uint32_t idx) { @@ -471,7 +477,13 @@ { m_entries.push_back (entry); } - + + void + Append (B base, S size) + { + m_entries.emplace_back(base, size); + } + bool RemoveEntrtAtIndex (uint32_t idx) {
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits