Page MenuHomePhabricator

Remove premature caching of the global variables list in CompileUnit.
ClosedPublic

Authored by aprantl on Apr 27 2018, 4:18 PM.

Details

Summary

Remove premature caching of the global variables list in CompileUnit.
This is fixing a bug where

(lldb) target var g_ptr

would propulate the global variables list with exactly one entry because SymbolFileDWARF::ParseVariables() was invoked with a list of DIEs pre-filtered by name, such that a subsequent call to

(lldb) fr var --show-globals

would only list that one variable, because CompileUnit::m_variables was already initialized, fooling CompileUnit::GetVariableList().

Diff Detail

Repository
rL LLVM

Event Timeline

aprantl created this revision.Apr 27 2018, 4:18 PM

This also adds test coverage for r224559, which is how I discovered this bug in the first place.

clayborg added inline comments.Apr 30 2018, 2:06 PM
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
4221 ↗(On Diff #144418)

So do we cache now somewhere else now that "sc.comp_unit->SetVariableList(variable_list_sp);" is removed?

aprantl added inline comments.Apr 30 2018, 2:21 PM
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
4221 ↗(On Diff #144418)

Yes, CompileUnit::GetVariableList() grabs the *complete* list of variables via (SymbolFileDWARF, ...)::ParseVariablesForContext and that still calls sc.comp_unit->SetVariableList(variables); which acts as the caching mechanism.

clayborg accepted this revision.Apr 30 2018, 2:29 PM

Sounds good. Looks fine.

This revision is now accepted and ready to land.Apr 30 2018, 2:29 PM
This revision was automatically updated to reflect the committed changes.
labath added a subscriber: labath.May 1 2018, 3:15 AM
labath added inline comments.
lldb/trunk/packages/Python/lldbsuite/test/lang/c/global_variables/TestGlobalVariables.py
42

this part wasn't correctly working for us on arm64, so I've split it off into a separate test, so we can still have coverage from the rest of the checks here. I think I managed to preserve the intent of your test, but you may want to have a quick look at the result (r331250).

63–65

Why do you need to run this command twice?

aprantl added inline comments.May 1 2018, 8:39 AM
lldb/trunk/packages/Python/lldbsuite/test/lang/c/global_variables/TestGlobalVariables.py
63–65

Sorry, that's leftover debugging code. I'll remove it!