[LLD][COFF] Support CF guards on ARM64X

[cjacek]

Both native and EC views share table chunks. Ensure relevant symbols are set in both symbol tables.

[llvmbot]

@llvm/pr-subscribers-platform-windows

@llvm/pr-subscribers-lld-coff

Author: Jacek Caban (cjacek)

Changes

Both native and EC views share table chunks. Ensure relevant symbols are set in both symbol tables.

---

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

2 Files Affected:

• (modified) lld/COFF/Writer.cpp (+23-10)
• (added) lld/test/COFF/arm64x-guardcf.s (+122)

diff --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp
index 58727c1615769..c746ea8e1d7b8 100644
--- a/lld/COFF/Writer.cpp
+++ b/lld/COFF/Writer.cpp
@@ -1995,10 +1995,17 @@ static void maybeAddAddressTakenFunction(SymbolRVASet &addressTakenSyms,
     // Common is always data, so it is ignored.
     break;
   case Symbol::DefinedAbsoluteKind:
-  case Symbol::DefinedSyntheticKind:
     // Absolute is never code, synthetic generally isn't and usually isn't
     // determinable.
     break;
+  case Symbol::DefinedSyntheticKind:
+    // For EC export thunks, mark both the thunk itself and its target.
+    if (auto expChunk = dyn_cast_or_null<ECExportThunkChunk>(
+            cast<Defined>(s)->getChunk())) {
+      addSymbolToRVASet(addressTakenSyms, cast<Defined>(s));
+      addSymbolToRVASet(addressTakenSyms, expChunk->target);
+    }
+    break;
   case Symbol::LazyArchiveKind:
   case Symbol::LazyObjectKind:
   case Symbol::LazyDLLSymbolKind:
@@ -2063,9 +2070,11 @@ void Writer::createGuardCFTables() {
     // with /guard:cf.
     for (ObjFile *file : ctx.objFileInstances) {
       if (file->hasGuardCF()) {
-        Symbol *flagSym = ctx.symtab.findUnderscore("__guard_flags");
-        cast<DefinedAbsolute>(flagSym)->setVA(
-            uint32_t(GuardFlags::CF_INSTRUMENTED));
+        ctx.forEachSymtab([&](SymbolTable &symtab) {
+          Symbol *flagSym = symtab.findUnderscore("__guard_flags");
+          cast<DefinedAbsolute>(flagSym)->setVA(
+              uint32_t(GuardFlags::CF_INSTRUMENTED));
+        });
         break;
       }
     }
@@ -2147,8 +2156,10 @@ void Writer::createGuardCFTables() {
     guardFlags |= uint32_t(GuardFlags::CF_LONGJUMP_TABLE_PRESENT);
   if (config->guardCF & GuardCFLevel::EHCont)
     guardFlags |= uint32_t(GuardFlags::EH_CONTINUATION_TABLE_PRESENT);
-  Symbol *flagSym = ctx.symtab.findUnderscore("__guard_flags");
-  cast<DefinedAbsolute>(flagSym)->setVA(guardFlags);
+  ctx.forEachSymtab([guardFlags](SymbolTable &symtab) {
+    Symbol *flagSym = symtab.findUnderscore("__guard_flags");
+    cast<DefinedAbsolute>(flagSym)->setVA(guardFlags);
+  });
 }
 
 // Take a list of input sections containing symbol table indices and add those
@@ -2219,10 +2230,12 @@ void Writer::maybeAddRVATable(SymbolRVASet tableSymbols, StringRef tableSym,
     tableChunk = make<RVATableChunk>(std::move(tableSymbols));
   rdataSec->addChunk(tableChunk);
 
-  Symbol *t = ctx.symtab.findUnderscore(tableSym);
-  Symbol *c = ctx.symtab.findUnderscore(countSym);
-  replaceSymbol<DefinedSynthetic>(t, t->getName(), tableChunk);
-  cast<DefinedAbsolute>(c)->setVA(tableChunk->getSize() / (hasFlag ? 5 : 4));
+  ctx.forEachSymtab([&](SymbolTable &symtab) {
+    Symbol *t = symtab.findUnderscore(tableSym);
+    Symbol *c = symtab.findUnderscore(countSym);
+    replaceSymbol<DefinedSynthetic>(t, t->getName(), tableChunk);
+    cast<DefinedAbsolute>(c)->setVA(tableChunk->getSize() / (hasFlag ? 5 : 4));
+  });
 }
 
 // Create CHPE metadata chunks.
diff --git a/lld/test/COFF/arm64x-guardcf.s b/lld/test/COFF/arm64x-guardcf.s
new file mode 100644
index 0000000000000..750bf0b3862c5
--- /dev/null
+++ b/lld/test/COFF/arm64x-guardcf.s
@@ -0,0 +1,122 @@
+// REQUIRES: aarch64, x86
+// RUN: split-file %s %t.dir && cd %t.dir
+
+// RUN: llvm-mc -filetype=obj -triple=aarch64-windows func-gfids.s -o func-gfids-arm64.obj
+// RUN: llvm-mc -filetype=obj -triple=arm64ec-windows func-gfids.s -o func-gfids-arm64ec.obj
+// RUN: llvm-mc -filetype=obj -triple=aarch64-windows func-exp.s -o func-exp-arm64.obj
+// RUN: llvm-mc -filetype=obj -triple=arm64ec-windows func-exp.s -o func-exp-arm64ec.obj
+// RUN: llvm-mc -filetype=obj -triple=aarch64-windows dllmain.s -o dllmain-arm64.obj
+// RUN: llvm-mc -filetype=obj -triple=arm64ec-windows dllmain.s -o dllmain-arm64ec.obj
+// RUN: llvm-mc -filetype=obj -triple=x86_64-windows func-amd64.s -o func-amd64.obj
+// RUN: llvm-mc -filetype=obj -triple=arm64ec-windows %S/Inputs/loadconfig-arm64ec.s -o loadconfig-arm64ec.obj
+// RUN: llvm-mc -filetype=obj -triple=aarch64-windows %S/Inputs/loadconfig-arm64.s -o loadconfig-arm64.obj
+
+
+// Check that CF guard tables contain both native and EC symbols and are referenced from both load configs.
+
+// RUN: lld-link -dll -noentry -machine:arm64x func-gfids-arm64.obj func-gfids-arm64ec.obj func-amd64.obj -guard:cf -out:out.dll \
+// RUN:          loadconfig-arm64ec.obj loadconfig-arm64.obj
+// RUN: llvm-readobj --coff-load-config out.dll | FileCheck --check-prefix=LOADCFG %s
+
+// LOADCFG:      LoadConfig [
+// LOADCFG:        GuardCFFunctionCount: 3
+// LOADCFG-NEXT:   GuardFlags [ (0x10500)
+// LOADCFG-NEXT:     CF_FUNCTION_TABLE_PRESENT (0x400)
+// LOADCFG-NEXT:     CF_INSTRUMENTED (0x100)
+// LOADCFG-NEXT:     CF_LONGJUMP_TABLE_PRESENT (0x10000)
+// LOADCFG-NEXT:   ]
+// LOADCFG:      ]
+// LOADCFG:      GuardFidTable [
+// LOADCFG-NEXT:   0x180001000
+// LOADCFG-NEXT:   0x180002000
+// LOADCFG-NEXT:   0x180003000
+// LOADCFG-NEXT: ]
+// LOADCFG:      HybridObject {
+// LOADCFG:        LoadConfig [
+// LOADCFG:          GuardCFFunctionCount: 3
+// LOADCFG-NEXT:     GuardFlags [ (0x10500)
+// LOADCFG-NEXT:       CF_FUNCTION_TABLE_PRESENT (0x400)
+// LOADCFG-NEXT:       CF_INSTRUMENTED (0x100)
+// LOADCFG-NEXT:       CF_LONGJUMP_TABLE_PRESENT (0x10000)
+// LOADCFG-NEXT:     ]
+// LOADCFG:        ]
+// LOADCFG:        GuardFidTable [
+// LOADCFG-NEXT:     0x180001000
+// LOADCFG-NEXT:     0x180002000
+// LOADCFG-NEXT:     0x180003000
+// LOADCFG-NEXT:   ]
+// LOADCFG:      ]
+
+
+// Check that exports from both views are present in CF guard tables.
+
+// RUN: lld-link -dll -noentry -machine:arm64x func-exp-arm64.obj func-exp-arm64ec.obj -guard:cf -out:out-exp.dll \
+// RUN:          loadconfig-arm64ec.obj loadconfig-arm64.obj
+// RUN: llvm-readobj --coff-load-config out-exp.dll | FileCheck --check-prefix=LOADCFG %s
+
+
+// Check that entry points from both views are present in CF guard tables.
+
+// RUN: lld-link -dll -machine:arm64x dllmain-arm64.obj dllmain-arm64ec.obj -guard:cf -out:out-entry.dll \
+// RUN:          loadconfig-arm64ec.obj loadconfig-arm64.obj
+// RUN: llvm-readobj --coff-load-config out-entry.dll | FileCheck --check-prefix=LOADCFG %s
+
+
+// Check that both load configs are marked as instrumented if any input object was built with /guard:cf.
+
+// RUN: lld-link -dll -noentry -machine:arm64x func-gfids-arm64ec.obj -out:out-nocfg.dll \
+// RUN:          loadconfig-arm64ec.obj loadconfig-arm64.obj
+
+// RUN: llvm-readobj --coff-load-config out-nocfg.dll | FileCheck --check-prefix=LOADCFG-INST %s
+
+// LOADCFG-INST:      LoadConfig [
+// LOADCFG-INST:        GuardFlags [ (0x100)
+// LOADCFG-INST-NEXT:     CF_INSTRUMENTED (0x100)
+// LOADCFG-INST-NEXT:   ]
+// LOADCFG-INST:      ]
+// LOADCFG-INST:      HybridObject {
+// LOADCFG-INST:        LoadConfig [
+// LOADCFG-INST:          GuardFlags [ (0x100)
+// LOADCFG-INST-NEXT:       CF_INSTRUMENTED (0x100)
+// LOADCFG-INST-NEXT:     ]
+// LOADCFG-INST:        ]
+// LOADCFG-INST:      ]
+
+#--- func-gfids.s
+        .def @feat.00; .scl 3; .type 0; .endef
+        .globl @feat.00
+@feat.00 = 0x800
+
+        .globl func
+func:
+        ret
+
+        .section .gfids$y,"dr"
+        .symidx func
+
+#--- func-amd64.s
+        .def @feat.00; .scl 3; .type 0; .endef
+        .globl @feat.00
+@feat.00 = 0x800
+
+        .globl func_amd64
+func_amd64:
+        ret
+
+        .section .gfids$y,"dr"
+        .symidx func_amd64
+
+#--- func-exp.s
+        .def func; .scl 2; .type 32; .endef
+        .globl func
+func:
+        ret
+
+        .section .drectve
+        .ascii "-export:func"
+
+#--- dllmain.s
+        .def _DllMainCRTStartup; .scl 2; .type 32; .endef
+        .globl _DllMainCRTStartup
+_DllMainCRTStartup:
+        ret

[mstorsjo]

LGTM

[ZequanWu]

This causes lld-link to crash for chromium windows build:

Stack dump:
0.      Program arguments: ..\\..\\third_party\\llvm-build\\Release+Asserts\\bin\\lld-link.exe /OUT:./nasm.exe /nologo -libpath:../../third_party/llvm-build/Release+Asserts/lib/clang/21/lib/windows /winsysroot:../../third_party/depot_tools/win_toolchain/vs_files/7393122652 /MACHINE:X64 /PDB:./nasm.exe.pdb @./nasm.exe.rsp /reproduce:repro.tar
Exception Code: 0xC0000005
 #0 0x00007ff720a18d26 lld::coff::Symbol::kind C:\src\chromium\src\third_party\llvm\lld\COFF\Symbols.h:69:0
 #1 0x00007ff720a18d26 lld::coff::Defined::getChunk C:\src\chromium\src\third_party\llvm\lld\COFF\Symbols.h:474:0
 #2 0x00007ff720a18d26 addSymbolToRVASet C:\src\chromium\src\third_party\llvm\lld\COFF\Writer.cpp:1964:0
 #3 0x00007ff720a07edb `anonymous namespace'::Writer::markSymbolsWithRelocations C:\src\chromium\src\third_party\llvm\lld\COFF\Writer.cpp:2040:0
 #4 0x00007ff720a07edb `anonymous namespace'::Writer::createGuardCFTables C:\src\chromium\src\third_party\llvm\lld\COFF\Writer.cpp:2092:0
 #5 0x00007ff720a07edb `anonymous namespace'::Writer::createMiscChunks C:\src\chromium\src\third_party\llvm\lld\COFF\Writer.cpp:1256:0
 #6 0x00007ff720a07edb `anonymous namespace'::Writer::run C:\src\chromium\src\third_party\llvm\lld\COFF\Writer.cpp:777:0
 #7 0x00007ff720a03b32 std::vector<std::pair<lld::coff::Chunk *,lld::coff::Defined *>,std::allocator<std::pair<lld::coff::Chunk *,lld::coff::Defined *> > >::_Tidy C:\src\chromium\src\third_party\depot_tools\win_toolchain\vs_files\7393122652\VC\Tools\MSVC\14.39.33519\include\vector:2052:0
 #8 0x00007ff720a03b32 std::vector<std::pair<lld::coff::Chunk *,lld::coff::Defined *>,std::allocator<std::pair<lld::coff::Chunk *,lld::coff::Defined *> > >::~vector C:\src\chromium\src\third_party\depot_tools\win_toolchain\vs_files\7393122652\VC\Tools\MSVC\14.39.33519\include\vector:765:0
 #9 0x00007ff720a03b32 `anonymous namespace'::Writer::~Writer C:\src\chromium\src\third_party\llvm\lld\COFF\Writer.cpp:204:0
#10 0x00007ff720a03b32 lld::coff::writeResult(class lld::coff::COFFLinkerContext &) C:\src\chromium\src\third_party\llvm\lld\COFF\Writer.cpp:355:0
#11 0x00007ff7207168a3 lld::coff::LinkerDriver::linkerMain(class llvm::ArrayRef<char const *>) C:\src\chromium\src\third_party\llvm\lld\COFF\Driver.cpp:2792:0
#12 0x00007ff72070bedd lld::coff::link(class llvm::ArrayRef<char const *>, class llvm::raw_ostream &, class llvm::raw_ostream &, bool, bool) C:\src\chromium\src\third_party\llvm\lld\COFF\Driver.cpp:98:0
#13 0x00007ff72070b642 lld::unsafeLldMain(class llvm::ArrayRef<char const *>, class llvm::raw_ostream &, class llvm::raw_ostream &, class llvm::ArrayRef<struct lld::DriverDef>, bool) C:\src\chromium\src\third_party\llvm\lld\Common\DriverDispatcher.cpp:163:0
#14 0x00007ff7206d1228 lld_main(int, char **, struct llvm::ToolContext const &) C:\src\chromium\src\third_party\llvm\lld\tools\lld\lld.cpp:115:0
#15 0x00007ff7206d1a35 main C:\src\chromium\src\third_party\llvm-build\Release+Asserts\tools\lld\tools\lld\lld-driver.cpp:17:0
#16 0x00007ff723c78de0 invoke_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:78:0
#17 0x00007ff723c78de0 __scrt_common_main_seh D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288:0
#18 0x00007fffd29e7374 (C:\Windows\System32\KERNEL32.DLL+0x17374)
#19 0x00007fffd443cc91 (C:\Windows\SYSTEM32\ntdll.dll+0x4cc91)

Attached a repro: repro.tar.gz

Please revert this change if it takes a while to fix.

[cjacek]

Sorry about that. We're missing classof for ECExportThunkChunk, so the dyn_cast_or_null is not right. It should be easy to fix, I'm working on it.

[cjacek]

#130106 should fix the problem.

[ZequanWu]

Given that fixes still need to going through review, I'll just revert this commit to keep lld-link working.

[mstorsjo]

@cjacek I guess this could/should be relanded now that the fix has been merged?

[cjacek]

Yes, I relanded it. @ZequanWu Thanks for the repro and the revert, it should be good now.