cmtice created this revision. cmtice added a reviewer: jingham. Herald added subscribers: cishida, hiraditya. Herald added a reviewer: int3. Herald added a project: lld-macho. Herald added a reviewer: lld-macho. cmtice requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: llvm-commits, lldb-commits, sstefan1. Herald added projects: LLDB, LLVM.
DWARF allows .dwo file paths to be relative rather than absolute. When they are relative, DWARF uses DW_AT_comp_dir to find the .dwo file. DW_AT_comp_dir can also be relative, making the entire search patch for the .dwo file relative. In this case, LLDB currently searches relative to its current working directory, i.e. the directory from which the debugger was launched. This is not right, as the compiler, which generated the relative paths, can have no idea where the debugger will be launched. The correct thing is to search relative to the location of the executable binary. That is what this patch does. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D97786 Files: lld/MachO/Driver.h lld/MachO/InputFiles.cpp lld/test/MachO/Inputs/iPhoneSimulator.sdk/usr/lib/libReexportSystem.tbd lld/test/MachO/Inputs/iPhoneSimulator.sdk/usr/lib/libSystem.tbd lld/test/MachO/reexport-nested-lib.s lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp llvm/include/llvm/TextAPI/MachO/InterfaceFile.h llvm/lib/TextAPI/MachO/InterfaceFile.cpp
Index: llvm/lib/TextAPI/MachO/InterfaceFile.cpp =================================================================== --- llvm/lib/TextAPI/MachO/InterfaceFile.cpp +++ llvm/lib/TextAPI/MachO/InterfaceFile.cpp @@ -124,6 +124,7 @@ const std::shared_ptr<InterfaceFile> &RHS) { return LHS->InstallName < RHS->InstallName; }); + Document->Parent = this; Documents.insert(Pos, Document); } Index: llvm/include/llvm/TextAPI/MachO/InterfaceFile.h =================================================================== --- llvm/include/llvm/TextAPI/MachO/InterfaceFile.h +++ llvm/include/llvm/TextAPI/MachO/InterfaceFile.h @@ -338,6 +338,9 @@ ///\param Document The library to inline with top level library. void addDocument(std::shared_ptr<InterfaceFile> &&Document); + /// Returns the pointer to parent document if exists or nullptr otherwise. + InterfaceFile *getParent() const { return Parent; } + /// Get the list of inlined libraries. /// /// \return Returns a list of the inlined frameworks. @@ -431,6 +434,7 @@ std::vector<std::shared_ptr<InterfaceFile>> Documents; std::vector<std::pair<Target, std::string>> UUIDs; SymbolMapType Symbols; + InterfaceFile *Parent = nullptr; }; template <typename DerivedT, typename KeyInfoT, typename BucketT> Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp =================================================================== --- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -1639,6 +1639,18 @@ return nullptr; dwo_file.SetFile(comp_dir, FileSpec::Style::native); + if (dwo_file.IsRelative()) { + // if DW_AT_comp_dir is relative, it should be relative to the location + // of the executable, not to the location from which the debugger was + // launched. + ModuleSpec module_spec; + module_spec.GetFileSpec() = m_objfile_sp->GetFileSpec(); + module_spec.GetSymbolFileSpec() = + FileSpec(m_objfile_sp->GetModule()->GetFileSpec().GetPath()); + llvm::StringRef exe_dir = + module_spec.GetFileSpec().GetDirectory().GetStringRef(); + dwo_file.PrependPathComponent(exe_dir); + } FileSystem::Instance().Resolve(dwo_file); dwo_file.AppendPathComponent(dwo_name); } Index: lld/test/MachO/reexport-nested-lib.s =================================================================== --- /dev/null +++ lld/test/MachO/reexport-nested-lib.s @@ -0,0 +1,28 @@ +# REQUIRES: x86 +# +# This tests that we can reference symbols from a dylib, +# re-exported by a top-level tapi document, which itself is +# re-exported by another top-level tapi document. +# +# RUN: rm -rf %t; mkdir -p %t +# RUN: llvm-mc -filetype obj -triple x86_64-apple-darwin %s -o %t/test.o +# RUN: %lld -o %t/test -syslibroot %S/Inputs/iPhoneSimulator.sdk -lReexportSystem %t/test.o +# RUN: llvm-objdump %t/test --macho --bind %t/test | FileCheck %s + +# CHECK: segment section address type addend dylib symbol +# CHECK: __DATA __data 0x{{[0-9a-f]*}} pointer 0 libReexportSystem __crashreporter_info__ +# CHECK: __DATA __data 0x{{[0-9a-f]*}} pointer 0 libReexportSystem _cache_create + +.text +.globl _main + +_main: + ret + +.data +// This symbol is from libSystem, which is re-exported by libReexportSystem. +// Reference it here to verify that it is visible. +.quad __crashreporter_info__ + +// This symbol is from /usr/lib/system/libcache.dylib, which is re-exported in libSystem. +.quad _cache_create Index: lld/test/MachO/Inputs/iPhoneSimulator.sdk/usr/lib/libSystem.tbd =================================================================== --- lld/test/MachO/Inputs/iPhoneSimulator.sdk/usr/lib/libSystem.tbd +++ lld/test/MachO/Inputs/iPhoneSimulator.sdk/usr/lib/libSystem.tbd @@ -6,8 +6,8 @@ current-version: 1281 exports: - archs: [ i386, x86_64 ] - re-exports: [ '/usr/lib/system/libcache.dylib' ] - symbols: [ __crashreporter_info__ ] + re-exports: [ '/usr/lib/system/libcache.dylib', ] + symbols: [ __crashreporter_info__, _cache_create ] --- !tapi-tbd-v3 archs: [ i386, x86_64 ] uuids: [ 'i386: 00000000-0000-0000-0000-000000000002', 'x86_64: 00000000-0000-0000-0000-000000000003' ] Index: lld/test/MachO/Inputs/iPhoneSimulator.sdk/usr/lib/libReexportSystem.tbd =================================================================== --- /dev/null +++ lld/test/MachO/Inputs/iPhoneSimulator.sdk/usr/lib/libReexportSystem.tbd @@ -0,0 +1,10 @@ +--- !tapi-tbd-v3 +archs: [ i386, x86_64 ] +uuids: [ 'i386: 00000000-0000-0000-0000-000000000000', 'x86_64: 00000000-0000-0000-0000-000000000001' ] +platform: ios +install-name: '/usr/lib/libReexportSystem.dylib' +exports: + - archs: [ i386, x86_64 ] + re-exports: [ '/usr/lib/libSystem' ] + symbols: [ __crashreporter_info__, _cache_create ] + Index: lld/MachO/InputFiles.cpp =================================================================== --- lld/MachO/InputFiles.cpp +++ lld/MachO/InputFiles.cpp @@ -575,19 +575,16 @@ // TBD files are parsed into a series of TAPI documents (InterfaceFiles), with // the first document storing child pointers to the rest of them. When we are -// processing a given TBD file, we store that top-level document here. When -// processing re-exports, we search its children for potentially matching -// documents in the same TBD file. Note that the children themselves don't -// point to further documents, i.e. this is a two-level tree. +// processing a given TBD file, we store that top-level document in +// currentTopLevelTapi. When processing re-exports, we search its children for +// potentially matching documents in the same TBD file. Note that the children +// themselves don't point to further documents, i.e. this is a two-level tree. // -// ld64 allows a TAPI re-export to reference documents nested within other TBD -// files, but that seems like a strange design, so this is an intentional -// deviation. -const InterfaceFile *currentTopLevelTapi = nullptr; - // Re-exports can either refer to on-disk files, or to documents within .tbd // files. -static Optional<DylibFile *> findDylib(StringRef path, DylibFile *umbrella) { +static Optional<DylibFile *> +findDylib(StringRef path, DylibFile *umbrella, + const InterfaceFile *currentTopLevelTapi) { if (path::is_absolute(path, path::Style::posix)) for (StringRef root : config->systemLibraryRoots) if (Optional<std::string> dylibPath = @@ -631,8 +628,10 @@ return false; } -void loadReexport(StringRef path, DylibFile *umbrella) { - Optional<DylibFile *> reexport = findDylib(path, umbrella); +void loadReexport(StringRef path, DylibFile *umbrella, + const InterfaceFile *currentTopLevelTapi) { + Optional<DylibFile *> reexport = + findDylib(path, umbrella, currentTopLevelTapi); if (!reexport) error("unable to locate re-export with install name " + path); else if (isImplicitlyLinked(path)) @@ -690,7 +689,7 @@ const auto *c = reinterpret_cast<const dylib_command *>(cmd); StringRef reexportPath = reinterpret_cast<const char *>(c) + read32le(&c->dylib.name); - loadReexport(reexportPath, umbrella); + loadReexport(reexportPath, umbrella, nullptr); } // FIXME: What about LC_LOAD_UPWARD_DYLIB, LC_LAZY_LOAD_DYLIB, @@ -701,7 +700,7 @@ const auto *c = reinterpret_cast<const dylib_command *>(cmd); StringRef dylibPath = reinterpret_cast<const char *>(c) + read32le(&c->dylib.name); - Optional<DylibFile *> dylib = findDylib(dylibPath, umbrella); + Optional<DylibFile *> dylib = findDylib(dylibPath, umbrella, nullptr); if (!dylib) error(Twine("unable to locate library '") + dylibPath + "' loaded from '" + toString(this) + "' for -flat_namespace"); @@ -758,17 +757,11 @@ } } - bool isTopLevelTapi = false; - if (currentTopLevelTapi == nullptr) { - currentTopLevelTapi = &interface; - isTopLevelTapi = true; - } + const InterfaceFile *topLevel = + interface.getParent() == nullptr ? &interface : interface.getParent(); for (InterfaceFileRef intfRef : interface.reexportedLibraries()) - loadReexport(intfRef.getInstallName(), umbrella); - - if (isTopLevelTapi) - currentTopLevelTapi = nullptr; + loadReexport(intfRef.getInstallName(), umbrella, topLevel); } ArchiveFile::ArchiveFile(std::unique_ptr<object::Archive> &&f) Index: lld/MachO/Driver.h =================================================================== --- lld/MachO/Driver.h +++ lld/MachO/Driver.h @@ -15,6 +15,12 @@ #include "llvm/Option/OptTable.h" #include "llvm/Support/MemoryBuffer.h" +namespace llvm { +namespace MachO { +class InterfaceFile; +} // namespace MachO +} // namespace llvm + namespace lld { namespace macho {
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits