From ca5be065c4c612554acdcae3ead01a1474eff296 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Tue, 5 Oct 2021 10:37:52 +0200 Subject: [PATCH] Revert "[lldb] Refactor variable parsing" This commit has introduced test failures in internal google tests. Working theory is they are caused by a genuine problem in the patch which gets tripped by some debug info from system libraries. Reverting while we try to reproduce the problem in a self-contained fashion. This reverts commit 601168e42037ac4433e74b920bb22f76d59ba420. --- .../Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp | 220 ++++++++++----------- .../Plugins/SymbolFile/DWARF/SymbolFileDWARF.h | 18 +- 2 files changed, 105 insertions(+), 133 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index c111ff6..f89fb16 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -2135,7 +2135,7 @@ void SymbolFileDWARF::FindGlobalVariables( } } - ParseAndAppendGlobalVariable(sc, die, variables); + ParseVariables(sc, die, LLDB_INVALID_ADDRESS, false, false, &variables); while (pruned_idx < variables.GetSize()) { VariableSP var_sp = variables.GetVariableAtIndex(pruned_idx); if (name_is_mangled || @@ -2188,7 +2188,7 @@ void SymbolFileDWARF::FindGlobalVariables(const RegularExpression ®ex, return true; sc.comp_unit = GetCompUnitForDWARFCompUnit(*dwarf_cu); - ParseAndAppendGlobalVariable(sc, die, variables); + ParseVariables(sc, die, LLDB_INVALID_ADDRESS, false, false, &variables); return variables.GetSize() - original_size < max_matches; }); @@ -3049,8 +3049,8 @@ size_t SymbolFileDWARF::ParseVariablesForContext(const SymbolContext &sc) { /*check_hi_lo_pc=*/true)) func_lo_pc = ranges.GetMinRangeBase(0); if (func_lo_pc != LLDB_INVALID_ADDRESS) { - const size_t num_variables = - ParseVariablesInFunctionContext(sc, function_die, func_lo_pc); + const size_t num_variables = ParseVariables( + sc, function_die.GetFirstChild(), func_lo_pc, true, true); // Let all blocks know they have parse all their variables sc.function->GetBlock(false).SetDidParseVariables(true, true); @@ -3479,137 +3479,117 @@ SymbolFileDWARF::FindBlockContainingSpecification( return DWARFDIE(); } -void SymbolFileDWARF::ParseAndAppendGlobalVariable( - const SymbolContext &sc, const DWARFDIE &die, - VariableList &cc_variable_list) { - if (!die) - return; - - dw_tag_t tag = die.Tag(); - if (tag != DW_TAG_variable && tag != DW_TAG_constant) - return; - - // Check to see if we have already parsed this variable or constant? - VariableSP var_sp = GetDIEToVariable()[die.GetDIE()]; - if (var_sp) { - cc_variable_list.AddVariableIfUnique(var_sp); - return; - } - - // We haven't already parsed it, lets do that now. - VariableListSP variable_list_sp; - DWARFDIE sc_parent_die = GetParentSymbolContextDIE(die); - dw_tag_t parent_tag = sc_parent_die.Tag(); - switch (parent_tag) { - case DW_TAG_compile_unit: - case DW_TAG_partial_unit: - if (sc.comp_unit != nullptr) { - variable_list_sp = sc.comp_unit->GetVariableList(false); - } else { - GetObjectFile()->GetModule()->ReportError( - "parent 0x%8.8" PRIx64 " %s with no valid compile unit in " - "symbol context for 0x%8.8" PRIx64 " %s.\n", - sc_parent_die.GetID(), sc_parent_die.GetTagAsCString(), die.GetID(), - die.GetTagAsCString()); - return; - } - break; - - default: - GetObjectFile()->GetModule()->ReportError( - "didn't find appropriate parent DIE for variable list for " - "0x%8.8" PRIx64 " %s.\n", - die.GetID(), die.GetTagAsCString()); - return; - } - - var_sp = ParseVariableDIE(sc, die, LLDB_INVALID_ADDRESS); - if (!var_sp) - return; - - cc_variable_list.AddVariableIfUnique(var_sp); - if (variable_list_sp) - variable_list_sp->AddVariableIfUnique(var_sp); -} - -size_t SymbolFileDWARF::ParseVariablesInFunctionContext( - const SymbolContext &sc, const DWARFDIE &die, - const lldb::addr_t func_low_pc) { - if (!die || !sc.function) +size_t SymbolFileDWARF::ParseVariables(const SymbolContext &sc, + const DWARFDIE &orig_die, + const lldb::addr_t func_low_pc, + bool parse_siblings, bool parse_children, + VariableList *cc_variable_list) { + if (!orig_die) return 0; - Block *block = - sc.function->GetBlock(/*can_create=*/true).FindBlockByID(die.GetID()); - const bool can_create = false; - VariableListSP variable_list_sp = block->GetBlockVariableList(can_create); - return ParseVariablesInFunctionContextRecursive(sc, die, func_low_pc, - *variable_list_sp); -} + VariableListSP variable_list_sp; -size_t SymbolFileDWARF::ParseVariablesInFunctionContextRecursive( - const lldb_private::SymbolContext &sc, const DWARFDIE &die, - const lldb::addr_t func_low_pc, VariableList &variable_list) { size_t vars_added = 0; - dw_tag_t tag = die.Tag(); + DWARFDIE die = orig_die; + while (die) { + dw_tag_t tag = die.Tag(); - if ((tag == DW_TAG_variable) || (tag == DW_TAG_constant) || - (tag == DW_TAG_formal_parameter)) { - VariableSP var_sp(ParseVariableDIE(sc, die, func_low_pc)); + // Check to see if we have already parsed this variable or constant? + VariableSP var_sp = GetDIEToVariable()[die.GetDIE()]; if (var_sp) { - variable_list.AddVariableIfUnique(var_sp); - ++vars_added; - } - } + if (cc_variable_list) + cc_variable_list->AddVariableIfUnique(var_sp); + } else { + // We haven't already parsed it, lets do that now. + if ((tag == DW_TAG_variable) || (tag == DW_TAG_constant) || + (tag == DW_TAG_formal_parameter && sc.function)) { + if (variable_list_sp.get() == nullptr) { + DWARFDIE sc_parent_die = GetParentSymbolContextDIE(orig_die); + dw_tag_t parent_tag = sc_parent_die.Tag(); + switch (parent_tag) { + case DW_TAG_compile_unit: + case DW_TAG_partial_unit: + if (sc.comp_unit != nullptr) { + variable_list_sp = sc.comp_unit->GetVariableList(false); + if (variable_list_sp.get() == nullptr) { + variable_list_sp = std::make_shared(); + } + } else { + GetObjectFile()->GetModule()->ReportError( + "parent 0x%8.8" PRIx64 " %s with no valid compile unit in " + "symbol context for 0x%8.8" PRIx64 " %s.\n", + sc_parent_die.GetID(), sc_parent_die.GetTagAsCString(), + orig_die.GetID(), orig_die.GetTagAsCString()); + } + break; - switch (tag) { - case DW_TAG_subprogram: - case DW_TAG_inlined_subroutine: - case DW_TAG_lexical_block: { - // If we start a new block, compute a new block variable list and recurse. - Block *block = - sc.function->GetBlock(/*can_create=*/true).FindBlockByID(die.GetID()); - if (block == nullptr) { - // This must be a specification or abstract origin with a - // concrete block counterpart in the current function. We need - // to find the concrete block so we can correctly add the - // variable to it. - const DWARFDIE concrete_block_die = FindBlockContainingSpecification( - GetDIE(sc.function->GetID()), die.GetOffset()); - if (concrete_block_die) - block = sc.function->GetBlock(/*can_create=*/true) - .FindBlockByID(concrete_block_die.GetID()); - } + case DW_TAG_subprogram: + case DW_TAG_inlined_subroutine: + case DW_TAG_lexical_block: + if (sc.function != nullptr) { + // Check to see if we already have parsed the variables for the + // given scope + + Block *block = sc.function->GetBlock(true).FindBlockByID( + sc_parent_die.GetID()); + if (block == nullptr) { + // This must be a specification or abstract origin with a + // concrete block counterpart in the current function. We need + // to find the concrete block so we can correctly add the + // variable to it + const DWARFDIE concrete_block_die = + FindBlockContainingSpecification( + GetDIE(sc.function->GetID()), + sc_parent_die.GetOffset()); + if (concrete_block_die) + block = sc.function->GetBlock(true).FindBlockByID( + concrete_block_die.GetID()); + } - if (block == nullptr) - return 0; + if (block != nullptr) { + const bool can_create = false; + variable_list_sp = block->GetBlockVariableList(can_create); + if (variable_list_sp.get() == nullptr) { + variable_list_sp = std::make_shared(); + block->SetVariableList(variable_list_sp); + } + } + } + break; - const bool can_create = false; - VariableListSP block_variable_list_sp = - block->GetBlockVariableList(can_create); - if (block_variable_list_sp.get() == nullptr) { - block_variable_list_sp = std::make_shared(); - block->SetVariableList(block_variable_list_sp); - } - for (DWARFDIE child = die.GetFirstChild(); child; - child = child.GetSibling()) { - vars_added += ParseVariablesInFunctionContextRecursive( - sc, child, func_low_pc, *block_variable_list_sp); + default: + GetObjectFile()->GetModule()->ReportError( + "didn't find appropriate parent DIE for variable list for " + "0x%8.8" PRIx64 " %s.\n", + orig_die.GetID(), orig_die.GetTagAsCString()); + break; + } + } + + if (variable_list_sp) { + VariableSP var_sp(ParseVariableDIE(sc, die, func_low_pc)); + if (var_sp) { + variable_list_sp->AddVariableIfUnique(var_sp); + if (cc_variable_list) + cc_variable_list->AddVariableIfUnique(var_sp); + ++vars_added; + } + } + } } - break; - } + bool skip_children = (sc.function == nullptr && tag == DW_TAG_subprogram); - default: - // Recurse to children with the same variable list. - for (DWARFDIE child = die.GetFirstChild(); child; - child = child.GetSibling()) { - vars_added += ParseVariablesInFunctionContextRecursive( - sc, child, func_low_pc, variable_list); + if (!skip_children && parse_children && die.HasChildren()) { + vars_added += ParseVariables(sc, die.GetFirstChild(), func_low_pc, true, + true, cc_variable_list); } - break; + if (parse_siblings) + die = die.GetSibling(); + else + die.Clear(); } - return vars_added; } diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h index d001742..ec8ce2a 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h @@ -378,19 +378,11 @@ protected: const DWARFDIE &die, const lldb::addr_t func_low_pc); - void - ParseAndAppendGlobalVariable(const lldb_private::SymbolContext &sc, - const DWARFDIE &die, - lldb_private::VariableList &cc_variable_list); - - size_t ParseVariablesInFunctionContext(const lldb_private::SymbolContext &sc, - const DWARFDIE &die, - const lldb::addr_t func_low_pc); - - size_t ParseVariablesInFunctionContextRecursive( - const lldb_private::SymbolContext &sc, const DWARFDIE &die, - const lldb::addr_t func_low_pc, - lldb_private::VariableList &variable_list); + size_t ParseVariables(const lldb_private::SymbolContext &sc, + const DWARFDIE &orig_die, + const lldb::addr_t func_low_pc, bool parse_siblings, + bool parse_children, + lldb_private::VariableList *cc_variable_list = nullptr); bool ClassOrStructIsVirtual(const DWARFDIE &die); -- 2.7.4