[PATCH] D36812: Add support for writing 64-bit symbol tables for archives when offsets become too large for 32-bit

[Rafael Avila de Espindola via llvm-commits]

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

Please commit a patch with just the rename and rebase.

> +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;
> +}


This helper can be static. Please make it so.
This is also a nice independent cleanup, so please commit it first and
rebase.

> -  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

This is quite an unfortunate code duplication.

The code already has some complications (MemberReferenceOffset + seek
back) to avoid having to pre compute offsets. Now it has both issues: it
is pre computing offsets and still has the original complications.

I don't think there is a reasonable way to decide if we need a 64 bit
symbol table and still avoid doing an extra pass over the archive
members.

That means we should make the most of that pass and replace the existing
complications.

An archive file looks like

"!<arch>\n"
symbol table
string table
real member 1
real member 2
...
real member n

We can align the start of the first real member to 8. That means that we
should be able in one pass over the members:

* Compute all the member headers and StringSave them somewhere.
* Compute a std::vector<StringRef>, with both the member headers and
  member data.
* Find the offset of each member from the start of the first member
  header.

With that we will then be able to decide if we need a 64 bit table or
not and write the archive in one pass.

It is my fault that I pushed for so long with the current design. Please
commit the K_GNU64 rename and isArchiveSymbol helper and I will take
care of refactoring the code to avoid the current complications.

Cheers,
Rafael