From bd328afad024d9099ce00c559efbbbeef95bdb2a Mon Sep 17 00:00:00 2001 From: Adrian Prantl Date: Fri, 8 Nov 2019 14:05:45 -0800 Subject: [PATCH 1/4] Add a testcase for .dSYM path remapping dictionaries. rdar://problem/56924558 (cherry picked from commit d24bce57c3ca2414ff5e53d8f7f3f007d6a946fe) --- .../DBGSourcePathRemapping/Inputs/main.c | 8 +++ .../macosx/DBGSourcePathRemapping/Makefile | 5 ++ .../TestDSYMSourcePathRemapping.py | 56 +++++++++++++++++++ 3 files changed, 69 insertions(+) create mode 100644 lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/main.c create mode 100644 lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile create mode 100644 lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py diff --git a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/main.c b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/main.c new file mode 100644 index 0000000000000..556bda3c17d1a --- /dev/null +++ b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/main.c @@ -0,0 +1,8 @@ +void stop() {} + +int main() +{ + stop(); + // Hello World! + return 0; +} diff --git a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile new file mode 100644 index 0000000000000..f36a8dc1e6713 --- /dev/null +++ b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile @@ -0,0 +1,5 @@ +BOTDIR = $(BUILDDIR)/buildbot +USERDIR = $(BUILDDIR)/user +C_SOURCES = $(BOTDIR)/main.c + +include Makefile.rules diff --git a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py new file mode 100644 index 0000000000000..d13a047486728 --- /dev/null +++ b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py @@ -0,0 +1,56 @@ +import lldb +from lldbsuite.test.decorators import * +import lldbsuite.test.lldbtest as lldbtest +import lldbsuite.test.lldbutil as lldbutil +import os +import unittest2 + + +class TestDSYMSourcePathRemapping(lldbtest.TestBase): + + mydir = lldbtest.TestBase.compute_mydir(__file__) + + def build(self): + botdir = self.getBuildArtifact('buildbot') + userdir = self.getBuildArtifact('user') + inputs = self.getSourcePath('Inputs') + lldbutil.mkdir_p(botdir) + lldbutil.mkdir_p(userdir) + import shutil + for f in ['main.c']: + shutil.copyfile(os.path.join(inputs, f), os.path.join(botdir, f)) + shutil.copyfile(os.path.join(inputs, f), os.path.join(userdir, f)) + + super(TestDSYMSourcePathRemapping, self).build() + + # Remove the build sources. + self.assertTrue(os.path.isdir(botdir)) + shutil.rmtree(botdir) + + # Create a plist. + import subprocess + dsym = self.getBuildArtifact('a.out.dSYM') + uuid = subprocess.check_output(["/usr/bin/dwarfdump", "--uuid", dsym] + ).decode("utf-8").split(" ")[1] + import re + self.assertTrue(re.match(r'[0-9a-fA-F-]+', uuid)) + plist = os.path.join(dsym, 'Contents', 'Resources', uuid + '.plist') + with open(plist, 'w') as f: + f.write('\n') + f.write('\n') + f.write('\n') + f.write('\n') + f.write(' DBGSourcePathRemapping\n') + f.write(' \n') + f.write(' ' + botdir + '\n') + f.write(' ' + userdir + '\n') + f.write(' \n') + f.write('\n') + f.write('\n') + + + @skipIf(debug_info=no_match("dsym")) + def test(self): + self.build() + lldbutil.run_to_name_breakpoint(self, 'main') + self.expect("source list", substrs=["Hello World"]) From 4c73a624bfb6acd684d0d110ad496c542fae13b9 Mon Sep 17 00:00:00 2001 From: Adrian Prantl Date: Fri, 8 Nov 2019 17:35:52 -0800 Subject: [PATCH 2/4] Fix a regression in macOS-style path remapping. When we switched to the LLVM .debug_line parser, the .dSYM-style path remapping logic stopped working for relative paths because of how RemapSourceFile silently fails for relative paths. This patch both makes the code more readable and fixes this particular bug. One interesting thing I learned is that Module::RemapSourceFile() is a macOS-only code path that operates on on the lldb::Module level and is completely separate from target.source-map, which operates on a per-Target level. Differential Revision: https://reviews.llvm.org/D70037 rdar://problem/56924558 (cherry picked from commit da83e96273527a137f2ebd77cedb920180eab621) Conflicts: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp --- .../DBGSourcePathRemapping/Inputs/main.c | 6 +-- .../DBGSourcePathRemapping/Inputs/relative.c | 5 ++ .../macosx/DBGSourcePathRemapping/Makefile | 5 ++ .../TestDSYMSourcePathRemapping.py | 11 +++-- .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 48 ++++++++----------- 5 files changed, 42 insertions(+), 33 deletions(-) create mode 100644 lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/relative.c diff --git a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/main.c b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/main.c index 556bda3c17d1a..41a6a46c92610 100644 --- a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/main.c +++ b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/main.c @@ -1,8 +1,8 @@ -void stop() {} +void relative(); int main() { - stop(); - // Hello World! + relative(); + // Hello Absolute! return 0; } diff --git a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/relative.c b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/relative.c new file mode 100644 index 0000000000000..02331834cf21f --- /dev/null +++ b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/relative.c @@ -0,0 +1,5 @@ +void stop() {} +void relative() { + stop(); + // Hello Relative! +} diff --git a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile index f36a8dc1e6713..8c82c73b13fc6 100644 --- a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile +++ b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile @@ -1,5 +1,10 @@ BOTDIR = $(BUILDDIR)/buildbot USERDIR = $(BUILDDIR)/user C_SOURCES = $(BOTDIR)/main.c +LD_EXTRAS = $(BOTDIR)/relative.o include Makefile.rules + +$(EXE): relative.o +relative.o: $(BOTDIR)/relative.c + cd $(BOTDIR) && $(CC) -c $(CFLAGS) -o $@ relative.c diff --git a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py index d13a047486728..2ee3791503920 100644 --- a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py +++ b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py @@ -17,7 +17,7 @@ def build(self): lldbutil.mkdir_p(botdir) lldbutil.mkdir_p(userdir) import shutil - for f in ['main.c']: + for f in ['main.c', 'relative.c']: shutil.copyfile(os.path.join(inputs, f), os.path.join(botdir, f)) shutil.copyfile(os.path.join(inputs, f), os.path.join(userdir, f)) @@ -52,5 +52,10 @@ def build(self): @skipIf(debug_info=no_match("dsym")) def test(self): self.build() - lldbutil.run_to_name_breakpoint(self, 'main') - self.expect("source list", substrs=["Hello World"]) + + target, process, _, _ = lldbutil.run_to_name_breakpoint( + self, 'main') + self.expect("source list -n main", substrs=["Hello Absolute"]) + bkpt = target.BreakpointCreateByName('relative') + lldbutil.continue_to_breakpoint(process, bkpt) + self.expect("source list -n relative", substrs=["Hello Relative"]) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index 20e6cd2d28a31..6dfeb1c5f7850 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -178,6 +178,23 @@ ParseLLVMLineTable(lldb_private::DWARFContext &context, return *line_table; } +static llvm::Optional +GetFileByIndex(const llvm::DWARFDebugLine::Prologue &prologue, size_t idx, + llvm::StringRef compile_dir, FileSpec::Style style) { + // Try to get an absolute path first. + std::string abs_path; + auto absolute = llvm::DILineInfoSpecifier::FileLineInfoKind::AbsoluteFilePath; + if (prologue.getFileNameByIndex(idx, compile_dir, absolute, abs_path, style)) + return std::move(abs_path); + + // Otherwise ask for a relative path. + std::string rel_path; + auto relative = llvm::DILineInfoSpecifier::FileLineInfoKind::Default; + if (!prologue.getFileNameByIndex(idx, compile_dir, relative, rel_path, style)) + return {}; + return std::move(rel_path); +} + static FileSpecList ParseSupportFilesFromPrologue( const lldb::ModuleSP &module, const llvm::DWARFDebugLine::Prologue &prologue, FileSpec::Style style, @@ -189,35 +206,12 @@ static FileSpecList ParseSupportFilesFromPrologue( FileSpec::GuessPathStyle(compile_dir); const size_t number_of_files = prologue.FileNames.size(); for (size_t idx = 1; idx <= number_of_files; ++idx) { - std::string original_file; - if (!prologue.getFileNameByIndex( - idx, compile_dir, - llvm::DILineInfoSpecifier::FileLineInfoKind::Default, original_file, - style)) { - // Always add an entry so the indexes remain correct. - support_files.EmplaceBack(); - continue; - } - - FileSpec::Style style = FileSpec::Style::native; - if (compile_dir_style) { - style = *compile_dir_style; - } else if (llvm::Optional file_style = - FileSpec::GuessPathStyle(original_file)) { - style = *file_style; - } - std::string remapped_file; - if (!prologue.getFileNameByIndex( - idx, compile_dir, - llvm::DILineInfoSpecifier::FileLineInfoKind::AbsoluteFilePath, - remapped_file, style)) { - // Always add an entry so the indexes remain correct. - support_files.EmplaceBack(original_file, style); - continue; - } + if (auto file_path = GetFileByIndex(prologue, idx, compile_dir, style)) + if (!module->RemapSourceFile(llvm::StringRef(*file_path), remapped_file)) + remapped_file = std::move(*file_path); - module->RemapSourceFile(llvm::StringRef(original_file), remapped_file); + // Unconditionally add an entry, so the indices match up. support_files.EmplaceBack(remapped_file, style); } From 47a41861f95ea37f984abd3c77951740057c4e3f Mon Sep 17 00:00:00 2001 From: Adrian Prantl Date: Mon, 11 Nov 2019 12:22:55 -0800 Subject: [PATCH 3/4] Replace tabs with spaces. (NFC) (cherry picked from commit 646d927175ebd0bb1d7af7d51b41bc1d7b1fe651) --- .../TestDSYMSourcePathRemapping.py | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py index 2ee3791503920..0f5daf51e975e 100644 --- a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py +++ b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py @@ -35,24 +35,24 @@ def build(self): import re self.assertTrue(re.match(r'[0-9a-fA-F-]+', uuid)) plist = os.path.join(dsym, 'Contents', 'Resources', uuid + '.plist') - with open(plist, 'w') as f: + with open(plist, 'w') as f: f.write('\n') f.write('\n') - f.write('\n') - f.write('\n') - f.write(' DBGSourcePathRemapping\n') - f.write(' \n') - f.write(' ' + botdir + '\n') - f.write(' ' + userdir + '\n') - f.write(' \n') - f.write('\n') - f.write('\n') + f.write('\n') + f.write('\n') + f.write(' DBGSourcePathRemapping\n') + f.write(' \n') + f.write(' ' + botdir + '\n') + f.write(' ' + userdir + '\n') + f.write(' \n') + f.write('\n') + f.write('\n') @skipIf(debug_info=no_match("dsym")) def test(self): self.build() - + target, process, _, _ = lldbutil.run_to_name_breakpoint( self, 'main') self.expect("source list -n main", substrs=["Hello Absolute"]) From 952413c24961cf195622a1991cc5827ed29ff561 Mon Sep 17 00:00:00 2001 From: Adrian Prantl Date: Fri, 8 Nov 2019 08:58:50 -0800 Subject: [PATCH 4/4] Adapt LLDB to clang API change in ObjCMethodDecl::create(). (cherry picked from commit 454acae97ca4ad25cac582afe66c616ad46e4dd1) --- .../AppleObjCRuntime/AppleObjCDeclVendor.cpp | 7 ++-- lldb/source/Symbol/ClangASTContext.cpp | 32 ++++++++++++------- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp index 3800b81d9768a..45704b2925ff8 100644 --- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp +++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp @@ -330,7 +330,8 @@ class ObjCRuntimeMethodType { const bool isInstance = instance; const bool isVariadic = false; - const bool isSynthesized = false; + const bool isPropertyAccessor = false; + const bool isSynthesizedAccessorStub = false; const bool isImplicitlyDeclared = true; const bool isDefined = false; const clang::ObjCMethodDecl::ImplementationControl impControl = @@ -377,8 +378,8 @@ class ObjCRuntimeMethodType { clang::ObjCMethodDecl *ret = clang::ObjCMethodDecl::Create( ast_ctx, clang::SourceLocation(), clang::SourceLocation(), sel, ret_type, nullptr, interface_decl, isInstance, isVariadic, - isSynthesized, isImplicitlyDeclared, isDefined, impControl, - HasRelatedResultType); + isPropertyAccessor, isSynthesizedAccessorStub, isImplicitlyDeclared, + isDefined, impControl, HasRelatedResultType); std::vector parm_vars; diff --git a/lldb/source/Symbol/ClangASTContext.cpp b/lldb/source/Symbol/ClangASTContext.cpp index 2cb01ce016067..14e9144424fec 100644 --- a/lldb/source/Symbol/ClangASTContext.cpp +++ b/lldb/source/Symbol/ClangASTContext.cpp @@ -8528,7 +8528,8 @@ bool ClangASTContext::AddObjCClassProperty( ? class_interface_decl->lookupInstanceMethod(getter_sel) : class_interface_decl->lookupClassMethod(getter_sel))) { const bool isVariadic = false; - const bool isSynthesized = false; + const bool isPropertyAccessor = false; + const bool isSynthesizedAccessorStub = false; const bool isImplicitlyDeclared = true; const bool isDefined = false; const clang::ObjCMethodDecl::ImplementationControl impControl = @@ -8539,7 +8540,8 @@ bool ClangASTContext::AddObjCClassProperty( *clang_ast, clang::SourceLocation(), clang::SourceLocation(), getter_sel, ClangUtil::GetQualType(property_clang_type_to_access), nullptr, class_interface_decl, isInstance, isVariadic, - isSynthesized, isImplicitlyDeclared, isDefined, impControl, + isPropertyAccessor, isSynthesizedAccessorStub, + isImplicitlyDeclared, isDefined, impControl, HasRelatedResultType); if (getter && metadata) @@ -8560,7 +8562,8 @@ bool ClangASTContext::AddObjCClassProperty( : class_interface_decl->lookupClassMethod(setter_sel))) { clang::QualType result_type = clang_ast->VoidTy; const bool isVariadic = false; - const bool isSynthesized = false; + const bool isPropertyAccessor = true; + const bool isSynthesizedAccessorStub = false; const bool isImplicitlyDeclared = true; const bool isDefined = false; const clang::ObjCMethodDecl::ImplementationControl impControl = @@ -8570,8 +8573,9 @@ bool ClangASTContext::AddObjCClassProperty( clang::ObjCMethodDecl *setter = clang::ObjCMethodDecl::Create( *clang_ast, clang::SourceLocation(), clang::SourceLocation(), setter_sel, result_type, nullptr, class_interface_decl, - isInstance, isVariadic, isSynthesized, isImplicitlyDeclared, - isDefined, impControl, HasRelatedResultType); + isInstance, isVariadic, isPropertyAccessor, + isSynthesizedAccessorStub, isImplicitlyDeclared, isDefined, + impControl, HasRelatedResultType); if (setter && metadata) ClangASTContext::SetMetadata(clang_ast, setter, *metadata); @@ -8673,10 +8677,16 @@ clang::ObjCMethodDecl *ClangASTContext::AddMethodToObjCObjectType( if (!method_function_prototype) return nullptr; - bool is_synthesized = false; - bool is_defined = false; - clang::ObjCMethodDecl::ImplementationControl imp_control = + const bool isInstance = (name[0] == '-'); + const bool isVariadic = false; + const bool isPropertyAccessor = false; + const bool isSynthesizedAccessorStub = false; + /// Force this to true because we don't have source locations. + const bool isImplicitlyDeclared = true; + const bool isDefined = false; + const clang::ObjCMethodDecl::ImplementationControl impControl = clang::ObjCMethodDecl::None; + const bool HasRelatedResultType = false; const unsigned num_args = method_function_prototype->getNumParams(); @@ -8692,10 +8702,8 @@ clang::ObjCMethodDecl *ClangASTContext::AddMethodToObjCObjectType( nullptr, // TypeSourceInfo *ResultTInfo, ClangASTContext::GetASTContext(ast)->GetDeclContextForType( ClangUtil::GetQualType(type)), - name[0] == '-', is_variadic, is_synthesized, - true, // is_implicitly_declared; we force this to true because we don't - // have source locations - is_defined, imp_control, false /*has_related_result_type*/); + isInstance, isVariadic, isPropertyAccessor, isSynthesizedAccessorStub, + isImplicitlyDeclared, isDefined, impControl, HasRelatedResultType); if (objc_method_decl == nullptr) return nullptr;