[Lldb-commits] [lldb] ca5be06 - Revert "[lldb] Refactor variable parsing"
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Tue Oct 5 01:46:40 PDT 2021
Author: Pavel Labath
Date: 2021-10-05T10:46:30+02:00
New Revision: ca5be065c4c612554acdcae3ead01a1474eff296
URL: https://github.com/llvm/llvm-project/commit/ca5be065c4c612554acdcae3ead01a1474eff296
DIFF: https://github.com/llvm/llvm-project/commit/ca5be065c4c612554acdcae3ead01a1474eff296.diff
LOG: 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.
Added:
Modified:
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
Removed:
################################################################################
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index c111ff6567364..f89fb1655d3bf 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<VariableList>();
+ }
+ } 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<VariableList>();
+ 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<VariableList>();
- 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 d001742342bd7..ec8ce2ac9ad7d 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -378,19 +378,11 @@ class SymbolFileDWARF : public lldb_private::SymbolFile,
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);
More information about the lldb-commits
mailing list