Remove premature caching of the global variables list in CompileUnit.

[aprantl]

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().

[aprantl]

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

[aprantl]

ping.

[clayborg]

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]

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]

Sounds good. Looks fine.

[labath]

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]

lldb/trunk/packages/Python/lldbsuite/test/lang/c/global_variables/TestGlobalVariables.py

63–65

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

lldb/trunk/packages/Python/lldbsuite/test/lang/c/global_variables/TestGlobalVariables.py

def test_c_global_variables(self):
         lldbutil.run_break_set_by_file_and_line(
             self, self.source, self.line, num_expected_locations=1, loc_exact=True)
 
         # Register our shared libraries for remote targets so they get
         # automatically uploaded
         environment = self.registerSharedLibrariesWithTarget(
             target, self.shlib_names)
 
+        # Test that static initialized variables can be inspected without process.
+        self.expect("target variable g_ptr", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['(int *)'])
+        self.expect("target variable *g_ptr", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['42'])
+
         # Now launch the process, and do not stop at entry point.
         process = target.LaunchSimple(
             None, environment, self.get_process_working_directory())
         self.assertTrue(process, PROCESS_IS_VALID)
 
         # The stop reason of the thread should be breakpoint.
         self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
                     substrs=['stopped',
                              'stop reason = breakpoint'])
 
         # The breakpoint should have a hit count of 1.
         self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
                     substrs=[' resolved, hit count = 1'])
 
         # Check that GLOBAL scopes are indicated for the variables.
+        self.runCmd("frame variable --show-types --scope --show-globals --no-args")
         self.expect(
             "frame variable --show-types --scope --show-globals --no-args",
             VARIABLES_DISPLAYED_CORRECTLY,
             substrs=[
-                'GLOBAL: (int) g_file_global_int = 42',
                 'STATIC: (const int) g_file_static_int = 2',
+                'STATIC: (const char *) g_func_static_cstr',
                 'GLOBAL: (const char *) g_file_global_cstr',
                 '"g_file_global_cstr"',
+                'GLOBAL: (int) g_file_global_int = 42',
+                'GLOBAL: (int) g_common_1 = 21',
+                'GLOBAL: (int *) g_ptr',
                 'STATIC: (const char *) g_file_static_cstr',
-                '"g_file_static_cstr"',
-                'GLOBAL: (int) g_common_1 = 21'])
+                '"g_file_static_cstr"'
+            ])
 
         # 'frame variable' should support address-of operator.
         self.runCmd("frame variable &g_file_global_int")
 
         # Exercise the 'target variable' command to display globals in a.c
         # file.
         self.expect("target variable g_a", VARIABLES_DISPLAYED_CORRECTLY,
                     substrs=['g_a', '123'])
def test_c_global_variables(self):
             substrs=[
                 'g_marked_spot.y',
                 '21'])
         self.expect(
             "target variable g_marked_spot.y",
             VARIABLES_DISPLAYED_CORRECTLY,
             matching=False,
             substrs=["can't be resolved"])
+
+        # Test that the statically initialized variable can also be
+        # inspected *with* a process.
+        self.expect("target variable *g_ptr", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['42'])

lldb/trunk/packages/Python/lldbsuite/test/lang/c/global_variables/main.c

 //===-- main.c --------------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 #include <stdio.h>
 
 int g_common_1; // Not initialized on purpose to cause it to be undefined external in .o file
 int g_file_global_int = 42;
 static const int g_file_static_int = 2;
 const char *g_file_global_cstr = "g_file_global_cstr";
 static const char *g_file_static_cstr = "g_file_static_cstr";
+int *g_ptr = &g_file_global_int;
 
 extern int g_a;
 int main (int argc, char const *argv[])
 {
     g_common_1 = g_file_global_int / g_file_static_int;
     static const char *g_func_static_cstr = "g_func_static_cstr";
     printf ("%s %s\n", g_file_global_cstr, g_file_static_cstr);
-    return g_file_global_int + g_a + g_common_1; // Set break point at this line.  //// break $source:$line; continue; var -global g_a -global g_global_int
+    return g_file_global_int + g_a + g_common_1 + *g_ptr; // Set break point at this line.  //// break $source:$line; continue; var -global g_a -global g_global_int
 }

lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

if (var_sp) {
           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 != NULL) {
               variable_list_sp = sc.comp_unit->GetVariableList(false);
               if (variable_list_sp.get() == NULL) {
                 variable_list_sp.reset(new VariableList());
-                sc.comp_unit->SetVariableList(variable_list_sp);
               }
             } 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());