[LLD][COFF] Add /nodbgdirmerge to control debug directory section #159235
Conversation
|
@llvm/pr-subscribers-platform-windows @llvm/pr-subscribers-lld-coff Author: None (kkent030315) ChangesResolves #141712. As described in the issue, this PR adds support for Full diff: https://github.com/llvm/llvm-project/pull/159235.diff 5 Files Affected:
diff --git a/lld/COFF/Config.h b/lld/COFF/Config.h
index a71476adcc493..76b7c1f61cd25 100644
--- a/lld/COFF/Config.h
+++ b/lld/COFF/Config.h
@@ -322,6 +322,7 @@ struct Configuration {
bool largeAddressAware = false;
bool highEntropyVA = false;
bool appContainer = false;
+ bool mergeDebugDirectory = true;
bool mingw = false;
bool warnMissingOrderSymbol = true;
bool warnLocallyDefinedImported = true;
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index acba156ce341d..4ed587bb7327d 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -2338,6 +2338,9 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
config->is64() &&
args.hasFlag(OPT_highentropyva, OPT_highentropyva_no, true);
+ // Handle /nodbgdirmerge
+ config->mergeDebugDirectory = args.hasArg(OPT_nodbgdirmerge);
+
if (!config->dynamicBase &&
(config->machine == ARMNT || isAnyArm64(config->machine)))
Err(ctx) << "/dynamicbase:no is not compatible with "
diff --git a/lld/COFF/Options.td b/lld/COFF/Options.td
index b5334de87b6a7..485db5a8b21c1 100644
--- a/lld/COFF/Options.td
+++ b/lld/COFF/Options.td
@@ -90,6 +90,8 @@ def machine : P<"machine", "Specify target platform">;
def merge : P<"merge", "Combine sections">;
def mllvm : P<"mllvm", "Options to pass to LLVM">;
def nodefaultlib : P<"nodefaultlib", "Remove a default library">;
+def nodbgdirmerge : F<"nodbgdirmerge">,
+ HelpText<"Emit the debug directory in a separate section">;
def opt : P<"opt", "Control optimizations">;
def order : P<"order", "Put functions in order">;
def out : P<"out", "Path to file to write output">;
diff --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp
index e0d0ac18ea5d5..274c6e613769e 100644
--- a/lld/COFF/Writer.cpp
+++ b/lld/COFF/Writer.cpp
@@ -324,6 +324,7 @@ class Writer {
OutputSection *bssSec;
OutputSection *rdataSec;
OutputSection *buildidSec;
+ OutputSection *cvinfoSec;
OutputSection *dataSec;
OutputSection *pdataSec;
OutputSection *idataSec;
@@ -1092,6 +1093,7 @@ void Writer::createSections() {
bssSec = createSection(".bss", bss | r | w);
rdataSec = createSection(".rdata", data | r);
buildidSec = createSection(".buildid", data | r);
+ cvinfoSec = createSection(".cvinfo", data | r);
dataSec = createSection(".data", data | r | w);
pdataSec = createSection(".pdata", data | r);
idataSec = createSection(".idata", data | r);
@@ -1228,7 +1230,13 @@ void Writer::createMiscChunks() {
});
// Create Debug Information Chunks
- debugInfoSec = config->mingw ? buildidSec : rdataSec;
+ if (config->mingw) {
+ debugInfoSec = buildidSec;
+ } else if (config->mergeDebugDirectory) {
+ debugInfoSec = cvinfoSec;
+ } else {
+ debugInfoSec = rdataSec;
+ }
if (config->buildIDHash != BuildIDHash::None || config->debug ||
config->repro || config->cetCompat || config->cetCompatStrict ||
config->cetCompatIpValidationRelaxed ||
diff --git a/lld/test/COFF/nodbgdirmerge.test b/lld/test/COFF/nodbgdirmerge.test
new file mode 100644
index 0000000000000..f36c8dfed3e84
--- /dev/null
+++ b/lld/test/COFF/nodbgdirmerge.test
@@ -0,0 +1,28 @@
+RUN: yaml2obj %p/Inputs/pdb1.yaml -o %t1.obj
+RUN: yaml2obj %p/Inputs/pdb2.yaml -o %t2.obj
+RUN: rm -f %t.dll %t.pdb
+
+## Check that it emits the debug directory in .build section when
+## /nodbgdirmerge is specified
+RUN: lld-link /debug /pdb:%t.pdb /pdbaltpath:test.pdb /dll /out:%t.dll \
+RUN: /entry:main /nodefaultlib /nodbgdirmerge %t1.obj %t2.obj
+RUN: llvm-readobj --sections %t.dll | FileCheck -check-prefix=CHECKNOTMERGED %s
+
+CHECKNOTMERGED: Section {
+CHECKNOTMERGED: Number: 3
+CHECKNOTMERGED: Name: .cvinfo
+CHECKNOTMERGED: Characteristics [ (0x40000040)
+CHECKNOTMERGED: IMAGE_SCN_CNT_INITIALIZED_DATA (0x40)
+CHECKNOTMERGED: IMAGE_SCN_MEM_READ (0x40000000)
+CHECKNOTMERGED: ]
+CHECKNOTMERGED: }
+
+## Check that it triggers merge on when /nodbgdirmerge is not specified
+RUN: lld-link /debug /pdb:%t.pdb /pdbaltpath:test.pdb /dll /out:%t.dll \
+RUN: /entry:main /nodefaultlib %t1.obj %t2.obj
+RUN: llvm-readobj --sections %t.dll | FileCheck -check-prefix=CHECKMERGED %s
+
+CHECKMERGED: Section {
+CHECKMERGED: Number: 3
+CHECKMERGED-NOT: Name: .cvinfo
+CHECKMERGED: }
|
kkent030315
force-pushed
the
nodbgdirmerge
branch
from
ee8fa56 to
080d7fe
Compare
Co-Authored-By: namazso <[email protected]>
|
Why ignore it on MinGW? |
See the code change in Writer.cpp - mingw always puts this in a separate |
| CHECKNOTMERGED-NEXT: PointerToRelocations: 0 | ||
| CHECKNOTMERGED-NEXT: PointerToLineNumbers: 0 | ||
| CHECKNOTMERGED-NEXT: RelocationCount: 0 | ||
| CHECKNOTMERGED-NEXT: LineNumberCount: 0 |
There was a problem hiding this comment.
This is fine as is - but as a general comment; it's also possible to make the test less prone to breaking on unrelated changes, by entirely omitting the values on the lines we're not interested in - it still matches the line so we can use -NEXT to keep things connected, but still matching those lines fuzzily.
Another potential approach is to skip -NEXT for the section we're not that interested in, then have e.g. CHECKNOTMERGED: LineNumberCount: CHECKNOTMERGED-NEXT: Characteristics [ (0x40000040), so the check for LineNumberCount: certainly will match the next such line, and force the full exact match on Characteristics to still be the next one (i.e. in the same section).
There was a problem hiding this comment.
We just need to check the presense of .cvinfo section and that's all, perhaps it should suffice we just have single line CHECKNOTMERGED: Name: .cvinfo as well as the negate test?
There was a problem hiding this comment.
Yes, that would probably also suffice. I guess technically we maybe also could want to check the contents of the section (not only that the section exists but also contains roughly the expected contents), but you're right that the rest of these checks here are kinda redundant.
4 participants
Resolves #141712.
As described in the issue, this PR adds support for
/nodbgdirmergeflag in LLD to align with MS link. When the flag is specified, the linker will emit the debug directory section in.cvinfosection, instead of merging it to the.rdata. The flag will be ignored on MinGW.