"target variable" not showing all global variable if we print any global variable before execution starts

[vbalu]

Summary

"target variable" at main not printing all the global variables if we have print ant global variable before starting execution. Instead it show only variables that we printed before starting execution.
It wont work on three scenarios.
case 1:

(lldb) print my_global_char ==> global variable
(char) $0 = 'X'
(lldb) b main
(lldb) r
(lldb) target variable
Global variables for **
(char) my_global_char = 'X' ==> not showing other global variables
(lldb)
case 2:

(lldb) target variable -r my_global_ch* ==> global variable
(char) $0 = 'X'
(lldb) b main
(lldb) r
(lldb) target variable
Global variables for **
(char) my_global_char = 'X' ==> not showing other global variables
(lldb)
case 3:

(lldb) target variable my_global_char ==> global variable
(char) $0 = 'X'
(lldb) b main
(lldb) r
(lldb) target variable
Global variables for **
(char) my_global_char = 'X' ==> not showing other global variables
(lldb)
For all other scopes we parse all the variables before we look for the required variable, so it would be updated in m_varibles.
For global scope, we look for required variable and only that updated m_variables which taken back for "target variable"

[labath]

Thanks for the patch. Could you also write a test case for the bug? It sounds like all that is necessary is to move the commands from your commit message into a test.

Jim, who would be a good reviewer for this?

[vbalu]

Added the test case.

[jingham]

I actually think the behavior you are seeing is the designed behavior, but it isn't clearly documented.

Target variable is designed to help manage the large number of global & static variables that you can have in a complex system. So we avoid having the command with no arguments just dumping all the global variables in every library in the program in most cases. That can be pages and pages of output...

The current method for this is: before you run, when target variable has no context, it will either search for globals & statics by name if you provide them, or show all the globals in whatever shared libraries you name. It doesn't ever show all globals everywhere in this case:

 > lldb ~/Projects/Sketch/build/Debug/Sketch.app
(lldb) target create "/Volumes/ThePlayground/Users/jingham/Projects/Sketch/build/Debug/Sketch.app"
Current executable set to '/Volumes/ThePlayground/Users/jingham/Projects/Sketch/build/Debug/Sketch.app' (x86_64).
(lldb) target variable
error: 'target variable' takes one or more global variable names as arguments

(lldb) target variable SKTAppAutosavingDelayPreferenceKey
(NSString *) SKTAppAutosavingDelayPreferenceKey = 0x00000001000223f8
(lldb) target variable --shlib Sketch
Global variables for /Volumes/ThePlayground/Users/jingham/Projects/Sketch/build/Debug/Sketch.app/Contents/MacOS/Sketch
(NSString *) SKTAppAutosavingDelayPreferenceKey = 0x00000001000223f8
(NSString *) SKTAppAutosavesPreferenceKey = 0x00000001000223d8
...

That first error could be a little nicer, actually...

Then when you are in a frame scope, what we show by default is the statics and globals defined in the CU holding that frame. That is why you aren't seeing other variables once you've stopped.

If you want to broaden the search you can either specify a name, or specify a shared library.

I think the current behavior is preferable to dumping all the globals, which if you have debug information for a substantial part of the system is overwhelming and not useful.

