Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LLD] [COFF] Mark the symbol _tls_used as a GC root #71336

Merged
merged 1 commit into from
Nov 7, 2023

Conversation

mstorsjo
Copy link
Member

@mstorsjo mstorsjo commented Nov 5, 2023

This is necessary if the object file containing _tls_used is built with -fdata-sections; if this symbol is present, lld is going to include references to it in the PE header, in the TLS_TABLE data directory.

Therefore, treat the symbol as a GC root, i.e. retain it (and whatever other symbols the section chunk references) when doing doing GC (/opt:ref), if the symbol is present.

This is necessary if the object file containing _tls_used is built
with -fdata-sections; if this symbol is present, lld is going to
include references to it in the PE header, in the TLS_TABLE data
directory.

Therefore, treat the symbol as a GC root, i.e. retain it (and whatever
other symbols the section chunk references) when doing doing GC
(/opt:ref), if the symbol is present.
@llvmbot
Copy link
Member

llvmbot commented Nov 5, 2023

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

@llvm/pr-subscribers-lld-coff

Author: Martin Storsjö (mstorsjo)

Changes

This is necessary if the object file containing _tls_used is built with -fdata-sections; if this symbol is present, lld is going to include references to it in the PE header, in the TLS_TABLE data directory.

Therefore, treat the symbol as a GC root, i.e. retain it (and whatever other symbols the section chunk references) when doing doing GC (/opt:ref), if the symbol is present.


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

2 Files Affected:

  • (modified) lld/COFF/Driver.cpp (+4)
  • (added) lld/test/COFF/tls-used-gc.s (+21)
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 5613c2e6993a5af..b2d0edd8cb2600e 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -2472,6 +2472,10 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
   // resolve symbols and write indices, but don't generate native code or link).
   ctx.symtab.compileBitcodeFiles();
 
+  if (Defined *d =
+          dyn_cast_or_null<Defined>(ctx.symtab.findUnderscore("_tls_used")))
+    config->gcroot.push_back(d);
+
   // If -thinlto-index-only is given, we should create only "index
   // files" and not object files. Index file creation is already done
   // in addCombinedLTOObject, so we are done if that's the case.
diff --git a/lld/test/COFF/tls-used-gc.s b/lld/test/COFF/tls-used-gc.s
new file mode 100644
index 000000000000000..4736be67494985f
--- /dev/null
+++ b/lld/test/COFF/tls-used-gc.s
@@ -0,0 +1,21 @@
+# REQUIRES: x86
+# RUN: llvm-mc -triple=x86_64-windows-gnu -filetype=obj -o %t.obj %s
+# RUN: lld-link %t.obj /out:%t.exe /entry:main /subsystem:console /opt:ref
+# RUN: llvm-readobj --file-headers %t.exe | FileCheck %s
+
+# CHECK: TLSTableRVA: 0x1000
+# CHECK: TLSTableSize: 0x28
+
+  .section .text@main,"xr",one_only,main
+  .globl main
+main:
+  ret
+
+  .section .tls$aaa
+tlsvar:
+  .long 1
+
+  .section .rdata$_tls_used,"dr",one_only,_tls_used
+  .globl _tls_used
+_tls_used:
+  .zero 40

Copy link
Collaborator

@rnk rnk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@mstorsjo mstorsjo merged commit 303370e into llvm:main Nov 7, 2023
6 checks passed
@mstorsjo mstorsjo deleted the lld-tls-used-gc branch November 7, 2023 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants