[LLD][COFF] Support /DEPENDENTLOADFLAG[:flags] #71537

nurmukhametov · 2023-11-07T14:00:03Z

This should fix #43935

llvmbot · 2023-11-07T14:00:36Z

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

@llvm/pr-subscribers-lld

Author: Alexey Nurmukhametov (nurmukhametov)

Changes

This should fix #43935

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

7 Files Affected:

(modified) lld/COFF/Config.h (+1)
(modified) lld/COFF/Driver.cpp (+4)
(modified) lld/COFF/Driver.h (+3)
(modified) lld/COFF/DriverUtils.cpp (+12)
(modified) lld/COFF/Options.td (+3)
(modified) lld/COFF/Writer.cpp (+31)
(added) lld/test/COFF/dependentflags.test (+28)

diff --git a/lld/COFF/Config.h b/lld/COFF/Config.h
index 1c338cc63fa87d2..bee6dc3ec3caea4 100644
--- a/lld/COFF/Config.h
+++ b/lld/COFF/Config.h
@@ -287,6 +287,7 @@ struct Configuration {
   uint32_t timestamp = 0;
   uint32_t functionPadMin = 0;
   uint32_t timeTraceGranularity = 0;
+  uint16_t dependentLoadFlags = 0;
   bool dynamicBase = true;
   bool allowBind = true;
   bool cetCompat = false;
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index b2d0edd8cb2600e..6563b45830e6ffd 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -2179,6 +2179,10 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
   for (auto *arg : args.filtered(OPT_functionpadmin, OPT_functionpadmin_opt))
     parseFunctionPadMin(arg);
 
+  // Handle /dependentloadflag
+  for (auto *arg : args.filtered(OPT_dependentloadflag, OPT_dependentloadflag_opt))
+    parseDependentLoadFlags(arg);
+
   if (tar) {
     llvm::TimeTraceScope timeScope("Reproducer: response file");
     tar->append("response.txt",
diff --git a/lld/COFF/Driver.h b/lld/COFF/Driver.h
index 549407539cdde9b..fa54de05befb580 100644
--- a/lld/COFF/Driver.h
+++ b/lld/COFF/Driver.h
@@ -233,6 +233,9 @@ class LinkerDriver {
   // Parses a string in the form of "[:<integer>]"
   void parseFunctionPadMin(llvm::opt::Arg *a);
 
+  // Parses a string in the form of "[:<integer>]"
+  void parseDependentLoadFlags(llvm::opt::Arg *a);
+
   // Parses a string in the form of "EMBED[,=<integer>]|NO".
   void parseManifest(StringRef arg);
 
diff --git a/lld/COFF/DriverUtils.cpp b/lld/COFF/DriverUtils.cpp
index 583f6af070b6258..510dd3c1ceb4bed 100644
--- a/lld/COFF/DriverUtils.cpp
+++ b/lld/COFF/DriverUtils.cpp
@@ -265,6 +265,18 @@ void LinkerDriver::parseFunctionPadMin(llvm::opt::Arg *a) {
   }
 }
 
+// Parses /dependentloadflag option argument.
+void LinkerDriver::parseDependentLoadFlags(llvm::opt::Arg *a) {
+  StringRef arg = a->getNumValues() ? a->getValue() : "";
+  if (!arg.empty()) {
+    if (arg.getAsInteger(0, ctx.config.dependentLoadFlags))
+      error("/dependentloadflag: invalid argument: " + arg);
+    return;
+  }
+  // No optional argument given. Default value is 0.
+  ctx.config.dependentLoadFlags = 0;
+}
+
 // Parses a string in the form of "EMBED[,=<integer>]|NO".
 // Results are directly written to
 // Config.
diff --git a/lld/COFF/Options.td b/lld/COFF/Options.td
index 977657a433dc581..abee6602726892d 100644
--- a/lld/COFF/Options.td
+++ b/lld/COFF/Options.td
@@ -56,6 +56,9 @@ def filealign : P<"filealign", "Section alignment in the output file">;
 def functionpadmin : F<"functionpadmin">;
 def functionpadmin_opt : P<"functionpadmin",
     "Prepares an image for hotpatching">;
+def dependentloadflag : F<"dependentloadflag">;
+def dependentloadflag_opt : P<"dependentloadflag",
+    "Sets the default load flags used to resolve the statically linked imports of a module">;
 def guard   : P<"guard", "Control flow guard">;
 def heap    : P<"heap", "Size of the heap">;
 def ignore : P<"ignore", "Specify warning codes to ignore">;
diff --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp
index 960328d686852a3..eb26241d5229662 100644
--- a/lld/COFF/Writer.cpp
+++ b/lld/COFF/Writer.cpp
@@ -264,6 +264,9 @@ class Writer {
 
   uint32_t getSizeOfInitializedData();
 
+  void changeLoadConfig();
+  template <typename T> void changeLoadConfigGuardData(T *loadConfig);
+
   void checkLoadConfig();
   template <typename T> void checkLoadConfigGuardData(const T *loadConfig);
 
@@ -696,6 +699,7 @@ void Writer::run() {
       writeHeader<pe32_header>();
     }
     writeSections();
+    changeLoadConfig();
     checkLoadConfig();
     sortExceptionTable();
 
@@ -2256,6 +2260,33 @@ void Writer::fixTlsAlignment() {
   }
 }
 
+void Writer::changeLoadConfig() {
+  Symbol *sym = ctx.symtab.findUnderscore("_load_config_used");
+  auto *b = cast_if_present<DefinedRegular>(sym);
+  if (!b) {
+    if (ctx.config.guardCF != GuardCFLevel::Off)
+      warn("Control Flow Guard is enabled but '_load_config_used' is missing");
+    return;
+  }
+
+  OutputSection *sec = ctx.getOutputSection(b->getChunk());
+  uint8_t *buf = buffer->getBufferStart();
+  uint8_t *secBuf = buf + sec->getFileOff();
+  uint8_t *symBuf = secBuf + (b->getRVA() - sec->getRVA());
+  if (ctx.config.is64())
+    changeLoadConfigGuardData(
+        reinterpret_cast<coff_load_configuration64 *>(symBuf));
+  else
+    changeLoadConfigGuardData(
+        reinterpret_cast<coff_load_configuration32 *>(symBuf));
+}
+
+template <typename T>
+void Writer::changeLoadConfigGuardData(T *loadConfig) {
+  if (ctx.config.dependentLoadFlags)
+    loadConfig->DependentLoadFlags = ctx.config.dependentLoadFlags;
+}
+
 void Writer::checkLoadConfig() {
   Symbol *sym = ctx.symtab.findUnderscore("_load_config_used");
   auto *b = cast_if_present<DefinedRegular>(sym);
diff --git a/lld/test/COFF/dependentflags.test b/lld/test/COFF/dependentflags.test
new file mode 100644
index 000000000000000..3507e098a06eb0c
--- /dev/null
+++ b/lld/test/COFF/dependentflags.test
@@ -0,0 +1,28 @@
+// ---- precomp-a.obj - x86_64, hotpatch
+RUN: llvm-mc -triple x86_64-windows-msvc -filetype=obj %S/Inputs/loadconfig-cfg-x64.s -o %t.ldcfg.obj
+
+RUN: lld-link %S/Inputs/precomp-a.obj %t.ldcfg.obj /out:%t.exe /nodefaultlib /force
+RUN: llvm-readobj --coff-load-config %t.exe | FileCheck %s --check-prefix BASE
+
+RUN: lld-link %S/Inputs/precomp-a.obj %t.ldcfg.obj /out:%t.exe /nodefaultlib /force /dependentloadflag
+RUN: llvm-readobj --coff-load-config %t.exe | FileCheck %s --check-prefix BASE
+
+RUN: lld-link %S/Inputs/precomp-a.obj %t.ldcfg.obj /out:%t.exe /nodefaultlib /force /dependentloadflag:0x800
+RUN: llvm-readobj --coff-load-config %t.exe | FileCheck %s --check-prefix FLAGS
+
+// ---- Many arguments
+RUN: lld-link %S/Inputs/precomp-a.obj %t.ldcfg.obj /out:%t.exe /nodefaultlib /force /dependentloadflag:0x800 /dependentloadflag
+RUN: llvm-readobj --coff-load-config %t.exe | FileCheck %s --check-prefix BASE
+
+RUN: lld-link %S/Inputs/precomp-a.obj %t.ldcfg.obj /out:%t.exe /nodefaultlib /force /dependentloadflag /dependentloadflag:0x800
+RUN: llvm-readobj --coff-load-config %t.exe | FileCheck %s --check-prefix FLAGS
+
+RUN: not lld-link %S/Inputs/precomp-a.obj %t.ldcfg.obj /out:%t.exe /nodefaultlib /force /dependentloadflag:zz 2>&1 | FileCheck %s --check-prefix FAIL
+RUN: not lld-link %S/Inputs/precomp-a.obj %t.ldcfg.obj /out:%t.exe /nodefaultlib /force /dependentloadflag:0xf0000 2>&1 | FileCheck %s --check-prefix FAIL-RANGE
+
+
+BASE: DependentLoadFlags: 0x0
+FLAGS: DependentLoadFlags: 0x800
+
+FAIL: lld-link: error: /dependentloadflag: invalid argument: zz
+FAIL-RANGE: lld-link: error: /dependentloadflag: invalid argument: 0xf0000
\ No newline at end of file

github-actions · 2023-11-07T14:10:27Z

✅ With the latest revision this PR passed the C/C++ code formatter.

tru

I am not good with the internals of what should change here, but here are some smaller nit comments.

tru · 2023-11-07T14:13:21Z

lld/test/COFF/dependentflags.test

+FLAGS: DependentLoadFlags: 0x800
+
+FAIL: lld-link: error: /dependentloadflag: invalid argument: zz
+FAIL-RANGE: lld-link: error: /dependentloadflag: invalid argument: 0xf0000


Missing EOF.

tru · 2023-11-07T14:14:11Z

lld/test/COFF/dependentflags.test

+RUN: llvm-readobj --coff-load-config %t.exe | FileCheck %s --check-prefix FLAGS
+
+RUN: not lld-link %S/Inputs/precomp-a.obj %t.ldcfg.obj /out:%t.exe /nodefaultlib /force /dependentloadflag:zz 2>&1 | FileCheck %s --check-prefix FAIL
+RUN: not lld-link %S/Inputs/precomp-a.obj %t.ldcfg.obj /out:%t.exe /nodefaultlib /force /dependentloadflag:0xf0000 2>&1 | FileCheck %s --check-prefix FAIL-RANGE


might want to have a test to make sure the last argument is the one being used.

like these ones? https://github.com/llvm/llvm-project/blob/9df498d635a534d1671bcc22a2334973b506d25a/lld/test/COFF/dependentflags.test#L11-L15

tru · 2023-11-07T14:15:45Z

lld/COFF/Writer.cpp

@@ -696,6 +699,7 @@ void Writer::run() {
      writeHeader<pe32_header>();
    }
    writeSections();
+    changeLoadConfig();


Should this run even if dependentloadflag is not set?

Not really, it is addressed in the latest revision.

tru · 2023-11-07T14:17:11Z

Also make sure to run clang-format to fix the formatting problems.

aganea · 2023-11-07T16:09:48Z

lld/COFF/Writer.cpp

@@ -2256,6 +2262,32 @@ void Writer::fixTlsAlignment() {
  }
 }

+void Writer::changeLoadConfig() {


This duplicates a lot of code from checkLoadConfig. Can you rather change that function to something like prepareLoadConfig and add this flag there?

Yes. I have changed these duplication to prepareLoadConfig. I hope it is what you meant.

lld/COFF/Writer.cpp

aganea

Looks good to me otherwise! Thank you!

lld/COFF/DriverUtils.cpp

tru

LGTM

nurmukhametov · 2023-11-08T17:06:08Z

I've fixed typo in commit message.

luporl · 2023-11-09T13:52:46Z

It looks like this broke clang-arm64-windows-msvc buildbot:
https://lab.llvm.org/buildbot/#/builders/65/builds/11819

nurmukhametov · 2023-11-09T13:56:13Z

It looks like this broke clang-arm64-windows-msvc buildbot: https://lab.llvm.org/buildbot/#/builders/65/builds/11819

Indeed, I probably had to provide # REQUIRES: x86 in lit tests?

aganea · 2023-11-09T13:57:20Z

@nurmukhametov Do you have commit rights? If yes you can push a fix commit directly without a PR.

nurmukhametov · 2023-11-09T14:10:07Z

@nurmukhametov Do you have commit rights? If yes you can push a fix commit directly without a PR.

No, I don't think I have.

This fixes: https://lab.llvm.org/buildbot/#/builders/65/builds/11819 Also see discussion in: #71537

aganea · 2023-11-09T15:25:04Z

Fixed in b371ea3

MaskRay · 2023-12-06T05:49:15Z

This should fix #43935

The description is too brief. I'd say "Close #43935: support /DEPENDENTLOADFLAG , which does XXX", so that a curious reader doesn't have to open #43935 to get a basic idea of what this option is about.

This fixes: https://lab.llvm.org/buildbot/#/builders/65/builds/11819 Also see discussion in: llvm/llvm-project#71537

llvmbot added lld lld:COFF platform:windows labels Nov 7, 2023

tru reviewed Nov 7, 2023

View reviewed changes

nurmukhametov force-pushed the dependentloadflag branch 2 times, most recently from 2a8bf36 to e34aa5b Compare November 7, 2023 15:46

aganea reviewed Nov 7, 2023

View reviewed changes

nurmukhametov force-pushed the dependentloadflag branch 2 times, most recently from 12c90e5 to 9df498d Compare November 7, 2023 17:10

aganea approved these changes Nov 7, 2023

View reviewed changes

lld/COFF/DriverUtils.cpp Outdated Show resolved Hide resolved

nurmukhametov force-pushed the dependentloadflag branch from 9df498d to 8403917 Compare November 8, 2023 12:15

nurmukhametov requested a review from tru November 8, 2023 13:34

tru approved these changes Nov 8, 2023

View reviewed changes

[LLD][COFF] Support /DEPENDENTLOADFLAG[:flags]

3b85a5e

nurmukhametov force-pushed the dependentloadflag branch from 8403917 to 3b85a5e Compare November 8, 2023 17:05

nurmukhametov changed the title ~~[LLD][COFF] Support /DEPENDENTLOADFLAGS[:flags]~~ [LLD][COFF] Support /DEPENDENTLOADFLAG[:flags] Nov 8, 2023

aganea merged commit 76947e0 into llvm:main Nov 8, 2023
3 checks passed

aganea added a commit that referenced this pull request Nov 9, 2023

[LLD][COFF] Fix /dependentloadflag tests with AArch64

b371ea3

This fixes: https://lab.llvm.org/buildbot/#/builders/65/builds/11819 Also see discussion in: #71537

qihangkong pushed a commit to rvgpu/llvm that referenced this pull request Apr 18, 2024

[LLD][COFF] Fix /dependentloadflag tests with AArch64

bceffb3

This fixes: https://lab.llvm.org/buildbot/#/builders/65/builds/11819 Also see discussion in: llvm/llvm-project#71537

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[LLD][COFF] Support /DEPENDENTLOADFLAG[:flags] #71537

[LLD][COFF] Support /DEPENDENTLOADFLAG[:flags] #71537

nurmukhametov commented Nov 7, 2023

llvmbot commented Nov 7, 2023 •

edited

Loading

github-actions bot commented Nov 7, 2023 •

edited

Loading

tru left a comment

tru Nov 7, 2023

tru Nov 7, 2023

nurmukhametov Nov 7, 2023

tru Nov 7, 2023

nurmukhametov Nov 8, 2023

tru commented Nov 7, 2023

aganea Nov 7, 2023

nurmukhametov Nov 7, 2023

aganea left a comment

tru left a comment

nurmukhametov commented Nov 8, 2023 •

edited

Loading

luporl commented Nov 9, 2023

nurmukhametov commented Nov 9, 2023

aganea commented Nov 9, 2023

nurmukhametov commented Nov 9, 2023

aganea commented Nov 9, 2023

MaskRay commented Dec 6, 2023 •

edited

Loading

[LLD][COFF] Support /DEPENDENTLOADFLAG[:flags] #71537

[LLD][COFF] Support /DEPENDENTLOADFLAG[:flags] #71537

Conversation

nurmukhametov commented Nov 7, 2023

llvmbot commented Nov 7, 2023 • edited Loading

github-actions bot commented Nov 7, 2023 • edited Loading

tru left a comment

Choose a reason for hiding this comment

tru Nov 7, 2023

Choose a reason for hiding this comment

tru Nov 7, 2023

Choose a reason for hiding this comment

nurmukhametov Nov 7, 2023

Choose a reason for hiding this comment

tru Nov 7, 2023

Choose a reason for hiding this comment

nurmukhametov Nov 8, 2023

Choose a reason for hiding this comment

tru commented Nov 7, 2023

aganea Nov 7, 2023

Choose a reason for hiding this comment

nurmukhametov Nov 7, 2023

Choose a reason for hiding this comment

aganea left a comment

Choose a reason for hiding this comment

tru left a comment

Choose a reason for hiding this comment

nurmukhametov commented Nov 8, 2023 • edited Loading

luporl commented Nov 9, 2023

nurmukhametov commented Nov 9, 2023

aganea commented Nov 9, 2023

nurmukhametov commented Nov 9, 2023

aganea commented Nov 9, 2023

MaskRay commented Dec 6, 2023 • edited Loading

llvmbot commented Nov 7, 2023 •

edited

Loading

github-actions bot commented Nov 7, 2023 •

edited

Loading

nurmukhametov commented Nov 8, 2023 •

edited

Loading

MaskRay commented Dec 6, 2023 •

edited

Loading