Add support for writing 64-bit symbol tables for archives when offsets become too large for 32-bit

[jakehehrlich]

Summary

This should fix https://bugs.llvm.org//show_bug.cgi?id=34189

This change makes it so that if writing a K_GNU style archive, you need to output a > 32-bit offset it should output in K_GNU64 style instead.

[mcgrathr]

You have a bunch of changes here to support individual members >=4G, but I'm not sure there is any need for that.
The bug is just about the whole archive being >=4G so that the member offsets need to be 64 bits.
If you want to support members >=4G I think that can be a separate change since it's not related to the bug.

[emaste]

include/llvm/Object/Archive.h

51–52 ↗

(On Diff #111438)

The comment still references a 32-bit limit.

[phosek]

lib/Object/ArchiveWriter.cpp

506

You should use static_cast here.

[phosek]

lib/Object/ArchiveWriter.cpp

152

This function is exactly the same as print32 except for the Val type, couldn't you use a single templated function and avoid the printNBits altogether?

[jakehehrlich]

1. Removed changes to allow for > 4GB members as Roland recommended
2. This had the consequence of nullifying the need for the static cast and the comment change
3. I merged print32 and print64 into a single file but I did not get rid of printNBits. The decision to switch to outputting a 64-bit symbol table is made dynamically so there will need to be an if statement that calls one template instance or the other of any template that might merge the behaviors. I see 3 possible options

1. use printNBits as I am
2. tempalte writeSymbolTable and replace the one call to printNBits in writeArchive with an if statement that decides which print to call. This would also need an if statement to decide which writeSymbolTable to call.
3. template writeSymbolTable and make a template that does what writeArchive does, call it writeArchiveT. writeArchive will the decide which type to instantiate writeArchiveT with while writeArchiveT will do the work that writeArchive does now.

option 1) seems the least invasive to me personally.

[jakehehrlich]

ping

[jakehehrlich]

I agree with Rafael's comment. K_MIPS64 is now K_GNU64 and printNBits now uses Kind to determine the size.

[jakehehrlich]

1. I committed the rename and rebased so that this diff now assumes that K_MIPS64 is now K_GNU64
2. I made all the helpers static

[• rafael]

With r314844 in place it should now be much easier to check if a 64 bit symbol table is needed or not.

[jakehehrlich]

I updated this to use the new changes that rafael made. Much clearner. Thanks Rafael!

[jakehehrlich]

This change adds --print-armap to the test. After https://reviews.llvm.org/D39379 lands this test will work. If the wrong index is used for "main" then the correct file is unlikely to show up making so this test should check that "main" is being looked up in the correct place.

lib/Object/ArchiveWriter.cpp

Expected<NewArchiveMember> NewArchiveMember::getFile(StringRef FileName,
   return std::move(M);
 }
 
 template <typename T>
 static void printWithSpacePadding(raw_fd_ostream &OS, T Data, unsigned Size,
                                   bool MayTruncate = false) {
   uint64_t OldPos = OS.tell();
   OS << Data;
-  unsigned SizeSoFar = OS.tell() - OldPos;
+  uint64_t SizeSoFar = OS.tell() - OldPos;
   if (Size > SizeSoFar) {
     OS.indent(Size - SizeSoFar);
   } else if (Size < SizeSoFar) {
     assert(MayTruncate && "Data doesn't fit in Size");
     // Some of the data this is used for (like UID) can be larger than the
     // space available in the archive format. Truncate in that case.
     OS.seek(OldPos + Size);
   }
 }
 
 static bool isBSDLike(object::Archive::Kind Kind) {
   switch (Kind) {
   case object::Archive::K_GNU:
+  case object::Archive::K_GNU64:
     return false;
   case object::Archive::K_BSD:
   case object::Archive::K_DARWIN:
     return true;
-  case object::Archive::K_GNU64:
   case object::Archive::K_DARWIN64:
   case object::Archive::K_COFF:
     break;
   }
   llvm_unreachable("not supported for writting");
 }
 
