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.

Sign up for GitHub

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

Jump to bottom

[lld][COFF] Merge .00cfg section into .rdata. #75207

Merged
merged 1 commit into from
Dec 13, 2023
Merged

[lld][COFF] Merge .00cfg section into .rdata. #75207

merged 1 commit into from
Dec 13, 2023
+20 −2

Conversation

cjacek
Copy link
Contributor

@cjacek cjacek commented Dec 12, 2023

.00cfg section is used by crt for load config and is merged by MS link.exe into .rdata.

cc @bylaws

@cjacek
[lld][COFF] Merge .00cfg section into .rdata.
ec25a7e 
.00cfg section is used by crt for load config and is merged by MS link.exe into .rdata.
@llvmbot
Copy link
Collaborator

llvmbot commented Dec 12, 2023

@llvm/pr-subscribers-lld-coff

Author: Jacek Caban (cjacek)

Changes

.00cfg section is used by crt for load config and is merged by MS link.exe into .rdata.

cc @bylaws

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

4 Files Affected:

  • (modified) lld/COFF/Driver.cpp (+1)
  • (modified) lld/test/COFF/Inputs/loadconfig-arm64ec.s (+1-1)
  • (modified) lld/test/COFF/Inputs/loadconfig-cfg-x64.s (+1-1)
  • (added) lld/test/COFF/merge-00cfg.s (+17)
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 99c1a60735adc5..d4a2f5767a2e87 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -1940,6 +1940,7 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
   parseMerge(".didat=.rdata");
   parseMerge(".edata=.rdata");
   parseMerge(".xdata=.rdata");
