Page MenuHomePhabricator

Add the ability to show when variables fails to be available when debug info is valid.
ClosedPublic

Authored by clayborg on Sep 1 2022, 3:31 PM.

Details

Summary

Many times when debugging variables might not be available even though a user can successfully set breakpoints and stops somewhere. Letting the user know will help users fix these kinds of issues and have a better debugging experience.

Examples of this include:

  • enabling -gline-tables-only and being able to set file and line breakpoints and yet see no variables
  • unable to open object file for DWARF in .o file debugging for darwin targets due to modification time mismatch or not being able to locate the N_OSO file.

This patch adds an new API to SBValueList:

lldb::SBError lldb::SBValueList::GetError();

object so that if you request a stack frame's variables using SBValueList SBFrame::GetVariables(...), you can get an error the describes why the variables were not available.

This patch adds the ability to get an error back when requesting variables from a lldb_private::StackFrame when calling GetVariableList.

It also now shows an error in response to "frame variable" if we have debug info and are unable to get varialbes due to an error as mentioned above:

(lldb) frame variable
error: "a.o" object from the "/tmp/libfoo.a" archive: either the .o file doesn't exist in the archive or the modification time (0x63111541) of the .o file doesn't match

Diff Detail

Event Timeline

clayborg created this revision.Sep 1 2022, 3:31 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 1 2022, 3:31 PM
clayborg requested review of this revision.Sep 1 2022, 3:31 PM
Herald added a project: Restricted Project. · View Herald Transcript
labath added inline comments.Sep 5 2022, 6:57 AM
lldb/include/lldb/Target/StackFrame.h
264

Could this return Expected<VariableList*> ?

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

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?

lldb/test/API/commands/frame/var/TestFrameVar.py
174–175

Why not just pass -grecord-command-line in CFLAGS_EXTRAS? I think then you should be able to remove @skipUnlessDarwin from this test...

clayborg added inline comments.Sep 6 2022, 4:13 PM
lldb/include/lldb/Target/StackFrame.h
264

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.

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

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.

4162–4163

I could rephrase this as "-gline-tables-only detected in compiler flags, variable information might be been removed for this compile unit"?

lldb/test/API/commands/frame/var/TestFrameVar.py
174–175

I thought I tried that, but I was able to get this to work as suggested. I will change this.

clayborg updated this revision to Diff 458310.Sep 6 2022, 4:19 PM

Use -grecord-command-line instead of setting RC_DEBUG_OPTIONS.

clayborg added inline comments.Sep 6 2022, 4:23 PM
lldb/include/lldb/Target/StackFrame.h
264

There are many places that call this function that also don't need to check the error and if we use a Expected<VariableList*>, we need to add a bunch of consumeError(...) code. See all of the call sites where I added a "nullptr" for the "error_ptr" to see why I chose to do it this way to keep the code cleaner. Many places are getting the variable list to look for things or use them for autocomplete.

labath added inline comments.Sep 7 2022, 6:28 AM
lldb/include/lldb/Target/StackFrame.h
264

Ok, I am convinced by the optionalness of the error in this case.

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

I get that, but this check is not completely correct in either direction. For example, clang -g1 will not produce variable information, but this code will not detect it. Same goes for clang -gmlt. And I probably missed some other ways one can prevent variable info from being generated. Keeping up with all the changes in clang flags will just be a game of whack-a-mole. If we checked the actual debug info, then we would catch all of these cases, including the (default) case when the compiler did not embed command line information into the debug info.

And I don't think we need to know the precise command line to give meaningful advice to the users. In all of these cases, sticking an extra -g at the end of the command line will cause the variables to reappear. If we wanted to, we could also put a link to the lldb web page where we can (at length) describe the various reasons why variables may not be available and how to fix them.

lldb/test/API/commands/frame/var/TestFrameVar.py
101

just change to assertIn(s, command_error), and then you get the error message for free.

174–175

Cool. Can we also remove @skipUnlessDarwin then?

lldb/test/API/functionalities/archives/Makefile
11–12

I don't know how important this is, but I believe the build was deleting the .o files to ensure that we access the copies within the archive. If you think that's fine, then just delete this line.

17–18

And probably this comment as well, because it doesn't make sense if we don't delete the other files.

clayborg updated this revision to Diff 458912.Sep 8 2022, 4:28 PM
  • Don't check for compiler arguments directly, look at all DIEs for a compile unit and return an error if there are no variable DIEs in any DIEs in each compile unit.
  • Use "assertIn(a, b)" instead of otther asserts
clayborg added inline comments.Sep 8 2022, 4:30 PM
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
4161

I switched over to just looking for any variable DIEs anywhere in the CU. This should cover all options. Let me know if you think we should add a top level web page and reference it in the error message?

lldb/test/API/functionalities/archives/Makefile
11–12

Yeah, part of my test touches the .o file and only rebuilds the libfoo.a so I have to leave the .o files there.

labath accepted this revision.Sep 9 2022, 5:59 AM

Looks good. Thanks.

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

I think it might be nice to have some kind of a "I can't debug" troubleshooting page, but I don't think we need to tie it to the future of this patch.

lldb/test/API/commands/frame/var/TestFrameVar.py
173

I guess this isn't necessary any more.

This revision is now accepted and ready to land.Sep 9 2022, 5:59 AM
clayborg added a comment.EditedSep 12 2022, 11:58 AM

Yikes, looks like you can't set a breakpoint by name 'main' on windows with the debug info stripped... I will add a skipIfWindows to the failing test.

clayborg reopened this revision.Sep 12 2022, 1:17 PM

Reopen for submission with windows fix.

This revision is now accepted and ready to land.Sep 12 2022, 1:17 PM
clayborg updated this revision to Diff 459545.Sep 12 2022, 1:17 PM

Added a @skipIfWindows to make on TestFrameVar.test_gline_tables_only as windows doesn't seem to be able to set a breakpoint by function name 'main' when debug info is -gline-tables-only

clayborg added inline comments.Sep 12 2022, 1:18 PM
lldb/test/API/commands/frame/var/TestFrameVar.py
166

This is the line that will fix the windows buildbots.

If anyone can accept this diff again it would be most appreciated, the only failing buildbot was windows and I added @skipIfWindows to the failing test.

yinghuitan accepted this revision.Sep 12 2022, 1:46 PM