Skip to content

Commit 4c73a62

Browse files
committed
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 da83e96) Conflicts: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
1 parent bd328af commit 4c73a62

File tree

5 files changed

+42
-33
lines changed

5 files changed

+42
-33
lines changed
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
void stop() {}
1+
void relative();
22

33
int main()
44
{
5-
stop();
6-
// Hello World!
5+
relative();
6+
// Hello Absolute!
77
return 0;
88
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
void stop() {}
2+
void relative() {
3+
stop();
4+
// Hello Relative!
5+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
BOTDIR = $(BUILDDIR)/buildbot
22
USERDIR = $(BUILDDIR)/user
33
C_SOURCES = $(BOTDIR)/main.c
4+
LD_EXTRAS = $(BOTDIR)/relative.o
45

56
include Makefile.rules
7+
8+
$(EXE): relative.o
9+
relative.o: $(BOTDIR)/relative.c
10+
cd $(BOTDIR) && $(CC) -c $(CFLAGS) -o $@ relative.c

lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ def build(self):
1717
lldbutil.mkdir_p(botdir)
1818
lldbutil.mkdir_p(userdir)
1919
import shutil
20-
for f in ['main.c']:
20+
for f in ['main.c', 'relative.c']:
2121
shutil.copyfile(os.path.join(inputs, f), os.path.join(botdir, f))
2222
shutil.copyfile(os.path.join(inputs, f), os.path.join(userdir, f))
2323

@@ -52,5 +52,10 @@ def build(self):
5252
@skipIf(debug_info=no_match("dsym"))
5353
def test(self):
5454
self.build()
55-
lldbutil.run_to_name_breakpoint(self, 'main')
56-
self.expect("source list", substrs=["Hello World"])
55+
56+
target, process, _, _ = lldbutil.run_to_name_breakpoint(
57+
self, 'main')
58+
self.expect("source list -n main", substrs=["Hello Absolute"])
59+
bkpt = target.BreakpointCreateByName('relative')
60+
lldbutil.continue_to_breakpoint(process, bkpt)
61+
self.expect("source list -n relative", substrs=["Hello Relative"])

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

Lines changed: 21 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,23 @@ ParseLLVMLineTable(lldb_private::DWARFContext &context,
178178
return *line_table;
179179
}
180180

181+
static llvm::Optional<std::string>
182+
GetFileByIndex(const llvm::DWARFDebugLine::Prologue &prologue, size_t idx,
183+
llvm::StringRef compile_dir, FileSpec::Style style) {
184+
// Try to get an absolute path first.
185+
std::string abs_path;
186+
auto absolute = llvm::DILineInfoSpecifier::FileLineInfoKind::AbsoluteFilePath;
187+
if (prologue.getFileNameByIndex(idx, compile_dir, absolute, abs_path, style))
188+
return std::move(abs_path);
189+
190+
// Otherwise ask for a relative path.
191+
std::string rel_path;
192+
auto relative = llvm::DILineInfoSpecifier::FileLineInfoKind::Default;
193+
if (!prologue.getFileNameByIndex(idx, compile_dir, relative, rel_path, style))
194+
return {};
195+
return std::move(rel_path);
196+
}
197+
181198
static FileSpecList ParseSupportFilesFromPrologue(
182199
const lldb::ModuleSP &module,
183200
const llvm::DWARFDebugLine::Prologue &prologue, FileSpec::Style style,
@@ -189,35 +206,12 @@ static FileSpecList ParseSupportFilesFromPrologue(
189206
FileSpec::GuessPathStyle(compile_dir);
190207
const size_t number_of_files = prologue.FileNames.size();
191208
for (size_t idx = 1; idx <= number_of_files; ++idx) {
192-
std::string original_file;
193-
if (!prologue.getFileNameByIndex(
194-
idx, compile_dir,
195-
llvm::DILineInfoSpecifier::FileLineInfoKind::Default, original_file,
196-
style)) {
197-
// Always add an entry so the indexes remain correct.
198-
support_files.EmplaceBack();
199-
continue;
200-
}
201-
202-
FileSpec::Style style = FileSpec::Style::native;
203-
if (compile_dir_style) {
204-
style = *compile_dir_style;
205-
} else if (llvm::Optional<FileSpec::Style> file_style =
206-
FileSpec::GuessPathStyle(original_file)) {
207-
style = *file_style;
208-
}
209-
210209
std::string remapped_file;
211-
if (!prologue.getFileNameByIndex(
212-
idx, compile_dir,
213-
llvm::DILineInfoSpecifier::FileLineInfoKind::AbsoluteFilePath,
214-
remapped_file, style)) {
215-
// Always add an entry so the indexes remain correct.
216-
support_files.EmplaceBack(original_file, style);
217-
continue;
218-
}
210+
if (auto file_path = GetFileByIndex(prologue, idx, compile_dir, style))
211+
if (!module->RemapSourceFile(llvm::StringRef(*file_path), remapped_file))
212+
remapped_file = std::move(*file_path);
219213

220-
module->RemapSourceFile(llvm::StringRef(original_file), remapped_file);
214+
// Unconditionally add an entry, so the indices match up.
221215
support_files.EmplaceBack(remapped_file, style);
222216
}
223217

0 commit comments

Comments
 (0)