Page MenuHomePhabricator

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

Authored by vbalu on May 1 2017, 10:04 PM.

Details

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"

Diff Detail

Repository
rL LLVM

Event Timeline

vbalu created this revision.May 1 2017, 10:04 PM

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 updated this revision to Diff 97569.May 3 2017, 1:26 AM

Added the test case.

jingham requested changes to this revision.May 3 2017, 10:55 AM
jingham added a reviewer: 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.

This revision now requires changes to proceed.May 3 2017, 10:55 AM
vbalu added a comment.May 3 2017, 11:46 PM

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.

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 updated this revision to Diff 98762.EditedMay 12 2017, 7:30 AM
vbalu edited edge metadata.

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.

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.

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 accepted this revision.May 31 2017, 10:38 AM

Okay, that seems reasonable.

This revision is now accepted and ready to land.May 31 2017, 10:38 AM