[PATCH] D84858: [llvm-libtool-darwin] Refactor ArchiveWriter

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai added a comment.

To add a bunch of context, one functionality we're adding to llvm-libtool-darwin is to produce universal binary outputs. For those who aren't familiar, universal binaries (also known as fat binaries) allow a single binary to have slices for multiple architectures. In libtool's case, if you pass it object files of different architectures as input, it'll group each architecture's object files together into a single-architecture static library, and then group those single-architecture static libraries into a universal static library. The functionality to create the universal static library exists in llvm-lipo, and we're refactoring that out into libObject in D84662 <https://reviews.llvm.org/D84662> to allow llvm-libtool-darwin to use it as well.

What we'd ideally want is to be able to construct an `object::Archive` in memory (in libtool), and then pass that `Archive` along to `writeUniversalBinary`. Unfortunately, the libObject facilities for reading and writing archives seem to be completely independent. Reading uses `Archive`, `Archive::Child`, etc., whereas writing uses `NewArchiveMember` and `writeArchive`. There's no way, as far as I can see, to construct an `Archive` by specifying its members; you can only construct one from a block of memory. Similarly, there's no way to serialize an `Archive` out to a file.

It'd perhaps be nice to unify the archive reading/writing interfaces, but there might be good reasons for the current split, and doing so would also be an **extensive** change. Instead, what we're doing here is adding the ability for `writeArchive` to give us back a MemoryBuffer (instead of writing to a temp file), and then parsing that MemoryBuffer as an `Archive`. That's a bit wasteful (less so if we eliminate the temp file and just keep things in memory), but given the completely separate reading/writing interfaces, it was the best solution we could think of. We're definitely open to better ideas if people have them!



================
Comment at: llvm/lib/Object/ArchiveWriter.cpp:673-674
+
+  ErrorOr<std::unique_ptr<MemoryBuffer>> Ret =
+      MemoryBuffer::getOpenFile(Temp->FD, ArcName, -1);
+  if (std::error_code EC = Ret.getError())
----------------
sameerarora101 wrote:
> My current approach involves factoring out the common code of writing the archive to a temporary FD. Now, in case of `writeArchive`, I simply write the temporary file to disk by invoking `Temp->keep(ArcName)`. However, for `writeArchiveBuffer`, I read the archive again in a `MemoryBuffer` using 
> ```
>   ErrorOr<std::unique_ptr<MemoryBuffer>> Ret =
>       MemoryBuffer::getOpenFile(Temp->FD, ArcName, -1);
> ```
> This memory buffer is then put in `ReturnBuffer` (which is the buffer allocated by the caller to receive the archive buffer).
> 
> Please let me know if there is a better approach to get the archive in a buffer than this current roundtrip method(writing to a FD and then reading back into a MemBuf).  Thanks!
We might be able to do something with raw_svector_ostream. I'll experiment a bit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84858