Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline

Differential D85740 Diff 287823 llvm/lib/Object/MachOUniversalWriter.cpp

Changeset View

Standalone View

View Options

llvm/lib/Object/MachOUniversalWriter.cpp

//===- MachOUniversalWriter.cpp - MachO universal binary writer---*- C++-*-===// //===- MachOUniversalWriter.cpp - MachO universal binary writer---*- C++-*-===//
// //
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information. // See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
// //
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
// //
// Defines the Slice class and writeUniversalBinary function for writing a MachO // Defines the Slice class and writeUniversalBinary function for writing a MachO
// universal binary file. // universal binary file.
// //
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
#include "llvm/Object/MachOUniversalWriter.h" #include "llvm/Object/MachOUniversalWriter.h"
#include "llvm/ADT/Triple.h"
#include "llvm/Object/Archive.h" #include "llvm/Object/Archive.h"
#include "llvm/Object/Binary.h" #include "llvm/Object/Binary.h"
#include "llvm/Object/Error.h" #include "llvm/Object/Error.h"
#include "llvm/Object/IRObjectFile.h"
#include "llvm/Object/MachO.h" #include "llvm/Object/MachO.h"
#include "llvm/Object/MachOUniversal.h" #include "llvm/Object/MachOUniversal.h"
#include "llvm/Support/FileOutputBuffer.h" #include "llvm/Support/FileOutputBuffer.h"
using namespace llvm; using namespace llvm;
using namespace object; using namespace object;
// For compatibility with cctools lipo, a file's alignment is calculated as the // For compatibility with cctools lipo, a file's alignment is calculated as the
▲ Show 20 Lines • Show All 48 Lines • ▼ Show 20 Lines
} }
Slice::Slice(const MachOObjectFile &O, uint32_t Align) Slice::Slice(const MachOObjectFile &O, uint32_t Align)
: B(&O), CPUType(O.getHeader().cputype), : B(&O), CPUType(O.getHeader().cputype),
CPUSubType(O.getHeader().cpusubtype), CPUSubType(O.getHeader().cpusubtype),
ArchName(std::string(O.getArchTriple().getArchName())), ArchName(std::string(O.getArchTriple().getArchName())),
P2Alignment(Align) {} P2Alignment(Align) {}
Slice::Slice(const IRObjectFile *IRO, uint32_t CPUType, uint32_t CPUSubType,
std::string ArchName, uint32_t Align)
: B(IRO), CPUType(CPUType), CPUSubType(CPUSubType),
ArchName(std::move(ArchName)), P2Alignment(Align) {}
Slice::Slice(const MachOObjectFile &O) : Slice(O, calculateAlignment(O)) {} Slice::Slice(const MachOObjectFile &O) : Slice(O, calculateAlignment(O)) {}
Expected<Slice> Slice::create(const Archive *A) { using MachoCPUTy = std::pair<unsigned, unsigned>;
alexander-shaposhnikovUnsubmitted
Done
ReplyInline Actions

nit: a blank line between 90 and 91 would not hurt

alexander-shaposhnikov: nit: a blank line between 90 and 91 would not hurt
static Expected<MachoCPUTy> getMachoCPUFromTriple(Triple TT) {
auto CPU = std::make_pair(MachO::getCPUType(TT), MachO::getCPUSubType(TT));
if (!CPU.first) {
return CPU.first.takeError();
}
if (!CPU.second) {
return CPU.second.takeError();
}
return std::make_pair(*CPU.first, *CPU.second);
}
static Expected<MachoCPUTy> getMachoCPUFromTriple(StringRef TT) {
return getMachoCPUFromTriple(Triple{TT});
}
Expected<Slice> Slice::create(const Archive *A, LLVMContext *LLVMCtx) {
Error Err = Error::success(); Error Err = Error::success();
std::unique_ptr<MachOObjectFile> FO = nullptr; std::unique_ptr<MachOObjectFile> MFO = nullptr;
std::unique_ptr<IRObjectFile> IRFO = nullptr;
for (const Archive::Child &Child : A->children(Err)) { for (const Archive::Child &Child : A->children(Err)) {
Expected<std::unique_ptr<Binary>> ChildOrErr = Child.getAsBinary(); Expected<std::unique_ptr<Binary>> ChildOrErr = Child.getAsBinary(LLVMCtx);
if (!ChildOrErr) if (!ChildOrErr)
return createFileError(A->getFileName(), ChildOrErr.takeError()); return createFileError(A->getFileName(), ChildOrErr.takeError());
Binary *Bin = ChildOrErr.get().get(); Binary *Bin = ChildOrErr.get().get();
if (Bin->isMachOUniversalBinary()) if (Bin->isMachOUniversalBinary())
return createStringError(std::errc::invalid_argument, return createStringError(std::errc::invalid_argument,
("archive member " + Bin->getFileName() + ("archive member " + Bin->getFileName() +
" is a fat file (not allowed in an archive)") " is a fat file (not allowed in an archive)")
.str() .str()
.c_str()); .c_str());
if (!Bin->isMachO()) if (Bin->isMachO()) {
MachOObjectFile *O = cast<MachOObjectFile>(Bin);
if (IRFO) {
return createStringError( return createStringError(
std::errc::invalid_argument, std::errc::invalid_argument,
("archive member " + Bin->getFileName() + "archive member %s is a MachO, while previous archive member "
" is not a MachO file (not allowed in an archive)") "%s was an IR LLVM object",
.str() O->getFileName().str().c_str(), IRFO->getFileName().str().c_str());
.c_str()); }
MachOObjectFile *O = cast<MachOObjectFile>(Bin); if (MFO &&
if (FO && std::tie(FO->getHeader().cputype, FO->getHeader().cpusubtype) != std::tie(MFO->getHeader().cputype, MFO->getHeader().cpusubtype) !=
std::tie(O->getHeader().cputype, O->getHeader().cpusubtype)) { std::tie(O->getHeader().cputype, O->getHeader().cpusubtype)) {
alexander-shaposhnikovUnsubmitted
Done
ReplyInline Actions

nit: explicit type

alexander-shaposhnikov: nit: explicit type
return createStringError( return createStringError(
std::errc::invalid_argument, std::errc::invalid_argument,
("archive member " + O->getFileName() + " cputype (" + ("archive member " + O->getFileName() + " cputype (" +
Twine(O->getHeader().cputype) + ") and cpusubtype(" + Twine(O->getHeader().cputype) + ") and cpusubtype(" +
Twine(O->getHeader().cpusubtype) + Twine(O->getHeader().cpusubtype) +
") does not match previous archive members cputype (" + ") does not match previous archive members cputype (" +
Twine(FO->getHeader().cputype) + ") and cpusubtype(" + Twine(MFO->getHeader().cputype) + ") and cpusubtype(" +
Twine(FO->getHeader().cpusubtype) + ") (all members must match) " + Twine(MFO->getHeader().cpusubtype) +
FO->getFileName()) ") (all members must match) " + MFO->getFileName())
.str()
.c_str());
}
if (!MFO) {
ChildOrErr.get().release();
MFO.reset(O);
}
alexander-shaposhnikovUnsubmitted
Done
ReplyInline Actions

not particularly important, but I'd probably convert "if" + "continue" into

if ( ... ) { ... } else if ( ... ) { ... } else { ... }

(it seems to be more readable)

alexander-shaposhnikov: not particularly important, but I'd probably convert "if" + "continue" into if ( ... ) { ...
aguinetAuthorUnsubmitted
Done
ReplyInline Actions

Okay will fix with new diff!

aguinet: Okay will fix with new diff!
} else if (Bin->isIR()) {
IRObjectFile *O = cast<IRObjectFile>(Bin);
if (MFO) {
return createStringError(std::errc::invalid_argument,
"archive member '%s' is an LLVM IR object, "
"while previous archive member "
"'%s' was a MachO",
O->getFileName().str().c_str(),
MFO->getFileName().str().c_str());
}
if (IRFO) {
Expected<MachoCPUTy> CPUO = getMachoCPUFromTriple(O->getTargetTriple());
Expected<MachoCPUTy> CPUFO =
getMachoCPUFromTriple(IRFO->getTargetTriple());
if (!CPUO)
return CPUO.takeError();
if (!CPUFO)
return CPUFO.takeError();
if (*CPUO != *CPUFO) {
return createStringError(
std::errc::invalid_argument,
("archive member " + O->getFileName() + " cputype (" +
Twine(CPUO->first) + ") and cpusubtype(" + Twine(CPUO->second) +
") does not match previous archive members cputype (" +
Twine(CPUFO->first) + ") and cpusubtype(" +
Twine(CPUFO->second) + ") (all members must match) " +
IRFO->getFileName())
.str() .str()
.c_str()); .c_str());
} }
if (!FO) { } else {
ChildOrErr.get().release(); ChildOrErr.get().release();
FO.reset(O); IRFO.reset(O);
} }
} else
return createStringError(std::errc::invalid_argument,
("archive member " + Bin->getFileName() +
" is neither a MachO file or an LLVM IR file "
"(not allowed in an archive)")
.str()
.c_str());
} }
if (Err) if (Err)
return createFileError(A->getFileName(), std::move(Err)); return createFileError(A->getFileName(), std::move(Err));
if (!FO) if (!MFO && !IRFO)
return createStringError( return createStringError(
std::errc::invalid_argument, std::errc::invalid_argument,
("empty archive with no architecture specification: " + ("empty archive with no architecture specification: " +
A->getFileName() + " (can't determine architecture for it)") A->getFileName() + " (can't determine architecture for it)")
.str() .str()
.c_str()); .c_str());
Slice ArchiveSlice = Slice(*(FO.get()), FO->is64Bit() ? 3 : 2); if (MFO) {
Slice ArchiveSlice(*(MFO.get()), MFO->is64Bit() ? 3 : 2);
ArchiveSlice.B = A; ArchiveSlice.B = A;
return ArchiveSlice; return ArchiveSlice;
} }
// For IR objects
Expected<Slice> ArchiveSliceOrErr = Slice::create(IRFO.get(), 0);
if (!ArchiveSliceOrErr)
return createFileError(A->getFileName(), ArchiveSliceOrErr.takeError());
alexander-shaposhnikovUnsubmitted
Done
ReplyInline Actions

nit:

  1. auto -> explicit type (since it's not clear from the right-hand side),
  2. for a single-line "if" braces are not necessary.

(and the same applies to lines 216 - 219)

alexander-shaposhnikov: nit: 1. auto -> explicit type (since it's not clear from the right-hand side), 2. for a single…
aguinetAuthorUnsubmitted
Done
ReplyInline Actions

Ack will fix with new diff

aguinet: Ack will fix with new diff
auto &ArchiveSlice = ArchiveSliceOrErr.get();
alexander-shaposhnikovUnsubmitted
Done
ReplyInline Actions

perhaps, given the check above this assert is not super useful
if you decide to keep it then we need to add a string message

assert (.... && "....")
alexander-shaposhnikov: perhaps, given the check above this assert is not super useful if you decide to keep it then…
aguinetAuthorUnsubmitted
Done
ReplyInline Actions

Indeed. I put it during development to make sure I didn't make any mistake, but I will remove it.

aguinet: Indeed. I put it during development to make sure I didn't make any mistake, but I will remove…
ArchiveSlice.B = A;
return Slice{std::move(ArchiveSlice)};
}
Expected<Slice> Slice::create(const IRObjectFile *IRO, uint32_t Align) {
Expected<MachoCPUTy> CPUOrErr = getMachoCPUFromTriple(IRO->getTargetTriple());
if (!CPUOrErr)
return CPUOrErr.takeError();
unsigned CPUType, CPUSubType;
std::tie(CPUType, CPUSubType) = CPUOrErr.get();
// We don't directly use the architecture name of the target triple T, as,
// for instance, thumb is treated as ARM by the MachOUniversal object.
std::string ArchName(
MachOObjectFile::getArchTriple(CPUType, CPUSubType).getArchName());
return Slice{IRO, CPUType, CPUSubType, std::move(ArchName), Align};
}
static Expected<SmallVector<MachO::fat_arch, 2>> static Expected<SmallVector<MachO::fat_arch, 2>>
buildFatArchList(ArrayRef<Slice> Slices) { buildFatArchList(ArrayRef<Slice> Slices) {
SmallVector<MachO::fat_arch, 2> FatArchList; SmallVector<MachO::fat_arch, 2> FatArchList;
uint64_t Offset = uint64_t Offset =
sizeof(MachO::fat_header) + Slices.size() * sizeof(MachO::fat_arch); sizeof(MachO::fat_header) + Slices.size() * sizeof(MachO::fat_arch);
for (const auto &S : Slices) { for (const auto &S : Slices) {
Offset = alignTo(Offset, 1ull << S.getP2Alignment()); Offset = alignTo(Offset, 1ull << S.getP2Alignment());
▲ Show 20 Lines • Show All 73 Lines • Show Last 20 Lines