Skip to content

Commit

Permalink
Revert "[lldb] Refactor variable parsing"
Browse files Browse the repository at this point in the history 
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 601168e.
  • Loading branch information
@labath
labath committed Oct 5, 2021
1 parent 8096759 commit ca5be06
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 133 deletions.
220 changes: 100 additions & 120 deletions lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
View file
Expand Up @@ -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 ||
Expand Down Expand Up @@ -2188,7 +2188,7 @@ void SymbolFileDWARF::FindGlobalVariables(const RegularExpression &regex,
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;
});
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}

Expand Down
18 changes: 5 additions & 13 deletions lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
View file
Expand Up @@ -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);

Expand Down

0 comments on commit ca5be06

Please sign in to comment.