[lld] Sort data chunks before code chunks on ARM64EC. #70722
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-lld @llvm/pr-subscribers-platform-windows Author: Jacek Caban (cjacek) ChangesDepends on !70721. I found this MSVC behavior while investigating !69100. This will allow simplifying that PR. Full diff: https://github.com/llvm/llvm-project/pull/70722.diff 5 Files Affected:
diff --git a/lld/COFF/Chunks.h b/lld/COFF/Chunks.h
index cbfeb5c025adbb2..156e7a807cb8fd7 100644
--- a/lld/COFF/Chunks.h
+++ b/lld/COFF/Chunks.h
@@ -116,7 +116,7 @@ class Chunk {
bool isHotPatchable() const;
MachineTypes getMachine() const;
- chpe_range_type getArm64ECRangeType() const;
+ std::optional<chpe_range_type> getArm64ECRangeType() const;
protected:
Chunk(Kind k = OtherKind) : chunkKind(k), hasData(true), p2Align(0) {}
@@ -180,6 +180,16 @@ class NonSectionChunk : public Chunk {
NonSectionChunk(Kind k = OtherKind) : Chunk(k) {}
};
+class NonSectionCodeChunk : public NonSectionChunk {
+public:
+ virtual uint32_t getOutputCharacteristics() const override {
+ return llvm::COFF::IMAGE_SCN_MEM_READ | llvm::COFF::IMAGE_SCN_MEM_EXECUTE;
+ }
+
+protected:
+ NonSectionCodeChunk(Kind k = OtherKind) : NonSectionChunk(k) {}
+};
+
// MinGW specific; information about one individual location in the image
// that needs to be fixed up at runtime after loading. This represents
// one individual element in the PseudoRelocTableChunk table.
@@ -427,7 +437,11 @@ inline MachineTypes Chunk::getMachine() const {
return static_cast<const NonSectionChunk *>(this)->getMachine();
}
-inline chpe_range_type Chunk::getArm64ECRangeType() const {
+inline std::optional<chpe_range_type> Chunk::getArm64ECRangeType() const {
+ // Data sections don't need codemap entries.
+ if (!(getOutputCharacteristics() & llvm::COFF::IMAGE_SCN_MEM_EXECUTE))
+ return std::nullopt;
+
switch (getMachine()) {
case AMD64:
return chpe_range_type::Amd64;
@@ -508,10 +522,10 @@ static const uint8_t importThunkARM64[] = {
// Windows-specific.
// A chunk for DLL import jump table entry. In a final output, its
// contents will be a JMP instruction to some __imp_ symbol.
-class ImportThunkChunk : public NonSectionChunk {
+class ImportThunkChunk : public NonSectionCodeChunk {
public:
ImportThunkChunk(COFFLinkerContext &ctx, Defined *s)
- : NonSectionChunk(ImportThunkKind), impSymbol(s), ctx(ctx) {}
+ : NonSectionCodeChunk(ImportThunkKind), impSymbol(s), ctx(ctx) {}
static bool classof(const Chunk *c) { return c->kind() == ImportThunkKind; }
protected:
@@ -560,7 +574,7 @@ class ImportThunkChunkARM64 : public ImportThunkChunk {
MachineTypes getMachine() const override { return ARM64; }
};
-class RangeExtensionThunkARM : public NonSectionChunk {
+class RangeExtensionThunkARM : public NonSectionCodeChunk {
public:
explicit RangeExtensionThunkARM(COFFLinkerContext &ctx, Defined *t)
: target(t), ctx(ctx) {
@@ -576,7 +590,7 @@ class RangeExtensionThunkARM : public NonSectionChunk {
COFFLinkerContext &ctx;
};
-class RangeExtensionThunkARM64 : public NonSectionChunk {
+class RangeExtensionThunkARM64 : public NonSectionCodeChunk {
public:
explicit RangeExtensionThunkARM64(COFFLinkerContext &ctx, Defined *t)
: target(t), ctx(ctx) {
diff --git a/lld/COFF/DLL.cpp b/lld/COFF/DLL.cpp
index 0b337a209c377db..6b516d8c6d5ef89 100644
--- a/lld/COFF/DLL.cpp
+++ b/lld/COFF/DLL.cpp
@@ -313,7 +313,7 @@ static const uint8_t tailMergeARM64[] = {
};
// A chunk for the delay import thunk.
-class ThunkChunkX64 : public NonSectionChunk {
+class ThunkChunkX64 : public NonSectionCodeChunk {
public:
ThunkChunkX64(Defined *i, Chunk *tm) : imp(i), tailMerge(tm) {}
@@ -330,7 +330,7 @@ class ThunkChunkX64 : public NonSectionChunk {
Chunk *tailMerge = nullptr;
};
-class TailMergeChunkX64 : public NonSectionChunk {
+class TailMergeChunkX64 : public NonSectionCodeChunk {
public:
TailMergeChunkX64(Chunk *d, Defined *h) : desc(d), helper(h) {}
@@ -382,7 +382,7 @@ class TailMergeUnwindInfoX64 : public NonSectionChunk {
}
};
-class ThunkChunkX86 : public NonSectionChunk {
+class ThunkChunkX86 : public NonSectionCodeChunk {
public:
ThunkChunkX86(COFFLinkerContext &ctx, Defined *i, Chunk *tm)
: imp(i), tailMerge(tm), ctx(ctx) {}
@@ -407,7 +407,7 @@ class ThunkChunkX86 : public NonSectionChunk {
const COFFLinkerContext &ctx;
};
-class TailMergeChunkX86 : public NonSectionChunk {
+class TailMergeChunkX86 : public NonSectionCodeChunk {
public:
TailMergeChunkX86(COFFLinkerContext &ctx, Chunk *d, Defined *h)
: desc(d), helper(h), ctx(ctx) {}
@@ -432,7 +432,7 @@ class TailMergeChunkX86 : public NonSectionChunk {
const COFFLinkerContext &ctx;
};
-class ThunkChunkARM : public NonSectionChunk {
+class ThunkChunkARM : public NonSectionCodeChunk {
public:
ThunkChunkARM(COFFLinkerContext &ctx, Defined *i, Chunk *tm)
: imp(i), tailMerge(tm), ctx(ctx) {
@@ -459,7 +459,7 @@ class ThunkChunkARM : public NonSectionChunk {
const COFFLinkerContext &ctx;
};
-class TailMergeChunkARM : public NonSectionChunk {
+class TailMergeChunkARM : public NonSectionCodeChunk {
public:
TailMergeChunkARM(COFFLinkerContext &ctx, Chunk *d, Defined *h)
: desc(d), helper(h), ctx(ctx) {
@@ -486,7 +486,7 @@ class TailMergeChunkARM : public NonSectionChunk {
const COFFLinkerContext &ctx;
};
-class ThunkChunkARM64 : public NonSectionChunk {
+class ThunkChunkARM64 : public NonSectionCodeChunk {
public:
ThunkChunkARM64(Defined *i, Chunk *tm) : imp(i), tailMerge(tm) {
setAlignment(4);
@@ -506,7 +506,7 @@ class ThunkChunkARM64 : public NonSectionChunk {
Chunk *tailMerge = nullptr;
};
-class TailMergeChunkARM64 : public NonSectionChunk {
+class TailMergeChunkARM64 : public NonSectionCodeChunk {
public:
TailMergeChunkARM64(Chunk *d, Defined *h) : desc(d), helper(h) {
setAlignment(4);
diff --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp
index 43d8e7c1d530859..895a6e00710c634 100644
--- a/lld/COFF/Writer.cpp
+++ b/lld/COFF/Writer.cpp
@@ -1389,7 +1389,9 @@ void Writer::sortECChunks() {
for (OutputSection *sec : ctx.outputSections) {
if (sec->isCodeSection())
llvm::stable_sort(sec->chunks, [=](const Chunk *a, const Chunk *b) {
- return a->getArm64ECRangeType() < b->getArm64ECRangeType();
+ std::optional<chpe_range_type> aType = a->getArm64ECRangeType(),
+ bType = b->getArm64ECRangeType();
+ return !aType || (bType && *aType < *bType);
});
}
}
diff --git a/lld/test/COFF/arm64ec-codemap.test b/lld/test/COFF/arm64ec-codemap.test
index 424456a6dee66f0..018810a86a34f22 100644
--- a/lld/test/COFF/arm64ec-codemap.test
+++ b/lld/test/COFF/arm64ec-codemap.test
@@ -110,6 +110,40 @@ DISASMM-NEXT: ...
DISASMM-NEXT: 180002ffe: 00 00 addb %al, (%rax)
DISASMM-NEXT: 180003000: b8 06 00 00 00 movl $0x6, %eax
+RUN: lld-link -out:testdm.dll -machine:arm64ec arm64ec-func-sym.obj x86_64-func-sym.obj codemap.obj \
+RUN: data-sec.obj loadconfig-arm64ec.obj -dll -noentry -merge:.testdata=.text -merge:.rdata=test
+
+RUN: llvm-readobj --coff-load-config testdm.dll | FileCheck -check-prefix=CODEMAPDM %s
+CODEMAPDM: CodeMap [
+CODEMAPDM-NEXT: 0x2000 - 0x2008 ARM64EC
+CODEMAPDM-NEXT: 0x3000 - 0x3006 X64
+CODEMAPDM-NEXT: 0x5200 - 0x5208 ARM64EC
+CODEMAPDM-NEXT: 0x6000 - 0x6006 X64
+CODEMAPDM-NEXT: ]
+
+Merging code data into code sections causes data to be separated from the code when sorting chunks.
+
+RUN: llvm-objdump -d testdm.dll | FileCheck -check-prefix=DISASMDM %s
+DISASMDM: Disassembly of section .text:
+DISASMDM-EMPTY:
+DISASMDM-NEXT: 0000000180001000 <.text>:
+DISASMDM-NEXT: 180001000: 00000001 udf #0x1
+DISASMDM-NEXT: ...
+DISASMDM-NEXT: 180002000: 52800040 mov w0, #0x2
+DISASMDM-NEXT: 180002004: d65f03c0 ret
+DISASMDM-NEXT: ...
+DISASMDM-NEXT: 180003000: b8 03 00 00 00 movl $0x3, %eax
+DISASMDM-NEXT: 180003005: c3 retq
+DISASMDM-EMPTY:
+DISASMDM-NEXT: Disassembly of section test:
+DISASMDM-EMPTY:
+DISASMDM-NEXT: 0000000180005000 <test>:
+DISASMDM: 180005200: 528000a0 mov w0, #0x5
+DISASMDM-NEXT: 180005204: d65f03c0 ret
+DISASMDM-NEXT: ...
+DISASMDM-NEXT: 180006000: b8 06 00 00 00 movl $0x6, %eax
+DISASMDM-NEXT: 180006005: c3 retq
+
#--- arm64-func-sym.s
.text
.globl arm64_func_sym
@@ -148,6 +182,10 @@ x86_64_func_sym2:
movl $6, %eax
retq
+#--- data-sec.s
+ .section .testdata, "rd"
+ .xword 1
+
#--- codemap.s
.section .rdata,"dr"
.globl code_map
diff --git a/lld/test/COFF/export-thunk.test b/lld/test/COFF/export-thunk.test
new file mode 100644
index 000000000000000..85e0e92bc0639b7
--- /dev/null
+++ b/lld/test/COFF/export-thunk.test
@@ -0,0 +1,14 @@
+REQUIRES: x86
+
+RUN: echo -e 'LIBRARY test.dll\nEXPORTS\nimpfunc\n' > %t.imp.def
+RUN: llvm-dlltool -m i386:x86-64 -d %t.imp.def -l %t.imp.lib
+RUN: lld-link -machine:amd64 -out:%t.dll -dll -noentry -lldmingw %t.imp.lib -export:impfunc -output-def:%t.def
+
+Check that the synthetic import thunk is exported as a function, not data.
+
+RUN: cat %t.def | FileCheck %s
+CHECK: EXPORTS
+CHECK-NEXT: impfunc @1
+
+RUN: cat %t.def | FileCheck -check-prefix=CHECK-NO-DATA %s
+CHECK-NO-DATA-NOT: DATA
|
cjacek
requested review from
rnk,
compnerd,
mstorsjo,
MaskRay,
amykhuang and
efriedma-quic
mstorsjo
approved these changes
Oct 31, 2023
lld/test/COFF/arm64ec-codemap.test
Outdated
| CODEMAPDM-NEXT: 0x6000 - 0x6006 X64 | ||
| CODEMAPDM-NEXT: ] | ||
|
|
||
| Merging code data into code sections causes data to be separated from the code when sorting chunks. |
There was a problem hiding this comment.
What's "code data" here, is there a typo in this sentence?
There was a problem hiding this comment.
It's indeed a typo, I meant "data section". I pushed a fixup, thanks for reviews!
cjacek
force-pushed
the
arm64ec-data-sort
branch
from
2b65745 to
5c0ecd4
Compare
|
Looks like this broke check-lld: http://45.33.8.238/linux/122206/step_11.txt Please take a look and revert for now if it takes a while to fix. |
Sorry about that. I pushed a fixup as 12e39a322fe7757a3794e7425ee265f219d6dd2e. I missed llvm-mc invocation when I was moving the test between branches and I didn't catch it because I had a stray object file in the tree. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends on !70721.
I found this MSVC behavior while investigating !69100. This will allow simplifying that PR.