[LLVM][DWARF] Add support for .gnu_debuglink #77715
Conversation
Added support to DWARFLibrary for .gnu_debug_link. This support is already in LLDB. With this all the tools (llvm-dwarfdump, llvm-gsymutil, etc) and projects like BOLT that rely on the DWARFLibrary can benefit.
|
@llvm/pr-subscribers-debuginfo @llvm/pr-subscribers-llvm-binary-utilities Author: Alexander Yermolovich (ayermolo) ChangesAdded support to DWARFLibrary for .gnu_debug_link. This support is already in Full diff: https://github.com/llvm/llvm-project/pull/77715.diff 4 Files Affected:
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h
index fa98cbcfc4d997..c9c2d0f9d13636 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h
@@ -127,6 +127,9 @@ class DWARFContext : public DIContext {
std::unique_ptr<const DWARFObject> DObj;
+ /// Binary containing debug information after objcopy --only-keep-debug
+ object::OwningBinary<object::ObjectFile> GnuLink;
+
// When set parses debug_info.dwo/debug_abbrev.dwo manually and populates CU
// Index, and TU Index for DWARF5.
bool ParseCUTUIndexManually = false;
@@ -138,7 +141,9 @@ class DWARFContext : public DIContext {
WithColor::defaultErrorHandler,
std::function<void(Error)> WarningHandler =
WithColor::defaultWarningHandler,
- bool ThreadSafe = false);
+ bool ThreadSafe = false,
+ object::OwningBinary<object::ObjectFile> GnuLink =
+ object::OwningBinary<object::ObjectFile>());
~DWARFContext() override;
DWARFContext(DWARFContext &) = delete;
diff --git a/llvm/include/llvm/Object/Binary.h b/llvm/include/llvm/Object/Binary.h
index ce870e25acafe0..61ee3dbc6edc14 100644
--- a/llvm/include/llvm/Object/Binary.h
+++ b/llvm/include/llvm/Object/Binary.h
@@ -202,6 +202,7 @@ template <typename T> class OwningBinary {
OwningBinary(std::unique_ptr<T> Bin, std::unique_ptr<MemoryBuffer> Buf);
OwningBinary(OwningBinary<T>&& Other);
OwningBinary<T> &operator=(OwningBinary<T> &&Other);
+ void operator()(OwningBinary<T> &&Other);
std::pair<std::unique_ptr<T>, std::unique_ptr<MemoryBuffer>> takeBinary();
@@ -227,6 +228,12 @@ OwningBinary<T> &OwningBinary<T>::operator=(OwningBinary &&Other) {
return *this;
}
+template <typename T> void OwningBinary<T>::operator()(OwningBinary &&Other) {
+ Bin = std::move(Other.Bin);
+ Buf = std::move(Other.Buf);
+ return *this;
+}
+
template <typename T>
std::pair<std::unique_ptr<T>, std::unique_ptr<MemoryBuffer>>
OwningBinary<T>::takeBinary() {
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
index c671aedbc9e52b..50015bcd18f86d 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
@@ -741,21 +741,19 @@ class ThreadSafeState : public ThreadUnsafeDWARFContextState {
}
};
-
-
DWARFContext::DWARFContext(std::unique_ptr<const DWARFObject> DObj,
std::string DWPName,
std::function<void(Error)> RecoverableErrorHandler,
std::function<void(Error)> WarningHandler,
- bool ThreadSafe)
- : DIContext(CK_DWARF),
- RecoverableErrorHandler(RecoverableErrorHandler),
- WarningHandler(WarningHandler), DObj(std::move(DObj)) {
- if (ThreadSafe)
- State = std::make_unique<ThreadSafeState>(*this, DWPName);
- else
- State = std::make_unique<ThreadUnsafeDWARFContextState>(*this, DWPName);
- }
+ bool ThreadSafe, OwningBinary<ObjectFile> GnuLink)
+ : DIContext(CK_DWARF), RecoverableErrorHandler(RecoverableErrorHandler),
+ WarningHandler(WarningHandler), DObj(std::move(DObj)),
+ GnuLink(std::move(GnuLink)) {
+ if (ThreadSafe)
+ State = std::make_unique<ThreadSafeState>(*this, DWPName);
+ else
+ State = std::make_unique<ThreadUnsafeDWARFContextState>(*this, DWPName);
+}
DWARFContext::~DWARFContext() = default;
@@ -2415,13 +2413,37 @@ DWARFContext::create(const object::ObjectFile &Obj,
std::function<void(Error)> RecoverableErrorHandler,
std::function<void(Error)> WarningHandler,
bool ThreadSafe) {
+ const object::ObjectFile *ObjPtr = &Obj;
+ OwningBinary<ObjectFile> GnuLinkObj;
+ for (const llvm::object::SectionRef &Section : Obj.sections()) {
+ Expected<StringRef> SectionNameOrErr = Section.getName();
+ if (SectionNameOrErr && *SectionNameOrErr == ".gnu_debuglink") {
+ Expected<StringRef> ContentsOrErr = Section.getContents();
+ if (ContentsOrErr) {
+ DataExtractor Data(*ContentsOrErr, Obj.isLittleEndian(),
+ Obj.getBytesInAddress());
+ uint64_t GnuDebuglinkOffset = 0;
+ const char *GnuLink = Data.getCStr(&GnuDebuglinkOffset, 0);
+ Expected<OwningBinary<ObjectFile>> GnuLinkObjTempOrErr =
+ object::ObjectFile::createObjectFile(GnuLink);
+ if (GnuLinkObjTempOrErr) {
+ GnuLinkObj = std::move(GnuLinkObjTempOrErr.get());
+ ObjPtr = GnuLinkObj.getBinary();
+ } else {
+ consumeError(GnuLinkObjTempOrErr.takeError());
+ }
+ } else {
+ consumeError(ContentsOrErr.takeError());
+ }
+ } else {
+ consumeError(SectionNameOrErr.takeError());
+ }
+ }
auto DObj = std::make_unique<DWARFObjInMemory>(
- Obj, L, RecoverableErrorHandler, WarningHandler, RelocAction);
- return std::make_unique<DWARFContext>(std::move(DObj),
- std::move(DWPName),
- RecoverableErrorHandler,
- WarningHandler,
- ThreadSafe);
+ *ObjPtr, L, RecoverableErrorHandler, WarningHandler, RelocAction);
+ return std::make_unique<DWARFContext>(std::move(DObj), std::move(DWPName),
+ RecoverableErrorHandler, WarningHandler,
+ ThreadSafe, std::move(GnuLinkObj));
}
std::unique_ptr<DWARFContext>
diff --git a/llvm/test/DebugInfo/gnu-debug-link-monolithic.ll b/llvm/test/DebugInfo/gnu-debug-link-monolithic.ll
new file mode 100644
index 00000000000000..09cab250c3ba38
--- /dev/null
+++ b/llvm/test/DebugInfo/gnu-debug-link-monolithic.ll
@@ -0,0 +1,101 @@
+
+; REQUIRES: x86_64-linux
+; RUN: rm -rf %t && mkdir %t
+; RUN: mkdir -p %t/gnuLink
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu %s -filetype=obj -o main.o
+; RUN: llvm-objcopy --only-keep-debug main.o main.o.debuginfo
+; RUN: llvm-objcopy --strip-debug --add-gnu-debuglink=main.o.debuginfo main.o
+; RUN: llvm-dwarfdump --debug-info -r 0 main.o | FileCheck --check-prefix=DWARFDUMP %s
+; RUN: llvm-gsymutil --convert main.o -o main.gsym | FileCheck --check-prefix=GSYM %s
+
+; DWARFDUMP: DW_TAG_compile_unit
+; GSYM: Loaded 2 functions from DWARF.
+
+;; Testing that llvm-dwarfdump and llvm-gsymutil work on a binary from which debug information
+;; was stripped after gnu_debug_link is created.
+;;clang++ -g2 -O0 main.cpp -c -emit-llvm -S
+;; int foo(int i) {
+;; return i + 1;
+;; }
+;;
+;; int main() {
+;; int j = 3;
+;; j = foo(j) + 1;
+;; return j;
+;; }
+
+
+; ModuleID = 'main.cpp'
+source_filename = "main.cpp"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: mustprogress noinline nounwind optnone uwtable
+define dso_local noundef i32 @_Z3fooi(i32 noundef %i) #0 !dbg !10 {
+entry:
+ %i.addr = alloca i32, align 4
+ store i32 %i, ptr %i.addr, align 4
+ call void @llvm.dbg.declare(metadata ptr %i.addr, metadata !15, metadata !DIExpression()), !dbg !16
+ %0 = load i32, ptr %i.addr, align 4, !dbg !17
+ %add = add nsw i32 %0, 1, !dbg !18
+ ret i32 %add, !dbg !19
+}
+
+; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+; Function Attrs: mustprogress noinline norecurse nounwind optnone uwtable
+define dso_local noundef i32 @main() #2 !dbg !20 {
+entry:
+ %retval = alloca i32, align 4
+ %j = alloca i32, align 4
+ store i32 0, ptr %retval, align 4
+ call void @llvm.dbg.declare(metadata ptr %j, metadata !23, metadata !DIExpression()), !dbg !24
+ store i32 3, ptr %j, align 4, !dbg !24
+ %0 = load i32, ptr %j, align 4, !dbg !25
+ %call = call noundef i32 @_Z3fooi(i32 noundef %0), !dbg !26
+ %add = add nsw i32 %call, 1, !dbg !27
+ store i32 %add, ptr %j, align 4, !dbg !28
+ %1 = load i32, ptr %j, align 4, !dbg !29
+ ret i32 %1, !dbg !30
+}
+
+attributes #0 = { mustprogress noinline nounwind optnone uwtable "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+attributes #1 = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }
+attributes #2 = { mustprogress noinline norecurse nounwind optnone uwtable "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6, !7, !8}
+!llvm.ident = !{!9}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 18.0.0git", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "main.cpp", directory: "/gnuLink", checksumkind: CSK_MD5, checksum: "cc30cb527607311c4a8449e92acdf1c5")
+!2 = !{i32 7, !"Dwarf Version", i32 5}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !{i32 8, !"PIC Level", i32 2}
+!6 = !{i32 7, !"PIE Level", i32 2}
+!7 = !{i32 7, !"uwtable", i32 2}
+!8 = !{i32 7, !"frame-pointer", i32 2}
+!9 = !{!"clang version 18.0.0git"}
+!10 = distinct !DISubprogram(name: "foo", linkageName: "_Z3fooi", scope: !1, file: !1, line: 1, type: !11, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !14)
+!11 = !DISubroutineType(types: !12)
+!12 = !{!13, !13}
+!13 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!14 = !{}
+!15 = !DILocalVariable(name: "i", arg: 1, scope: !10, file: !1, line: 1, type: !13)
+!16 = !DILocation(line: 1, column: 13, scope: !10)
+!17 = !DILocation(line: 2, column: 10, scope: !10)
+!18 = !DILocation(line: 2, column: 12, scope: !10)
+!19 = !DILocation(line: 2, column: 3, scope: !10)
+!20 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 5, type: !21, scopeLine: 5, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !14)
+!21 = !DISubroutineType(types: !22)
+!22 = !{!13}
+!23 = !DILocalVariable(name: "j", scope: !20, file: !1, line: 6, type: !13)
+!24 = !DILocation(line: 6, column: 7, scope: !20)
+!25 = !DILocation(line: 7, column: 11, scope: !20)
+!26 = !DILocation(line: 7, column: 7, scope: !20)
+!27 = !DILocation(line: 7, column: 14, scope: !20)
+!28 = !DILocation(line: 7, column: 5, scope: !20)
+!29 = !DILocation(line: 8, column: 10, scope: !20)
+!30 = !DILocation(line: 8, column: 3, scope: !20)
|
MaskRay
changed the title
| @@ -127,6 +127,9 @@ class DWARFContext : public DIContext { | |||
|
|
|||
| std::unique_ptr<const DWARFObject> DObj; | |||
|
|
|||
| /// Binary containing debug information after objcopy --only-keep-debug | |||
| object::OwningBinary<object::ObjectFile> GnuLink; | |||
There was a problem hiding this comment.
Could we avoid having this sort of invasive change by switching entirely to the debuglink binary early on & then the rest of the codepaths would work as they do today? (perhaps look at where the debuginfod is wired in - I would tihnk debuglink might work in a similar way?)
There was a problem hiding this comment.
I was aiming to do that here. This is the first entrance path for multiple tools that create DWARFContext. Otherwise this will need to be handled on caller side at multiple points. Unless I am missing something.
Let me take a look into debuginfod. I didn't think outside of lldb it was supported in various tools.
There was a problem hiding this comment.
Could we avoid having this sort of invasive change by switching entirely to the debuglink binary early on & then the rest of the codepaths would work as they do today? (perhaps look at where the debuginfod is wired in - I would tihnk debuglink might work in a similar way?)
OK poking around llvm source code looks like DebugInfoD is used on adhoc bases in various tools.
DebuginfodFetcher -- used in llvm-objdump, llvm-symbolizer, llvm-cov
Looking into APIs declared in Debuginfod.h used in LLDB, llvm-debuginfod-find
So doesn't look like there is a central place where it is "automatically" supported, and it's up to each tool to add support for it.
There was a problem hiding this comment.
@dwblaikie I realize in ::create is not super ideal. I was originally thinking about doing it in the constructor, but then if people go through ::create then DWARFObjInMemory is created for no reason.
Generally speaking I think this should be handled similar to .dwp where it's under the hood and consumers of debug information shouldn't really care/know if debug info is in main binary or in a separate elf object.
There was a problem hiding this comment.
Debuginfod (the "protocol") is actually quite underspecified for this scenario (I'm in the middle of adding full support for split, stripped, and "only-keep-debug" symbol acquisition through libdebuginfod) You can't replace the original binary with the .gnu.debuglink binary, because there's a high probability that the linked binary is missing a bunch of stuff from the stripped binary.
From what I've gathered they appear to be used as the "original split dwarf" because a common use is to make the .gnu.debuglink point to the an "objcopy --only-keep-debug" version of the binary, so it doesn't have anything except the DWARF data (there's almost no overlap between the --strip-debug and --only-keep-debug versions of the binary, and I think you need both do actually debug stuff...)
There was a problem hiding this comment.
As far as llvm's DWARF library is concerned, I /think/ we can replace the binary with the debuglink binary, though (even if it is only-keep-debug) - and that is what this patch is doing. Presumably that's what debuginfod support does too? (well, one way or another - as you say, perhaps underspecified for all these different flavors of split/partial binaries, etc - for now, I guess debuginfod support is retrieving the whole unstripped binary, so it's strictly better/more complete than the case being supported here with debuglink - but that points to, even more, that they could use the same path in this case (because the path is good enough for debuglink to only-keep-debug, so it's certainly fine to use that path for the more complete whole-unstripped-binary case for debuginfod))
There was a problem hiding this comment.
I feel like I'm missing something. As I understand it, replacing the stripped binary with an OKD binary would remove sections that are necessary for full symbolication, general debugging, and proper program execution (.symtab, .strtab, .shstrtab in my simple tests). If we replaced the stripped binary, how would we deal with those sections?
There was a problem hiding this comment.
I feel like I'm missing something. As I understand it, replacing the stripped binary with an OKD binary would remove sections that are necessary for full symbolication, general debugging, and proper program execution (.symtab, .strtab, .shstrtab in my simple tests). If we replaced the stripped binary, how would we deal with those sections?
.symtab is actually copied
That being said the .eh_frame is not. :/
There was a problem hiding this comment.
Looking more closely through DWARFContext.cpp except for dump in DWARFContext getting DwarfObject is controlled through getDWARFObj().
One alternative is in DWARFContext to have both gnulink one, and main binary one. Depending if gnulink is present in it return either it or main binary DWARFObject. With a default parameter in getDWARFObj that can override behavior for apis that need it. Like when eh_frame is used.
This way code can also be moved into constructor.
There was a problem hiding this comment.
Presumably that's what debuginfod support does too? (well, one way or another - as you say, perhaps underspecified for all these different flavors of split/partial binaries, etc - for now, I guess debuginfod support is retrieving the whole unstripped binary, so it's strictly better/more complete than the case being supported here with debuglink - but that points to, even more, that they could use the same path in this case (because the path is good enough for debuglink to only-keep-debug, so it's certainly fine to use that path for the more complete whole-unstripped-binary case for debuginfod))
What I ended up doing for debuginfod is kind-of situational; generally the tools that can invoke debuginfod look for debug info in the binaries that are directly available to them, but failing that, they retrieve the debug binary and then use it just for its DWARF. For tools that only operate on DWARF, the fetched binary can replace the original, but for other tools, the DWARF that the tool analyzes may be loaded from a different binary than the one the rest of the tool operates on.
Last time I checked (and if I understand their code correctly), the GNU implementation handles this in a more centralized way; both debuglink sections and debuginfod lookups can provide additional auxiliary sections that get tacked on to a binary. (Unfortunately I don't remember the precise semantics of this.) Something that can merge sections like this would be the best of both worlds, since it would produce an uber-image that could be passed around trivially. But I didn't see a straightforward way to do this in LLVM.
EDIT: It's also desirable for debuginfod at least that a fully stripped binary be usable w/ it; more than just DWARF might be loaded... section descriptions, symbols, etc etc. Basically anything not present in the most stripped version of the binaries. Do we have a similar conern for .gnu_debuglink? It would really depend on what supported ways there are to generate such a pair of files. For debuginfod the relationship between them is arbitrary, since their only link is that they share a build ID.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
@kevinfrei since you just posted some debuginfod work (#78605) I'm tagging you in here (though perhaps @petrhosek has some thoughts/other folks that'd want to be involved) Seems like between debuginfod and gnu_debuglink - those are at least two ways to lookup the original binary with debug info given a stripped binary, and we should have some unified way/place that attempts these various resolution options. Ah, I think I see why you're adding the GnuLink member (now that I see it has no usage, it's only a lifetime issue) - because usually the lifetime of the object is managed by the caller of DWARFContext::create, but because create is opening a new object it has to preserve the object's lifetime longer. I think I'd still rather see that not affect DWARFContext's members - for instance, DWARFContext::create could take a non-const reference to Or it could be a separate function - possibly exposed by DWARFContext (but I know you want something to be a drop-in replacement for all DWARFContext usage & this wouldn't provide that) that takes the OwningBinary/ObjectFile and remaps it, if necessary, to the linked debug info (gnu_debuglink, debuginfod, or whatever else). |
Debuginfod is a service to acquire symbols/source from a repository, while the .gnu.debuglink is a way to connect a stripped binary file with it's corresponding debug info (with a different CRC for verification :/ ). Debuginfod uses only the .note.gnu.build-id to connect files. They're not completely separate, but I don't think making a "debuglink" SymbolLocator plugin makes any sort of sense, while it's a very rational design for interacting with Debuginfod. I am, however, fairly new to the space (my background is compilers) so I'm definitely open to learning what I'm missing... |
Not sure I understand... - for arguments sake, lets say debuglink only lets you find a only-keep-debug binary (say, using this as a model: https://help.totalview.io/previous_releases/2019/HTML/index.html#page/Reference_Guide/Usinggnu_debuglinkFiles.html ). So debuglink lets you find the only-keep-debug binary. Debuginfod lets you find the unstripped binary, yeah? (among other things) So, for the purposes of this library (libDebugInfoDWARF) those two things are equivalent - these tools only look at the debug sections. So from this perspective, these actions (debuginfod finding/retrieving an unstripped binary, and debuglink finding a only-keep-debug binary) are equivalent - two different ways of achieving the same end, getting the debug info sections. I think? Perhaps I'm misunderstanding something of these two tools. |
I think it's a difference in functionality/complexity. The level of complexity to create it is also lower. It's two objcopy commands vs setting up a server, etc. |
|
A direct alternative to In llvm-project/llvm/lib/DebugInfo/Symbolize/Symbolize.cpp Lines 520 to 523 in e611a4c I think we should factor the debug link implementation out of the symbolizer into a reusable library, similarly to what @mysterymath has done for the build ID implementation in 9812948, so it can be reused both inside the symbolizer and In |
I added a comment to the discussion earlier about the semantics of |
Let me take a look at lookUpDebuglinkObject, after some internal stuff. |
Added support to DWARFLibrary for .gnu_debuglink. This support is already in
LLDB. With this all the tools (llvm-dwarfdump, llvm-gsymutil, etc) and projects
like BOLT that rely on the DWARFLibrary can benefit.