Page MenuHomePhabricator

[lldb] Don't process symlinks deep inside DWARFUnit
ClosedPublic

Authored by labath on Dec 20 2019, 7:40 AM.

Details

Summary

This code is handling debug info paths starting with /proc/self/cwd,
which is one of the mechanisms people use to obtain "relocatable" debug
info (the idea being that one starts the debugger with an appropriate
cwd and things "just work").

Instead of resolving the symlinks inside DWARFUnit, we can do the same
thing more elegantly by hooking into the existing Module path remapping
code. Since llvm::DWARFUnit does not support any similar functionality,
doing things this way is also a step towards unifying llvm and lldb
dwarf parsers.

Diff Detail

Event Timeline

labath created this revision.Dec 20 2019, 7:40 AM
Herald added a project: Restricted Project. · View Herald Transcript

Does the mean these mappings will show up in the settings command?

JDevlieghere accepted this revision.Dec 20 2019, 3:57 PM
This revision is now accepted and ready to land.Dec 20 2019, 3:57 PM

Does the mean these mappings will show up in the settings command?

No, these won't show up in the target.source-map setting because this is setting the "per-module" mapping list, and not the global target map. I don't thing we actually have a mechanism to display the per-module maps...

This revision was automatically updated to reflect the committed changes.
labath reopened this revision.Jan 9 2020, 7:11 AM
This revision is now accepted and ready to land.Jan 9 2020, 7:11 AM
labath updated this revision to Diff 237076.Jan 9 2020, 7:11 AM

Use a slightly different approach as the previous one didn't work on macos (the
reason for that is that each .o file gets a separate module -- the patch
populated the path map of the .o modules, but the remapping was being done by
the main module).

In this version I move the remapping code from SymbolFileDWARF into generic
code. I think this makes sense as the /proc/self/cwd trick is not really
specific to DWARF. It can be used e.g. with clang-cl and PDBs too (if
cross-compiling on a platform which supports symbolic links).

A small behavioral change here is that in order for a change in the setting
value to "kick in", one has to change it before the module gets created --
previously one could do it before debug info gets parsed. I don't think this is
particularly important as the only way to get reliable results was to change the
value before creating a target -- debug info parsing happens lazily, and a
pending breakpoing could cause it to get parsed immediately.

labath requested review of this revision.Jan 9 2020, 7:13 AM
labath edited reviewers, added: jingham; removed: jdoerfert.

Re-requesting review, as this is essentially a different patch.

jdoerfert resigned from this revision.Jan 9 2020, 8:32 AM
JDevlieghere accepted this revision.Jan 13 2020, 11:14 AM

LGTM

lldb/include/lldb/Core/Module.h
972

Nit: can you move the comment before the variable with ///? ///< rarely makes sense with the 80 col limit and it's already broken in the comment for m_old_symfiles...

This revision is now accepted and ready to land.Jan 13 2020, 11:14 AM
labath marked 2 inline comments as done.Mon, Jan 20, 3:58 AM
labath added inline comments.
lldb/include/lldb/Core/Module.h
972

Good point.

This revision was automatically updated to reflect the committed changes.
labath marked an inline comment as done.

Looks like this broke the ASAN bot?

http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake-sanitized/783/testReport/junit/lldb-api/functionalities_breakpoint_comp_dir_symlink/TestCompDirSymLink_py/

lldb-api.functionalities/breakpoint/comp_dir_symlink.TestCompDirSymLink.py (from lldb-api):

=================================================================
==83604==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fff77668712 bp 0x7ffee0e29f10 sp 0x7ffee0e29f10 T0)
==83604==The signal is caused by a READ memory access.
==83604==Hint: address points to the zero page.
    #0 0x7fff77668712 in _platform_strlen+0x12 (libsystem_platform.dylib:x86_64+0x1712)
    #1 0x10ee1191b in wrap_readlink+0x5b (libclang_rt.asan_osx_dynamic.dylib:x86_64+0x3991b)
    #2 0x119a61e57 in lldb_private::FileSystem::Readlink(lldb_private::FileSpec const&, lldb_private::FileSpec&) FileSystemPosix.cpp:46
    #3 0x1197f4b0d in lldb_private::ModuleListProperties::UpdateSymlinkMappings() ModuleList.cpp:122
    #4 0x119af5664 in lldb_private::OptionValueFileSpecList::SetValueFromString(llvm::StringRef, lldb_private::VarSetOperationType) OptionValueFileSpecList.cpp
    #5 0x119b01851 in lldb_private::OptionValueProperties::SetSubValue(lldb_private::ExecutionContext const*, lldb_private::VarSetOperationType, llvm::StringRef, llvm::StringRef) OptionValueProperties.cpp:209
    #6 0x119886a80 in lldb_private::Properties::SetPropertyValue(lldb_private::ExecutionContext const*, lldb_private::VarSetOperationType, llvm::StringRef, llvm::StringRef) UserSettingsController.cpp:49
    #7 0x1197234cb in lldb_private::Debugger::SetPropertyValue(lldb_private::ExecutionContext const*, lldb_private::VarSetOperationType, llvm::StringRef, llvm::StringRef) Debugger.cpp:275
    #8 0x11c2e19ef in CommandObjectSettingsSet::DoExecute(llvm::StringRef, lldb_private::CommandReturnObject&) CommandObjectSettings.cpp:222
    #9 0x119aba342 in lldb_private::CommandObjectRaw::Execute(char const*, lldb_private::CommandReturnObject&) CommandObject.cpp:1003
    #10 0x119a94b4a in lldb_private::CommandInterpreter::HandleCommand(char const*, lldb_private::LazyBool, lldb_private::CommandReturnObject&, lldb_private::ExecutionContext*, bool, bool) CommandInterpreter.cpp:1762
    #11 0x118c1b547 in lldb::SBCommandInterpreter::HandleCommand(char const*, lldb::SBExecutionContext&, lldb::SBCommandReturnObject&, bool) SBCommandInterpreter.cpp:281
    #12 0x118c1a83a in lldb::SBCommandInterpreter::HandleCommand(char const*, lldb::SBCommandReturnObject&, bool) SBCommandInterpreter.cpp:259
    #13 0x1193cfa28 in _wrap_SBCommandInterpreter_HandleCommand(_object*, _object*) LLDBWrapPython.cpp:14074

Looks like a missing nullptr check.

Thanks. I believe this should be fixed by 0157a74be. I'll check the bot to confirm.