[PATCH] D89891: [llvm-ar][Object] Fix detection of need for 64-bit archive symbol tables

[Andrew Ng via Phabricator via llvm-commits]

andrewng added a comment.

In D89891#2345374 <https://reviews.llvm.org/D89891#2345374>, @MaskRay wrote:

> I understand that a test may be problematic because it requires 4+GiB output. Do you have a script to craft such a test case so that people working on llvm-ar can run it on demand? The test can have `UNSUPPORTED: *`.

No, I don't have such a script. I actually had a use case that was showing the issue. I've updated the existing testing to be more precise to provide coverage.



================
Comment at: llvm/lib/Object/ArchiveWriter.cpp:610
+    raw_svector_ostream SymtabHeader(SymtabHeaderBuf);
+    writeSymbolTableHeader(SymtabHeader, Kind, Deterministic, SymtabSize);
+
----------------
grimar wrote:
> I was a bit confused when saw `writeSymbolTableHeader` here.
> `Kind` can be changed below:
> 
> ```
>     if (LastOffset >= (1ULL << Sym64Threshold)) {
>       if (Kind == object::Archive::K_DARWIN)
>         Kind = object::Archive::K_DARWIN64;
>       else
>         Kind = object::Archive::K_GNU64;
>     }
> ```
> 
> I think would be more natural to call `writeSymbolTableHeader` after last possible modification of `Kind`, i.e. after this code block. Is it possible?
This call to `writeSymbolTableHeader` is purely used to determine the size of the header and is not a write to the output which happens later.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89891/new/

https://reviews.llvm.org/D89891