-static void print32(raw_ostream &Out, object::Archive::Kind Kind,
-                    uint32_t Val) {
+template <class T>
+static void print(raw_ostream &Out, object::Archive::Kind Kind, T Val) {
   if (isBSDLike(Kind))
     support::endian::Writer<support::little>(Out).write(Val);
   else
     support::endian::Writer<support::big>(Out).write(Val);
 }
 
+static bool is64BitKind(object::Archive::Kind Kind) {
+  switch (Kind) {
+  case object::Archive::K_GNU:
+  case object::Archive::K_BSD:
+  case object::Archive::K_DARWIN:
+  case object::Archive::K_COFF:
+    return false;
+  case object::Archive::K_DARWIN64:
+  case object::Archive::K_GNU64:
+    return true;
+  }
+  llvm_unreachable("not supported for writting");
+}
+
+static void printNBits(raw_ostream &Out, object::Archive::Kind Kind,
+                       uint64_t Val) {
+  if (is64BitKind(Kind))
+    print<uint64_t>(Out, Kind, Val);
+  else
+    print<uint32_t>(Out, Kind, Val);
+}
+
 static void printRestOfMemberHeader(
     raw_fd_ostream &Out, const sys::TimePoint<std::chrono::seconds> &ModTime,
     unsigned UID, unsigned GID, unsigned Perms, unsigned Size) {
   printWithSpacePadding(Out, sys::toTimeT(ModTime), 12);
   printWithSpacePadding(Out, UID, 6, true);
   printWithSpacePadding(Out, GID, 6, true);
   printWithSpacePadding(Out, format("%o", Perms), 8);
   printWithSpacePadding(Out, Size, 10);

 static sys::TimePoint<std::chrono::seconds> now(bool Deterministic) {
   using namespace std::chrono;
 
   if (!Deterministic)
     return time_point_cast<seconds>(system_clock::now());
   return sys::TimePoint<seconds>();
 }
 
+static bool isArchiveSymbol(const object::BasicSymbolRef &S) {
+  uint32_t Symflags = S.getFlags();
+  if (Symflags & object::SymbolRef::SF_FormatSpecific)
+    return false;
+  if (!(Symflags & object::SymbolRef::SF_Global))
+    return false;
+  if (Symflags & object::SymbolRef::SF_Undefined &&
+      !(Symflags & object::SymbolRef::SF_Indirect))
+    return false;
+  return true;
+}
+
 // Returns the offset of the first reference to a member offset.
-static ErrorOr<unsigned>
+static ErrorOr<uint64_t>
 writeSymbolTable(raw_fd_ostream &Out, object::Archive::Kind Kind,
                  ArrayRef<NewArchiveMember> Members,
                  std::vector<unsigned> &MemberOffsetRefs, bool Deterministic) {
-  unsigned HeaderStartOffset = 0;
-  unsigned BodyStartOffset = 0;
+  uint64_t HeaderStartOffset = 0;
+  uint64_t BodyStartOffset = 0;
   SmallString<128> NameBuf;
   raw_svector_ostream NameOS(NameBuf);
   LLVMContext Context;
   for (unsigned MemberNum = 0, N = Members.size(); MemberNum < N; ++MemberNum) {
     MemoryBufferRef MemberBuffer = Members[MemberNum].Buf->getMemBufferRef();
     Expected<std::unique_ptr<object::SymbolicFile>> ObjOrErr =
         object::SymbolicFile::createSymbolicFile(
             MemberBuffer, llvm::file_magic::unknown, &Context);
     if (!ObjOrErr) {
       // FIXME: check only for "not an object file" errors.
       consumeError(ObjOrErr.takeError());
       continue;
     }
     object::SymbolicFile &Obj = *ObjOrErr.get();
 
     if (!HeaderStartOffset) {
       HeaderStartOffset = Out.tell();
       if (isBSDLike(Kind))
         printBSDMemberHeader(Out, "__.SYMDEF", now(Deterministic), 0, 0, 0, 0);
+      else if (is64BitKind(Kind))
+        printGNUSmallMemberHeader(Out, "/SYM64", now(Deterministic), 0, 0, 0,
+                                  0);
       else
         printGNUSmallMemberHeader(Out, "", now(Deterministic), 0, 0, 0, 0);
       BodyStartOffset = Out.tell();
-      print32(Out, Kind, 0); // number of entries or bytes
+      printNBits(Out, Kind, 0); // number of entries or bytes
     }
 
     for (const object::BasicSymbolRef &S : Obj.symbols()) {
-      uint32_t Symflags = S.getFlags();
-      if (Symflags & object::SymbolRef::SF_FormatSpecific)
-        continue;
-      if (!(Symflags & object::SymbolRef::SF_Global))
-        continue;
-      if (Symflags & object::SymbolRef::SF_Undefined &&
-          !(Symflags & object::SymbolRef::SF_Indirect))
+      if (!isArchiveSymbol(S))
         continue;
 
-      unsigned NameOffset = NameOS.tell();
+      uint64_t NameOffset = NameOS.tell();
       if (auto EC = S.printName(NameOS))
         return EC;
       NameOS << '\0';
       MemberOffsetRefs.push_back(MemberNum);
       if (isBSDLike(Kind))
-        print32(Out, Kind, NameOffset);
-      print32(Out, Kind, 0); // member offset
+        print<uint32_t>(Out, Kind, NameOffset);
+      printNBits(Out, Kind, 0); // member offset
     }
   }
 
   if (HeaderStartOffset == 0)
     return 0;
 
   // ld64 prefers the cctools type archive which pads its string table to a
   // boundary of sizeof(int32_t).
   if (isBSDLike(Kind))
     for (unsigned P = OffsetToAlignment(NameOS.tell(), sizeof(int32_t)); P--;)
       NameOS << '\0';
 
   StringRef StringTable = NameOS.str();
   if (isBSDLike(Kind))
-    print32(Out, Kind, StringTable.size()); // byte count of the string table
+    print<uint32_t>(Out, Kind,
+                    StringTable.size()); // byte count of the string table
   Out << StringTable;
   // If there are no symbols, emit an empty symbol table, to satisfy Solaris
   // tools, older versions of which expect a symbol table in a non-empty
   // archive, regardless of whether there are any symbols in it.
   if (StringTable.size() == 0)
-    print32(Out, Kind, 0);
+    print<uint32_t>(Out, Kind, 0);
 
   // ld64 requires the next member header to start at an offset that is
   // 4 bytes aligned.
   unsigned Pad = OffsetToAlignment(Out.tell(), 4);
   while (Pad--)
     Out.write(uint8_t(0));
 
   // Patch up the size of the symbol table now that we know how big it is.
-  unsigned Pos = Out.tell();
+  uint64_t Pos = Out.tell();
   const unsigned MemberHeaderSize = 60;
   Out.seek(HeaderStartOffset + 48); // offset of the size field.
   printWithSpacePadding(Out, Pos - MemberHeaderSize - HeaderStartOffset, 10);
 
   // Patch up the number of symbols.
   Out.seek(BodyStartOffset);
   unsigned NumSyms = MemberOffsetRefs.size();
   if (isBSDLike(Kind))
-    print32(Out, Kind, NumSyms * 8);
+    print<uint32_t>(Out, Kind, NumSyms * 8);
   else
-    print32(Out, Kind, NumSyms);
+    printNBits(Out, Kind, NumSyms);
 
   Out.seek(Pos);
-  return BodyStartOffset + 4;
+  return BodyStartOffset + (is64BitKind(Kind) ? 8 : 4);
 }
 
 std::error_code
 llvm::writeArchive(StringRef ArcName, std::vector<NewArchiveMember> &NewMembers,
                    bool WriteSymtab, object::Archive::Kind Kind,
                    bool Deterministic, bool Thin,
                    std::unique_ptr<MemoryBuffer> OldArchiveBuf) {
   assert((!Thin || !isBSDLike(Kind)) && "Only the gnu format has a thin mode");
   SmallString<128> TmpArchive;
   int TmpArchiveFD;
   if (auto EC = sys::fs::createUniqueFile(ArcName + ".temp-archive-%%%%%%%.a",
                                           TmpArchiveFD, TmpArchive))
     return EC;
 
   tool_output_file Output(TmpArchive, TmpArchiveFD);
   raw_fd_ostream &Out = Output.os();
   if (Thin)
     Out << "!<thin>\n";
   else
     Out << "!<arch>\n";
 
   std::vector<unsigned> MemberOffsetRefs;
 
-  unsigned MemberReferenceOffset = 0;
+  uint64_t MemberReferenceOffset = 0;
   if (WriteSymtab) {
-    ErrorOr<unsigned> MemberReferenceOffsetOrErr = writeSymbolTable(
+    // check if we need to switch to 64-bit by computing size from here
+    uint64_t MaxOffset = MemberReferenceOffset;
+    uint64_t LastOffset = 0;
+    LLVMContext Context;
+    // Compute the size of each member plus the size to be added to the symbols
+    for (const auto &M : NewMembers) {
+      MemoryBufferRef MemberBuffer = M.Buf->getMemBufferRef();
+      // The offset of this is the previous
+      LastOffset = MaxOffset;
+      // Add size of header
+      MaxOffset += 60;
+      // Add the size of the buffer
+      MaxOffset += M.Buf->getBufferSize();
+      // Add the size of padding
+      if (Kind == object::Archive::K_DARWIN)
+        MaxOffset += OffsetToAlignment(M.Buf->getBufferSize(), 8);
+      // Add the size added at the end in order to get an even location
+      MaxOffset += MaxOffset & 1;
+      // Now we need to add space for each symbol
+      Expected<std::unique_ptr<object::SymbolicFile>> ObjOrErr =
+          object::SymbolicFile::createSymbolicFile(
+              MemberBuffer, llvm::file_magic::unknown, &Context);
+      if (!ObjOrErr) {
+        // FIXME: check only for "not an object file" errors.
+        consumeError(ObjOrErr.takeError());
+        continue;
+      }
+      object::SymbolicFile &Obj = *ObjOrErr.get();
+      for (const object::BasicSymbolRef &S : Obj.symbols()) {
+        if (!isArchiveSymbol(S))
+          continue;
+        // Add space for the symbol offset
+        MaxOffset += 4;
+        // Add space for the string in the string table
+        SmallString<128> NameBuf;
+        raw_svector_ostream NameLengthBuf(NameBuf);
+        S.printName(NameLengthBuf);
+        MaxOffset += NameLengthBuf.tell();
+      }
+    }
+    if (LastOffset >> 32 != 0)
+      Kind = object::Archive::K_GNU64;
+    // Store the location in case we have to do this all over again
+    ErrorOr<uint64_t> MemberReferenceOffsetOrErr = writeSymbolTable(
         Out, Kind, NewMembers, MemberOffsetRefs, Deterministic);
     if (auto EC = MemberReferenceOffsetOrErr.getError())
       return EC;
     MemberReferenceOffset = MemberReferenceOffsetOrErr.get();
   }
 
   std::vector<unsigned> StringMapIndexes;
   if (!isBSDLike(Kind))
     writeStringTable(Out, ArcName, NewMembers, StringMapIndexes, Thin);
 
   std::vector<unsigned>::iterator StringMapIndexIter = StringMapIndexes.begin();
-  std::vector<unsigned> MemberOffset;
+  std::vector<uint64_t> MemberOffset;
   for (const NewArchiveMember &M : NewMembers) {
     MemoryBufferRef File = M.Buf->getMemBufferRef();
-    unsigned Padding = 0;
+    uint64_t Padding = 0;
 
-    unsigned Pos = Out.tell();
+    uint64_t Pos = Out.tell();
     MemberOffset.push_back(Pos);
 
     // ld64 expects the members to be 8-byte aligned for 64-bit content and at
     // least 4-byte aligned for 32-bit content.  Opt for the larger encoding
     // uniformly.  This matches the behaviour with cctools and ensures that ld64
     // is happy with archives that we generate.
     if (Kind == object::Archive::K_DARWIN)
       Padding = OffsetToAlignment(M.Buf->getBufferSize(), 8);
 
     printMemberHeader(Out, Kind, Thin, M.MemberName, StringMapIndexIter,
                       M.ModTime, M.UID, M.GID, M.Perms,
                       M.Buf->getBufferSize() + Padding);
 
     if (!Thin)
       Out << File.getBuffer();
 
     while (Padding--)
       Out << '\n';
     if (Out.tell() % 2)
       Out << '\n';
   }
 
   if (MemberReferenceOffset) {
     Out.seek(MemberReferenceOffset);
     for (unsigned MemberNum : MemberOffsetRefs) {
       if (isBSDLike(Kind))
         Out.seek(Out.tell() + 4); // skip over the string offset
-      print32(Out, Kind, MemberOffset[MemberNum]);
+      printNBits(Out, Kind, MemberOffset[MemberNum]);
     }
   }
 
   Output.keep();
   Out.close();
 
   // At this point, we no longer need whatever backing memory
   // was used to generate the NewMembers. On Windows, this buffer

test/Object/archive-SYM64-write.test

+# RUN: yaml2obj %s > %t
+# RUN: rm -f %t.lib
+# RUN: llvm-ar cr %t.lib %t
+# RUN: mv %t %t2
+# RUN: llvm-ar cr %t.lib %t2 %p/Inputs/trivial-object-test.elf-x86-64
+# RUN: llvm-nm %t.lib | FileCheck %s
+
+!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:            2147483648
+
+# 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