[lld] Implement getOutputCharacteristics for non-section code thunks.

[cjacek]

This will be useful for ARM64EC, but it also fixes MinGW export handling when synthetic function symbols are exported.

[llvmbot]

@llvm/pr-subscribers-lld
@llvm/pr-subscribers-platform-windows

@llvm/pr-subscribers-lld-coff

Author: Jacek Caban (cjacek)

Changes

This will be useful for ARM64EC, but it also fixes MinGW export handling when synthetic function symbols are exported.

---

Full diff: https://github.com/llvm/llvm-project/pull/70721.diff

3 Files Affected:

• (modified) lld/COFF/Chunks.h (+14-4)
• (modified) lld/COFF/DLL.cpp (+8-8)
• (added) lld/test/COFF/export-thunk.test (+14)

diff --git a/lld/COFF/Chunks.h b/lld/COFF/Chunks.h
index cbfeb5c025adbb2..b82bc9416775d7c 100644
--- a/lld/COFF/Chunks.h
+++ b/lld/COFF/Chunks.h
@@ -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.
@@ -508,10 +518,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 +570,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 +586,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/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

[mstorsjo]

LGTM, thanks!

The case of reexporting the thunk of an imported function is kinda contrieved though - I presume that only would happen in a contrieved setup when manually requesting it to be exported? The autoexporter doesn't export them, right?

[cjacek]

Thanks for review!

The case of reexporting the thunk of an imported function is kinda contrieved though - I presume that only would happen in a contrieved setup when manually requesting it to be exported? The autoexporter doesn't export them, right?

Yes, autoexporter doesn't export them because it exports only DefinedRegular and DefinedCommon, so DefinedImportThunk is skipped.

It's indeed not a common use case and I made up the test just because I noticed that this code path will change. But it's also not entirely contrieved, I know a few such examples in Wine code base. One of them would be kernel32.dll, which forwards most of functions to kernelbase.dll, but doesn't use forwarding exports because some applications expect actual entry points. On non-x86 targets Wine exports import thunks instead. (Wine generates .edata section itself, so this commit is not relevant to it, but the mechanism is the same).

[mstorsjo]

Thanks for review!

The case of reexporting the thunk of an imported function is kinda contrieved though - I presume that only would happen in a contrieved setup when manually requesting it to be exported? The autoexporter doesn't export them, right?

Yes, autoexporter doesn't export them because it exports only DefinedRegular and DefinedCommon, so DefinedImportThunk is skipped.

It's indeed not a common use case and I made up the test just because I noticed that this code path will change. But it's also not entirely contrieved, I know a few such examples in Wine code base. One of them would be kernel32.dll, which forwards most of functions to kernelbase.dll, but doesn't use forwarding exports because some applications expect actual entry points. On non-x86 targets Wine exports import thunks instead. (Wine generates .edata section itself, so this commit is not relevant to it, but the mechanism is the same).

Ok, great - thanks for the clarification!