Skip to content

[LLD] [COFF] Preserve directives and export names from LTO objects #78802

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

Merged
merged 1 commit into from
Jan 20, 2024

Conversation

mstorsjo
Copy link
Member

The export names are saved as StringRefs pointing into the COFF directives. In the case of LTO objects, this can be memory allocated that is owned by the LTO InputFile, which gets destructed when doing the compilation.

In the case of LTO objects from an older version of LLVM, which require being upgraded when loaded, the directives string gets destructed, while when using LTO objects of a matching version (the common case), the directives string points into memory that doesn't get destructed on LTO compilation.

Test this by linking a bundled binary LTO object file, from an older version of LLVM.

This fixes issue #78591, and downstream issue mstorsjo/llvm-mingw#392.

The export names are saved as StringRefs pointing into the
COFF directives. In the case of LTO objects, this can be
memory allocated that is owned by the LTO InputFile, which
gets destructed when doing the compilation.

In the case of LTO objects from an older version of LLVM, which
require being upgraded when loaded, the directives string gets
destructed, while when using LTO objects of a matching
version (the common case), the directives string points into
memory that doesn't get destructed on LTO compilation.

Test this by linking a bundled binary LTO object file, from
an older version of LLVM.

This fixes issue llvm#78591.
@llvmbot
Copy link
Member

llvmbot commented Jan 19, 2024

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

@llvm/pr-subscribers-lld-coff

Author: Martin Storsjö (mstorsjo)

Changes

The export names are saved as StringRefs pointing into the COFF directives. In the case of LTO objects, this can be memory allocated that is owned by the LTO InputFile, which gets destructed when doing the compilation.

In the case of LTO objects from an older version of LLVM, which require being upgraded when loaded, the directives string gets destructed, while when using LTO objects of a matching version (the common case), the directives string points into memory that doesn't get destructed on LTO compilation.

Test this by linking a bundled binary LTO object file, from an older version of LLVM.

This fixes issue #78591, and downstream issue mstorsjo/llvm-mingw#392.


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

3 Files Affected:

  • (modified) lld/COFF/InputFiles.cpp (+1-1)
  • (added) lld/test/COFF/Inputs/lto-directives.obj ()
  • (added) lld/test/COFF/lto-directives.test (+15)
diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp
index cd744800cb0dec..22cc0e3e5dbaf7 100644
--- a/lld/COFF/InputFiles.cpp
+++ b/lld/COFF/InputFiles.cpp
@@ -1081,7 +1081,7 @@ void BitcodeFile::parse() {
     if (objSym.isUsed())
       ctx.config.gcroot.push_back(sym);
   }
-  directives = obj->getCOFFLinkerOpts();
+  directives = saver.save(obj->getCOFFLinkerOpts());
 }
 
 void BitcodeFile::parseLazy() {
diff --git a/lld/test/COFF/Inputs/lto-directives.obj b/lld/test/COFF/Inputs/lto-directives.obj
new file mode 100644
index 00000000000000..a413b0ab92f4e4
Binary files /dev/null and b/lld/test/COFF/Inputs/lto-directives.obj differ
diff --git a/lld/test/COFF/lto-directives.test b/lld/test/COFF/lto-directives.test
new file mode 100644
index 00000000000000..5890d426764c4e
--- /dev/null
+++ b/lld/test/COFF/lto-directives.test
@@ -0,0 +1,15 @@
+; REQUIRES: x86
+
+;; Test linking an LTO object file that contains directives. The
+;; LTO object file is built with an older toolchain, to force it
+;; to be upgraded when loaded.
+
+;; The input file is compiled from source that looks like this:
+;;   void __declspec(dllexport) entry(void) { }
+;; with this command:
+;;   clang -target x86_64-windows-msvc -c main.c -flto
+
+; RUN: lld-link /entry:entry /subsystem:console %p/Inputs/lto-directives.obj /dll /out:%t.dll
+; RUN: llvm-readobj --coff-exports %t.dll | FileCheck %s
+
+; CHECK: Name: entry

Copy link
Member

@MaskRay MaskRay left a comment

Choose a reason for hiding this comment

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

I confirm that this fixes https://github.com/mstorsjo/llvm-mingw/files/13934972/repro.tar.gz (xz disguised as gz :))

@mstorsjo mstorsjo merged commit d098651 into llvm:main Jan 20, 2024
@mstorsjo mstorsjo deleted the lld-lto-directives branch January 20, 2024 14:15
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