[Object][Archive] Recompute headers and symbol map when switching from COFF to GNU64

[cjacek]

COFF format has no 64-bit version, so we use GNU64 instead. Since this changes the headers, we need to recalculate everything.

Fixes #160112.

[llvmbot]

@llvm/pr-subscribers-llvm-binary-utilities

Author: Jacek Caban (cjacek)

Changes

COFF format has no 64-bit version, so we use GNU64 instead. Since this changes the headers, we need to recalculate everything.

Fixes #160112.

---

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

2 Files Affected:

• (modified) llvm/lib/Object/ArchiveWriter.cpp (+12-2)
• (added) llvm/test/tools/llvm-lib/sym64-threshold.test (+27)

diff --git a/llvm/lib/Object/ArchiveWriter.cpp b/llvm/lib/Object/ArchiveWriter.cpp
index 6fc0889afc6a8..56b47c54318a8 100644
--- a/llvm/lib/Object/ArchiveWriter.cpp
+++ b/llvm/lib/Object/ArchiveWriter.cpp
@@ -1119,10 +1119,20 @@ Error writeArchiveToStream(raw_ostream &Out,
     // to switch to 64-bit. Note that the file can be larger than 4GB as long as
     // the last member starts before the 4GB offset.
     if (*HeadersSize + LastMemberHeaderOffset >= Sym64Threshold) {
-      if (Kind == object::Archive::K_DARWIN)
+      switch (Kind) {
+      case object::Archive::K_COFF:
+        // COFF format has no 64-bit version, so we use GNU64 instead.
+        // Since this changes the headers, we need to recalculate everything.
+        return writeArchiveToStream(Out, NewMembers, WriteSymtab,
+                                    object::Archive::K_GNU64, Deterministic,
+                                    Thin, IsEC, Warn);
+      case object::Archive::K_DARWIN:
         Kind = object::Archive::K_DARWIN64;
-      else
+        break;
+      default:
         Kind = object::Archive::K_GNU64;
+        break;
+      }
       HeadersSize.reset();
     }
   }
diff --git a/llvm/test/tools/llvm-lib/sym64-threshold.test b/llvm/test/tools/llvm-lib/sym64-threshold.test
new file mode 100644
index 0000000000000..d53bd5d8a06ab
--- /dev/null
+++ b/llvm/test/tools/llvm-lib/sym64-threshold.test
@@ -0,0 +1,27 @@
+# RUN: yaml2obj --docnum=1 %s -o %t01234567890234567789.obj
+# RUN: env SYM64_THRESHOLD=100 llvm-lib -out:%t.lib %t01234567890234567789.obj
+# RUN: llvm-readobj %t.lib
+
+--- !COFF
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [  ]
+sections:
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       4
+    SectionData:     ''
+symbols:
+  - Name:            .text
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          0
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        0
+      Number:          1
+...

[efriedma-quic]

Can we represent everything we need in gnu format archives? Like, if we don't emit the symbol map, will that cause problems for a COFF linker?

[cjacek]

I think that should be sufficient in most cases. Before 4fcbf38, we always used the GNU format for COFF files. I rechecked, and MSVC link.exe accepts both GNU and GNU64 formats.

The motivation for using the COFF format was ARM64EC. On ARM64EC, we need it when we have both native and EC object files so that they use different maps. I’ll update the patch to emit an error in this case.

[mstorsjo]

Can we check some aspect of the output here, instead of just requiring that llvm-readobj exits successfully?

[cjacek]

I changed it to check archive map with llvm-nm instead.

[nico]

Thanks!

If link.exe can handle GNU files, maybe we should default to GNU in LibDriver.cpp unless we're targeting ARM64EC?

But I'm happy with anything that unbreaks things.

[cjacek]

Thanks for reviews.

If link.exe can handle GNU files, maybe we should default to GNU in LibDriver.cpp unless we're targeting ARM64EC?

It would probably be fine in practice, but it would be a small step backward in terms of compatibility, so I’d prefer to stick with this (not a strong opinion, though).