Page MenuHomePhabricator

[lldb/DWARF] Ignore debug info pointing to the low addresses
ClosedPublic

Authored by labath on Oct 19 2021, 1:52 AM.

Details

Summary

specifically, ignore addresses that point before the first code section.

This resurrects D87172 with several notable changes:

  • it fixes a bug where the early exits in InitializeObject left m_first_code_address "initialized" to LLDB_INVALID_ADDRESS (0xfff..f), which caused _everything_ to be ignored.
  • it extends the line table fix to function parsing as well, where it replaces a similar check which was checking the executable permissions of the section. This was insufficient because some position-independent elf executables can have an executable segment mapped at file address zero. (What makes this fix different is that it checks for the executable-ness of the sections contained within that segment, and those will not be at address zero.)
  • It uses a different test case, with an elf file with near-zero addresses, and checks for both line table and function parsing.

Diff Detail

Event Timeline

labath requested review of this revision.Oct 19 2021, 1:52 AM
labath created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptOct 19 2021, 1:52 AM
shafik added a subscriber: shafik.Oct 19 2021, 9:01 AM
shafik added inline comments.
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
472

It would be nice to wrap this up into InitializeFirstCodeAddress(...) it could be done by making iterating over the section list a lambda and then checking after that or splitting it into a helper etc

labath updated this revision to Diff 380715.Oct 19 2021, 9:49 AM

Move the recursion into a helper function

labath added inline comments.Oct 19 2021, 9:50 AM
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
472

Yeah I was wondering about that myself. I've created a new function for that now.

clayborg accepted this revision.Oct 19 2021, 12:09 PM

Looks good. Do we need a follow up patch to avoid creating functions that should have been stripped?

This revision is now accepted and ready to land.Oct 19 2021, 12:09 PM
labath added a comment.EditedOct 20 2021, 1:57 AM

Looks good. Do we need a follow up patch to avoid creating functions that should have been stripped?

I'm not entirely sure what you mean by that. Are you referring to the fact that SymbolFileDWARF::ResolveFunction creates an lldb_private::Function, which it does not add the to result SymbolContextList (instead of not creating the object in the first place?

If yes, then that sounds like a good idea. I don't know if this has more user-facing implications, but I was definitely puzzled by the fact that the stripped function still showed up in "image dump symfile". I'll see what can be done about that.

Looks good. Do we need a follow up patch to avoid creating functions that should have been stripped?

I'm not entirely sure what you mean by that. Are you referring to the fact that SymbolFileDWARF::ResolveFunction creates an lldb_private::Function, which it does not add the to result SymbolContextList (instead of not creating the object in the first place?

If yes, then that sounds like a good idea. I don't know if this has more user-facing implications, but I was definitely puzzled by the fact that the stripped function still showed up in "image dump symfile". I'll see what can be done about that.

Yeah, that is what I was talking about. I think if we modify SymbolFileDWARF::ParseFunction() that would be the easiest to quickly get the ranges for the DW_TAG_subprogram and avoid calling DWARFASTParser::ParseFunctionFromDWARF() if the low PC is less than m_first_code_address we can avoid bloat in the ASTs and keep our memory footprint down in the LLDB process.