I'm also confused about the changes to the expression parser lookup. The expression parser isn't trying to limit information in the way "target variable" does, and should already look everywhere for globals (of course starting from the current frame -> containing CU -> containing dylib and finally out to all dylibs. I don't see any indication from your examples that this isn't working, and your test only test "target variable", it doesn't use the expression parser.

[vbalu]

i am confused as I dont see it is similar way when i don't print global variable before strting inferior.
For the c file "lldbsuite/test/functionalities/target_command/globals.c" i see "target variable" behaves differently as below.

Without printing global variable before starting inferior:

(lldb) target create "global"
Current executable set to 'global' (x86_64).
(lldb) b main
Breakpoint 1: where = global`main + 30 at globals.c:18, address = 0x000000000040074e
(lldb) r
Process 37645 launching
Process 37645 launched: '/b/vignesh/lldb_temp/global' (x86_64)
Process 37645 stopped
* thread #1, name = 'global', stop reason = breakpoint 1.1
    frame #0: 0x000000000040074e global`main(argc=1, argv=0x00007fffffffe9d0) at globals.c:18
   15
   16   int main (int argc, char const *argv[])
   17   {
-> 18           int a =0;
   19           int b =0;
   20
   21       printf("global char: %c\n", my_global_char);
(lldb) target variable
Global variables for /b/vignesh/lldb_temp/globals.c in /b/vignesh/lldb_temp/global:
(char) my_global_char = 'X'
(const char *) my_global_str = 0x0000000000400806 "abc"
(const char **) my_global_str_ptr = 0x0000000000600af0
(int) my_static_int = 228
(lldb)

With printing global variable before starting inferior

(lldb) target create "global"
Current executable set to 'global' (x86_64).
(lldb) target variable my_global_char
(char) my_global_char = 'X'
(lldb) b main
Breakpoint 1: where = global`main + 30 at globals.c:18, address = 0x000000000040074e
(lldb) r
Process 37681 launching
Process 37681 launched: '/b/vignesh/lldb_temp/global' (x86_64)
Process 37681 stopped
* thread #1, name = 'global', stop reason = breakpoint 1.1
    frame #0: 0x000000000040074e global`main(argc=1, argv=0x00007fffffffe9d0) at globals.c:18
   15
   16   int main (int argc, char const *argv[])
   17   {
-> 18           int a =0;
   19           int b =0;
   20
   21       printf("global char: %c\n", my_global_char);
lldb) target variable
Global variables for /b/vignesh/lldb_temp/globals.c in /b/vignesh/lldb_temp/global:
(char) my_global_char = 'X'
(lldb)

Please let me know if above change in behavior is expected. If so Please explain.

[jingham]

Yes, I was printing a global variable that WASN'T defined in the source file I later stopped in. So it worked for me. The bug is really that looking up a variable by name goes through and adds only that variable to the CU that defines it, but the CU doesn't make note of the fact that it didn't actually fully populate the m_variables for the CU. So then later on when you ask for them, we think we got them all, which of course we haven't.

The fix needs to be in CompileUnit. It shouldn't use m_variables.get to tell itself that it has finished getting variables for the CU. It has to keep another flag telling itself that it has gotten ALL the variables defined in the comp unit, and if that's false, it should keep adding.

[vbalu]

Modified as suggested. I used the flag option "flagsParsedVariables" to monitor if global variable are completely parsed or not.
Looks like it never used. For a compile unit global variable will be parsed only through the method "GetVariableList" so i set the flag there.

[jingham]

That looks like the right way to do it. What was your thinking behind returning null rather than the partial list of variables already parsed if can_create is false? That doesn't seem like what somebody who is bothering to pass in can_create false intends.

[vbalu]

In D32732#753348, @jingham wrote:

That looks like the right way to do it. What was your thinking behind returning null rather than the partial list of variables already parsed if can_create is false? That doesn't seem like what somebody who is bothering to pass in can_create false intends.

The issue is use of m_variables when it is partially Parsed. when a function looking of a global variable count it just call the GetVariableList with "can_create=false" which will return the partially parsed m_variabe leads to wrong count.
"can_create" avoid the recursion while parsing the variables and to get the parsed global variable list to the callee. So i thought returning NULL should be wise instead of sending partially parsed list.

[jingham]

Okay, that seems reasonable.

packages/Python/lldbsuite/test/functionalities/target_command/TestTargetCommand.py

class targetCommandTestCase(TestBase):
     def test_target_variable_command_no_fail(self):
         """Test 'target variable' command before and after starting the inferior."""
         d = {'C_SOURCES': 'globals.c', 'EXE': 'globals'}
         self.build(dictionary=d)
         self.addTearDownCleanup(dictionary=d)
 
         self.do_target_variable_command_no_fail('globals')
 
+    @expectedFailureAndroid(archs=['aarch64'])
+    def test_target_variable_after_print_command(self):
+        """Test 'print' command before and 'target var' after starting the inferior."""
+        d = {'C_SOURCES': 'globals.c', 'EXE': 'globals'}
+        self.build(dictionary=d)
+        self.addTearDownCleanup(dictionary=d)
+
+        self.do_target_variable_after_print_command('globals')
+
+    @expectedFailureAndroid(archs=['aarch64'])
+    def test_target_variable_with_regex(self):
+        """Test 'target var -r' command before and 'target var' after starting the inferior."""
+        d = {'C_SOURCES': 'globals.c', 'EXE': 'globals'}
+        self.build(dictionary=d)
+        self.addTearDownCleanup(dictionary=d)
+
+        self.do_target_variable_with_regex('globals')
+
     def do_target_command(self):
         """Exercise 'target create', 'target list', 'target select' commands."""
         exe_a = os.path.join(os.getcwd(), "a.out")
         exe_b = os.path.join(os.getcwd(), "b.out")
         exe_c = os.path.join(os.getcwd(), "c.out")
 
         self.runCmd("target list")
         output = self.res.GetOutput()
def do_target_variable_command_no_fail(self, exe_name):
 
         # New feature: you don't need to specify the variable(s) to 'target vaiable'.
         # It will find all the global and static variables in the current
         # compile unit.
         self.expect("target variable",
                     substrs=['my_global_char',
                              'my_global_str',
                              'my_global_str_ptr',
-                             'my_static_int'])
+                             'my_static_int',
+                             'my_global_int'])
 
         self.expect(
             "target variable my_global_str",
             VARIABLES_DISPLAYED_CORRECTLY,
             substrs=[
                 'my_global_str',
                 '"abc"'])
         self.expect(
def do_target_variable_command_no_fail(self, exe_name):
             VARIABLES_DISPLAYED_CORRECTLY,
             substrs=['a'])
         self.expect(
             "target variable my_global_char",
             VARIABLES_DISPLAYED_CORRECTLY,
             substrs=[
                 "my_global_char",
                 "'X'"])