+  parseMerge(".00cfg=.rdata");
   parseMerge(".bss=.data");
 
   if (config->mingw) {
diff --git a/lld/test/COFF/Inputs/loadconfig-arm64ec.s b/lld/test/COFF/Inputs/loadconfig-arm64ec.s
index 8bb5ccfed8ebc8..a270d281095dd6 100644
--- a/lld/test/COFF/Inputs/loadconfig-arm64ec.s
+++ b/lld/test/COFF/Inputs/loadconfig-arm64ec.s
@@ -1,4 +1,4 @@
-        .section .rdata,"dr"
+        .section .00cfg,"dr"
         .globl _load_config_used
         .p2align 3, 0
 _load_config_used:
diff --git a/lld/test/COFF/Inputs/loadconfig-cfg-x64.s b/lld/test/COFF/Inputs/loadconfig-cfg-x64.s
index 1440b115f46ad4..349d8c5a8db36e 100644
--- a/lld/test/COFF/Inputs/loadconfig-cfg-x64.s
+++ b/lld/test/COFF/Inputs/loadconfig-cfg-x64.s
@@ -1,6 +1,6 @@
 # This is the _load_config_used definition needed for /guard:cf tests.
 
-        .section .rdata,"dr"
+        .section .00cfg,"dr"
 .globl _load_config_used
 _load_config_used:
         .long 256
diff --git a/lld/test/COFF/merge-00cfg.s b/lld/test/COFF/merge-00cfg.s
new file mode 100644
index 00000000000000..89a4c8f0834210
--- /dev/null
+++ b/lld/test/COFF/merge-00cfg.s
@@ -0,0 +1,17 @@
+// REQUIRES: x86
+
+// RUN: llvm-mc -filetype=obj -triple=x86_64-windows %s -o %t-x86_64.obj
+// RUN: llvm-mc -filetype=obj -triple=i686-windows %s -o %t-x86.obj
+// RUN: lld-link -machine:amd64 -out:%t-x86_64.dll %t-x86_64.obj -dll -noentry
+// RUN: lld-link -machine:x86 -out:%t-x86.dll %t-x86.obj -dll -noentry -safeseh:no
+
+// RUN: llvm-readobj --hex-dump=.rdata %t-x86_64.dll | FileCheck %s -check-prefix=RDATA
+// RUN: llvm-readobj --hex-dump=.rdata %t-x86.dll | FileCheck %s -check-prefix=RDATA
+// RDATA: 78563412
+
+// RUN: llvm-readobj --sections %t-x86_64.dll | FileCheck %s -check-prefix=SECTIONS
+// RUN: llvm-readobj --sections %t-x86.dll | FileCheck %s -check-prefix=SECTIONS
+// SECTIONS-NOT: .00cfg
+
+        .section ".00cfg", "dr"
+        .long 0x12345678

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 12, 2023

@llvm/pr-subscribers-platform-windows

Author: Jacek Caban (cjacek)

Changes

.00cfg section is used by crt for load config and is merged by MS link.exe into .rdata.

cc @bylaws

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

4 Files Affected:

  • (modified) lld/COFF/Driver.cpp (+1)
  • (modified) lld/test/COFF/Inputs/loadconfig-arm64ec.s (+1-1)
  • (modified) lld/test/COFF/Inputs/loadconfig-cfg-x64.s (+1-1)
  • (added) lld/test/COFF/merge-00cfg.s (+17)
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 99c1a60735adc..d4a2f5767a2e8 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -1940,6 +1940,7 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
   parseMerge(".didat=.rdata");
   parseMerge(".edata=.rdata");
   parseMerge(".xdata=.rdata");
+  parseMerge(".00cfg=.rdata");
   parseMerge(".bss=.data");
 
   if (config->mingw) {
diff --git a/lld/test/COFF/Inputs/loadconfig-arm64ec.s b/lld/test/COFF/Inputs/loadconfig-arm64ec.s
index 8bb5ccfed8ebc..a270d281095dd 100644
--- a/lld/test/COFF/Inputs/loadconfig-arm64ec.s
+++ b/lld/test/COFF/Inputs/loadconfig-arm64ec.s
@@ -1,4 +1,4 @@
-        .section .rdata,"dr"
+        .section .00cfg,"dr"
         .globl _load_config_used
         .p2align 3, 0
 _load_config_used:
diff --git a/lld/test/COFF/Inputs/loadconfig-cfg-x64.s b/lld/test/COFF/Inputs/loadconfig-cfg-x64.s
index 1440b115f46ad..349d8c5a8db36 100644
--- a/lld/test/COFF/Inputs/loadconfig-cfg-x64.s
+++ b/lld/test/COFF/Inputs/loadconfig-cfg-x64.s
@@ -1,6 +1,6 @@
 # This is the _load_config_used definition needed for /guard:cf tests.
 
-        .section .rdata,"dr"
+        .section .00cfg,"dr"
 .globl _load_config_used
 _load_config_used:
         .long 256
diff --git a/lld/test/COFF/merge-00cfg.s b/lld/test/COFF/merge-00cfg.s
new file mode 100644
index 0000000000000..89a4c8f083421
--- /dev/null
+++ b/lld/test/COFF/merge-00cfg.s
@@ -0,0 +1,17 @@
+// REQUIRES: x86
+
+// RUN: llvm-mc -filetype=obj -triple=x86_64-windows %s -o %t-x86_64.obj
+// RUN: llvm-mc -filetype=obj -triple=i686-windows %s -o %t-x86.obj
+// RUN: lld-link -machine:amd64 -out:%t-x86_64.dll %t-x86_64.obj -dll -noentry
+// RUN: lld-link -machine:x86 -out:%t-x86.dll %t-x86.obj -dll -noentry -safeseh:no
+
+// RUN: llvm-readobj --hex-dump=.rdata %t-x86_64.dll | FileCheck %s -check-prefix=RDATA
+// RUN: llvm-readobj --hex-dump=.rdata %t-x86.dll | FileCheck %s -check-prefix=RDATA
+// RDATA: 78563412
+
+// RUN: llvm-readobj --sections %t-x86_64.dll | FileCheck %s -check-prefix=SECTIONS
+// RUN: llvm-readobj --sections %t-x86.dll | FileCheck %s -check-prefix=SECTIONS
+// SECTIONS-NOT: .00cfg
+
+        .section ".00cfg", "dr"
+        .long 0x12345678

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 12, 2023

@llvm/pr-subscribers-lld

Author: Jacek Caban (cjacek)

Changes

.00cfg section is used by crt for load config and is merged by MS link.exe into .rdata.

cc @bylaws

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

4 Files Affected:

  • (modified) lld/COFF/Driver.cpp (+1)
  • (modified) lld/test/COFF/Inputs/loadconfig-arm64ec.s (+1-1)
  • (modified) lld/test/COFF/Inputs/loadconfig-cfg-x64.s (+1-1)
  • (added) lld/test/COFF/merge-00cfg.s (+17)
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 99c1a60735adc..d4a2f5767a2e8 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -1940,6 +1940,7 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
   parseMerge(".didat=.rdata");
   parseMerge(".edata=.rdata");
   parseMerge(".xdata=.rdata");
+  parseMerge(".00cfg=.rdata");
   parseMerge(".bss=.data");
 
   if (config->mingw) {
diff --git a/lld/test/COFF/Inputs/loadconfig-arm64ec.s b/lld/test/COFF/Inputs/loadconfig-arm64ec.s
index 8bb5ccfed8ebc..a270d281095dd 100644
--- a/lld/test/COFF/Inputs/loadconfig-arm64ec.s
+++ b/lld/test/COFF/Inputs/loadconfig-arm64ec.s
@@ -1,4 +1,4 @@
-        .section .rdata,"dr"
+        .section .00cfg,"dr"
         .globl _load_config_used
         .p2align 3, 0
 _load_config_used:
diff --git a/lld/test/COFF/Inputs/loadconfig-cfg-x64.s b/lld/test/COFF/Inputs/loadconfig-cfg-x64.s
index 1440b115f46ad..349d8c5a8db36 100644
--- a/lld/test/COFF/Inputs/loadconfig-cfg-x64.s
+++ b/lld/test/COFF/Inputs/loadconfig-cfg-x64.s
@@ -1,6 +1,6 @@
 # This is the _load_config_used definition needed for /guard:cf tests.
 
-        .section .rdata,"dr"
+        .section .00cfg,"dr"
 .globl _load_config_used
 _load_config_used:
         .long 256
diff --git a/lld/test/COFF/merge-00cfg.s b/lld/test/COFF/merge-00cfg.s
new file mode 100644
index 0000000000000..89a4c8f083421
--- /dev/null
+++ b/lld/test/COFF/merge-00cfg.s
@@ -0,0 +1,17 @@
+// REQUIRES: x86
+
+// RUN: llvm-mc -filetype=obj -triple=x86_64-windows %s -o %t-x86_64.obj
+// RUN: llvm-mc -filetype=obj -triple=i686-windows %s -o %t-x86.obj
+// RUN: lld-link -machine:amd64 -out:%t-x86_64.dll %t-x86_64.obj -dll -noentry
+// RUN: lld-link -machine:x86 -out:%t-x86.dll %t-x86.obj -dll -noentry -safeseh:no
+
+// RUN: llvm-readobj --hex-dump=.rdata %t-x86_64.dll | FileCheck %s -check-prefix=RDATA
+// RUN: llvm-readobj --hex-dump=.rdata %t-x86.dll | FileCheck %s -check-prefix=RDATA
+// RDATA: 78563412
+
+// RUN: llvm-readobj --sections %t-x86_64.dll | FileCheck %s -check-prefix=SECTIONS
+// RUN: llvm-readobj --sections %t-x86.dll | FileCheck %s -check-prefix=SECTIONS
+// SECTIONS-NOT: .00cfg
+
+        .section ".00cfg", "dr"
+        .long 0x12345678

rnk
rnk approved these changes Dec 12, 2023
View reviewed changes
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.

Thank you for looking into this, this has been a longstanding known issue.

MaskRay
MaskRay approved these changes Dec 12, 2023
View reviewed changes
lld/test/COFF/Inputs/loadconfig-arm64ec.s
@@ -1,4 +1,4 @@
.section .rdata,"dr"
Copy link
Member

Choose a reason for hiding this comment

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

Do we lose test coverage by removing .rdata? If not, LGTM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we don't lose coverage. We still have other tests that declare _load_config_used in .rdata, so we should be protected from somehow accidentally depending on it being in .00cfg. I made the change to Inputs/ to widen coverage of merging and making shared load config more similar to real-world case feels right.

Thanks for reviews!

@cjacek cjacek merged commit f78024c into llvm:main Dec 13, 2023
7 checks passed
@cjacek cjacek deleted the merge-00cfg branch December 13, 2023 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rnk rnk rnk approved these changes

@compnerd compnerd compnerd approved these changes

@MaskRay MaskRay MaskRay approved these changes

@mstorsjo mstorsjo Awaiting requested review from mstorsjo

Assignees
No one assigned
Labels
lld:COFF lld platform:windows
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@cjacek @llvmbot @rnk @compnerd @MaskRay