This diff adds support for universal binaries (https://en.wikipedia.org/wiki/Universal_binary) to llvm-objcopy .
Just a few minor comments from me, but I don't really follow the logic as always, so this will need others to comment on it.
You could also just inline this text directly in the file and feed %s as the input to llvm-as.
Here and the other consumeError site probably deserve comments explaining why it's okay to throw away the error reported.
Preferred error message style starts with a lower-case letter.
Is there anything that allows us to uniquely identify which slice is the problematic one (e.g. a member name or similar)? If so, it might be helpful to report it as part of the message.
Not sure what's expected here.
Sorry for the confusion. Perhaps just changing the comment slightly to something like the following would clarify things:
// The methods getAsArchive, getAsObjectFile, getAsIRObject of the class // ObjectForArch return an Error in case of the type mismatch. We need to check each // in turn to see what kind of file this is, so ignore errors produced along the way.
(The key bit being that this answers the question why we shouldn't report the error for ArOrErr - because we also need to see if it is an object file)
LGTM barring the comment about more tests.
Is the expectation that this constructor will be used when the Archive & is from an existing universal binary, and it's therefore okay to bypass all the checks in the Slice::create overload that takes an Archive &? If so, we should explicitly comment that (preferably as a Doxygen comment in the header), otherwise people might be confused in the future about when they want to use which function.
All of these tests are just for copies. We should also have some tests for performing actual operations (e.g. stripping) to show that we correctly apply the operation to each slice.
You probably want an rm -f %t.archive.i386 before this, otherwise I believe ar will append to any existing file with that name (e.g. a leftover from a previous test run).
This is actually invalid, but I guess it doesn't hurt to support it. From the Apple man page for libtool:
I'd up both of these to 2, since I imagine that's the most common universal binary size.
yeah, this constructor takes cputype, cpusubtype, and align and doesn't infer them from the archive members (e.g. both lipo/llvm-lipo allow to specify a custom value for the alignment). The main use case is creating a Slice from an archive coming from a universal binary.
Unfortunately I see this change causing ASan failures in llvm-objcopy/MachO/strip-all.test:
ERROR: AddressSanitizer: stack-use-after-scope on address 0x7fed6fb0f550 at pc 0x55e8dd15e257 bp 0x7ffe7c565d90 sp 0x7ffe7c565558 READ of size 6 at 0x7fed6fb0f550 thread T0 #0 0x55e8dd15e256 in __asan_memcpy llvm-project/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:22:3 #1 0x55e8de076a4b in CopyStringRef(char*, llvm::StringRef) llvm-project/llvm/lib/Support/MemoryBuffer.cpp:61:5 #2 0x55e8de07678e in llvm::WritableMemoryBuffer::getNewUninitMemBuffer(unsigned long, llvm::Twine const&) llvm-project/llvm/lib/Support/MemoryBuffer.cpp:303:3 #3 0x55e8de076d3e in llvm::WritableMemoryBuffer::getNewMemBuffer(unsigned long, llvm::Twine const&) llvm-project/llvm/lib/Support/MemoryBuffer.cpp:315:13 #4 0x55e8dd178c27 in llvm::objcopy::MemBuffer::allocate(unsigned long) llvm-project/llvm/tools/llvm-objcopy/Buffer.cpp:64:9 #5 0x55e8dd379b13 in llvm::objcopy::macho::MachOWriter::write() llvm-project/llvm/tools/llvm-objcopy/MachO/MachOWriter.cpp:502:19 #6 0x55e8dd33b80e in llvm::objcopy::macho::executeObjcopyOnBinary(llvm::objcopy::CopyConfig const&, llvm::object::MachOObjectFile&, llvm::objcopy::Buffer&) llvm-project/llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp:390:17 #7 0x55e8dd33d216 in llvm::objcopy::macho::executeObjcopyOnMachOUniversalBinary(llvm::objcopy::CopyConfig&, llvm::object::MachOUniversalBinary const&, llvm::objcopy::Buffer&) llvm-project/llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp:439:19 #8 0x55e8dd38bcf2 in executeObjcopyOnBinary(llvm::objcopy::CopyConfig&, llvm::object::Binary&, llvm::objcopy::Buffer&) llvm-project/llvm/tools/llvm-objcopy/llvm-objcopy.cpp:150:12 #9 0x55e8dd38d4bc in executeObjcopy(llvm::objcopy::CopyConfig&) llvm-project/llvm/tools/llvm-objcopy/llvm-objcopy.cpp:290:21