+
+    def do_target_variable_after_print_command(self, exe_name):
+        """Exercise 'print' command before and 'target var' after starting the inferior."""
+        self.runCmd("file " + exe_name, CURRENT_EXECUTABLE_SET)
+
+        self.expect(
+            "target variable my_global_char",
+            VARIABLES_DISPLAYED_CORRECTLY,
+            substrs=[
+                "my_global_char",
+                "'X'"])
+
+        self.runCmd("b main")
+        self.runCmd("run")
+
+        # To test whether all varaible are shown if we print only one variable before
+        # starting inferior
+        self.expect("target variable",
+                    substrs=['my_global_char',
+                             'my_global_str',
+                             'my_global_str_ptr',
+                             'my_static_int',
+                             'my_global_int'])
+
+    def do_target_variable_with_regex(self, exe_name):
+        """Exercise 'target var -r' command before and 'target var' after starting the inferior."""
+        self.runCmd("file " + exe_name, CURRENT_EXECUTABLE_SET)
+
+        self.expect(
+            "target variable -r my_global_ch*",
+            VARIABLES_DISPLAYED_CORRECTLY,
+            substrs=[
+                "my_global_char",
+                "'X'"])
+
+        self.runCmd("b main")
+        self.runCmd("run")
+
+        # To test whether all varaible are shown if we use regex for one variable before
+        # starting inferior
+        self.expect("target variable",
+                    substrs=['my_global_char',
+                             'my_global_str',
+                             'my_global_str_ptr',
+                             'my_static_int',
+                             'my_global_int'])

packages/Python/lldbsuite/test/functionalities/target_command/globals.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>
 
 char my_global_char = 'X';
 const char* my_global_str = "abc";
 const char **my_global_str_ptr = &my_global_str;
 static int my_static_int = 228;
+int my_global_int = 10;
 
 int main (int argc, char const *argv[])
 {
     printf("global char: %c\n", my_global_char);
     
     printf("global str: %s\n", my_global_str);
 
     printf("argc + my_static_int = %d\n", (argc + my_static_int));
     
     return 0;
 }

source/Symbol/CompileUnit.cpp

void CompileUnit::SetDebugMacros(const DebugMacrosSP &debug_macros_sp) {
   if (debug_macros_sp.get() == nullptr)
     m_flags.Clear(flagsParsedDebugMacros);
   else
     m_flags.Set(flagsParsedDebugMacros);
   m_debug_macros_sp = debug_macros_sp;
 }
 
 VariableListSP CompileUnit::GetVariableList(bool can_create) {
-  if (m_variables.get() == nullptr && can_create) {
+  // Return NULL if m_variables are partially parsed
+  if (!can_create && m_flags.IsClear(flagsParsedVariables))
+    return NULL;
+  if ((m_variables.get() == nullptr || m_flags.IsClear(flagsParsedVariables)) &&
+      can_create) {
     SymbolContext sc;
     CalculateSymbolContext(&sc);
     assert(sc.module_sp);
     sc.module_sp->GetSymbolVendor()->ParseVariablesForContext(sc);
+    m_flags.Set(flagsParsedVariables);
   }
 
   return m_variables;
 }
 
 uint32_t CompileUnit::FindLineEntry(uint32_t start_idx, uint32_t line,
                                     const FileSpec *file_spec_ptr, bool exact,
                                     LineEntry *line_entry_ptr) {