Add flag to ArchiveWriter to test GNU64 format more efficiently

[jakehehrlich]

Summary

Even with the sparse file optimizations the SYM64 test can still be painfully slow. This unnecessarily slows down devs. It's critical that we test that the switch to the SYM64 format occurs at 4GB but there isn't any better of a way to fake the size of the file than sparse files. This change introduces a flag that allows the cutoff to be arbitrarily set to whatever power of two is desired. The flag is hidden as it really isn't meant to be used outside this one test. This is unfortunate but appears necessary, at least until the average hard drive is much faster.

The changes to the test require some explanation. Prior to this change we knew that the SYM64 format was being used because the file was simply too large to have validly handled this case if the SYM64 format were not used. To ensure that the SYM64 format is still being used I am grepping the file for "SYM64". Without changing the filename however this would be pointless because "SYM64" would occur in the file either way. So the filename of the test is also changed in order to avoid this issue.

[ruiu]

include/llvm/Object/ArchiveWriter.h

41–45 ↗

(On Diff #124857)

Instead of adding a new parameter, can't you just pass K_GNU64 as a kind?

tools/llvm-ar/llvm-ar.cpp

102 ↗

(On Diff #124857)

and change this to ForceGnu64 or something.

[pcc]

tools/llvm-ar/llvm-ar.cpp

102 ↗

(On Diff #124857)

If this flag is only meant to be used in one test, perhaps it would be better to move it into ArchiveWriter.cpp? That way, we can avoid extending the interface to the archive writer.

[jakehehrlich]

1. Removed the flag from llvm-ar. I didn't realize we could do that but it makes sense. Good idea.
2. Added a default value to the flag so that we don't need any code to handle that.

[jakehehrlich]

@Rui, Just adding a flag to force SYM64 only tests half of this code and ignores the code that detects the shift so it isn't really a good test if we force the particular format.

[ruiu]

lib/Object/ArchiveWriter.cpp

38–39 ↗

(On Diff #125024)

I think it improves code readability a bit if you rename this Sym64Threshold or something and make it have a value of 1<<32 instead of 32.

[pcc]

lib/Object/ArchiveWriter.cpp

490 ↗

(On Diff #125024)

Do you need the .getValue() part? There should be an implicit conversion from the cl::opt to the value type (i.e. int).

[jakehehrlich]

1. I agree, the name I gave the flag was ugly. I prefer Sym64Threshold, so I made that change.
2. Added the comment
3. Removed getValue()

[ruiu]

LGTM

llvm/trunk/lib/Object/ArchiveWriter.cpp

 #if !defined(_MSC_VER) && !defined(__MINGW32__)
 #include <unistd.h>
 #else
 #include <io.h>
 #endif
 
 using namespace llvm;
 
+// The SYM64 format is used when an archive's member offsets are larger than
+// 32-bits can hold. The need for this shift in format is detected by
+// writeArchive. To test this we need to generate a file with a member that has
+// an offset larger than 32-bits but this demands a very slow test. To speed
+// the test up we use this flag to pretend like the cutoff happens before
+// 32-bits and instead happens at some much smaller value.
+static cl::opt<int> Sym64Threshold("sym64-threshold", cl::Hidden,
+                                   cl::init(32));
+
 NewArchiveMember::NewArchiveMember(MemoryBufferRef BufRef)
     : Buf(MemoryBuffer::getMemBuffer(BufRef, false)),
       MemberName(BufRef.getBufferIdentifier()) {}
 
 Expected<NewArchiveMember>
 NewArchiveMember::getOldMember(const object::Archive::Child &OldMember,
                                bool Deterministic) {
   Expected<llvm::MemoryBufferRef> BufOrErr = OldMember.getMemoryBufferRef();
for (const auto& M : Data) {
       // Account for the size of each part associated with the member.
       MaxOffset += M.Header.size() + M.Data.size() + M.Padding.size();
       // We assume 32-bit symbols to see if 32-bit symbols are possible or not.
       MaxOffset += M.Symbols.size() * 4;
     }
     // If LastOffset isn't going to fit in a 32-bit varible we need 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 (LastOffset >> 32 != 0)
+    if (LastOffset >= (1ULL << Sym64Threshold))
       Kind = object::Archive::K_GNU64;
   }
 
   Expected<sys::fs::TempFile> Temp =
       sys::fs::TempFile::create(ArcName + ".temp-archive-%%%%%%%.a");
   if (!Temp)
     return Temp.takeError();
 

llvm/trunk/test/Object/archive-GNU64-write.test

+# REQUIRES: llvm-64-bits
+# REQUIRES: system-linux
+
+# RUN: yaml2obj %s > %t
+# RUN: dd if=%t of=%t bs=1 count=0 seek=1M
+# RUN: rm -f %t.lib
+# RUN: cp %t %t2
+# RUN: llvm-ar -sym64-threshold=19 cr %t.lib %t %t2 %p/Inputs/trivial-object-test.elf-x86-64
+# RUN: llvm-nm --print-armap %t.lib | FileCheck %s
+# RUN: grep SYM64 %t.lib
+
+# Delete temp files. They are too large.
+# RUN: rm -f %t %t2 %t.lib
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .data
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    AddressAlign:    0x0000000000000001
+    Content:         "00"
+    Size:            32
+
+# CHECK:      Archive map
+# CHECK-NEXT: main in trivial-object-test.elf-x86-64
+
+# CHECK:    archive-GNU64-write.test.tmp:
+
+# CHECK:    archive-GNU64-write.test.tmp2:
+
+# CHECK:    trivial-object-test.elf-x86-64:
+# CHECK-NEXT:                     U SomeOtherFunction
+# CHECK-NEXT:    0000000000000000 T main
+# CHECK-NEXT:                     U puts

llvm/trunk/test/Object/archive-SYM64-write.test

-# REQUIRES: llvm-64-bits
-# REQUIRES: system-linux
-
-# RUN: yaml2obj %s > %t
-# RUN: dd if=%t of=%t bs=1 count=0 seek=2200M
-# RUN: rm -f %t.lib
-# RUN: cp %t %t2
-# RUN: llvm-ar cr %t.lib %t %t2 %p/Inputs/trivial-object-test.elf-x86-64
-# RUN: llvm-nm --print-armap %t.lib | FileCheck %s
-
-# Delete temp files. They are too large.
-# RUN: rm -f %t %t2 %t.lib
-
-!ELF
-FileHeader:
-  Class:           ELFCLASS64
-  Data:            ELFDATA2LSB
-  Type:            ET_EXEC
-  Machine:         EM_X86_64
-Sections:
-  - Name:            .data
-    Type:            SHT_PROGBITS
-    Flags:           [ SHF_ALLOC ]
-    AddressAlign:    0x0000000000000001
-    Content:         "00"
-    Size:            32
-
-# CHECK:      Archive map
-# CHECK-NEXT: main in trivial-object-test.elf-x86-64
-
-# CHECK:    archive-SYM64-write.test.tmp:
-
-# CHECK:    archive-SYM64-write.test.tmp2:
-
-# CHECK:    trivial-object-test.elf-x86-64:
-# CHECK-NEXT:                     U SomeOtherFunction
-# CHECK-NEXT:    0000000000000000 T main
-# CHECK-NEXT:                     U puts