[Lldb-commits] [PATCH] D133164: Add the ability to show when variables fails to be available when debug info is valid.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.


================
Comment at: lldb/include/lldb/Target/StackFrame.h:264
   ///     A pointer to a list of variables.
-  VariableList *GetVariableList(bool get_file_globals);
+  VariableList *GetVariableList(bool get_file_globals, Status *error_ptr);
 
----------------
labath wrote:
> Could this return `Expected<VariableList*>` ?
We don't have the equivalent of Expected in LLDB that uses a Status, so I opted for this as it was already being used. I started with Expected<VariableList*> but the code using it was messy and had conversions to llvm::Error and back to Status again. Also using an Expected with a pointer didn't look good and caused some bad looking code.


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:4161
+  if (command) {
+    if (command->contains(" -gline-tables-only"))
+      return Status("-gline-tables-only enabled, no variable information is "
----------------
labath wrote:
> This isn't a particularly reliable way of detecting whether variable information was emitted. For example a command line `clang -gline-tables-only -g2` will in fact produce full debug info and `clang -g1` will not. Could we make that determination based on the presence of actual variable DIEs in the debug info? Perhaps query the index whether it knows of any (global) variable or any type defined within the compile unit?
This function only gets called when there are no variables in a stack frame at all and checks for reasons why. So if "-gline-tables-only -g2" is used, this function won't get called if there were variables.

I planned to make a follow up patch that detected no variables in a compile uint by checking the compile unit's abbreviation set to see if it had any DW_TAG_variable or DW_TAG_formal_parameter definitions, and if there weren't any respond with "-gline-tables-only might be enabled....". 

If we see this option for sure and have the side affect of there being no variables, I would like to user the users know what they can do to fix things if at all possible. 


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:4162-4163
+    if (command->contains(" -gline-tables-only"))
+      return Status("-gline-tables-only enabled, no variable information is "
+                    "available in debug info for this compile unit");
+  }
----------------
I could rephrase this as "-gline-tables-only detected in compiler flags, variable information might be been removed for this compile unit"?


================
Comment at: lldb/test/API/commands/frame/var/TestFrameVar.py:174-175
+        '''
+        self.build(dictionary={'CFLAGS_EXTRAS': '-gline-tables-only'},
+                   env={"RC_DEBUG_OPTIONS": "1"})
+        exe = self.getBuildArtifact("a.out")
----------------
labath wrote:
> Why not just pass `-grecord-command-line` in CFLAGS_EXTRAS? I think then you should be able to remove @skipUnlessDarwin from this test...
I thought I tried that, but I was able to get this to work as suggested. I will change this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133164/new/

https://reviews.llvm.org/D133164