Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[lipo] Support creating Universal 64 bit Mach-O files. #67737

Merged
merged 1 commit into from
Sep 30, 2023

Conversation

drodriguez
Copy link
Contributor

Xcode lipo seems to support a non-documented -fat64 option that creates Universal Mach-O archives using 64 bit versions of the fat_arch header, which allows offsets larger than 32 bits to be specified.

Modify llvm-lipo to support the same flag, and use the value of the flag to use either 32 bits or 64 bits Mach-O headers.

The Mach-O universal writer allows specifying a new option to write these 64 bits headers. The default is still using 32 bits.

dsymutil implemented support for a similar flag in https://reviews.llvm.org/D146879.

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 28, 2023

@llvm/pr-subscribers-llvm-binary-utilities

Changes

Xcode lipo seems to support a non-documented -fat64 option that creates Universal Mach-O archives using 64 bit versions of the fat_arch header, which allows offsets larger than 32 bits to be specified.

Modify llvm-lipo to support the same flag, and use the value of the flag to use either 32 bits or 64 bits Mach-O headers.

The Mach-O universal writer allows specifying a new option to write these 64 bits headers. The default is still using 32 bits.

dsymutil implemented support for a similar flag in https://reviews.llvm.org/D146879.


Full diff: https://github.com/llvm/llvm-project/pull/67737.diff

5 Files Affected:

  • (modified) llvm/include/llvm/Object/MachOUniversalWriter.h (+2-2)
  • (modified) llvm/lib/Object/MachOUniversalWriter.cpp (+60-23)
  • (added) llvm/test/tools/llvm-lipo/create-fat64.test (+11)
  • (modified) llvm/tools/llvm-lipo/LipoOpts.td (+3)
  • (modified) llvm/tools/llvm-lipo/llvm-lipo.cpp (+6-3)
diff --git a/llvm/include/llvm/Object/MachOUniversalWriter.h b/llvm/include/llvm/Object/MachOUniversalWriter.h
index 4004f25f3fb7ecc..73527a023cb0f63 100644
--- a/llvm/include/llvm/Object/MachOUniversalWriter.h
+++ b/llvm/include/llvm/Object/MachOUniversalWriter.h
@@ -97,9 +97,9 @@ class Slice {
   }
 };
 
-Error writeUniversalBinary(ArrayRef<Slice> Slices, StringRef OutputFileName);
+Error writeUniversalBinary(ArrayRef<Slice> Slices, StringRef OutputFileName, bool UseFat64 = false);
 
-Error writeUniversalBinaryToStream(ArrayRef<Slice> Slices, raw_ostream &Out);
+Error writeUniversalBinaryToStream(ArrayRef<Slice> Slices, raw_ostream &Out, bool UseFat64 = false);
 
 } // end namespace object
 
diff --git a/llvm/lib/Object/MachOUniversalWriter.cpp b/llvm/lib/Object/MachOUniversalWriter.cpp
index 909a10b2c072a2e..54ffd0bf475ca3e 100644
--- a/llvm/lib/Object/MachOUniversalWriter.cpp
+++ b/llvm/lib/Object/MachOUniversalWriter.cpp
@@ -240,25 +240,48 @@ Expected<Slice> Slice::create(const IRObjectFile &IRO, uint32_t Align) {
   return Slice{IRO, CPUType, CPUSubType, std::move(ArchName), Align};
 }
 
-static Expected<SmallVector<MachO::fat_arch, 2>>
+template <typename FatArchTy>
+struct FatArchTraits {
+  static const uint64_t OffsetLimit;
+  static const std::string StructName;
+  static const uint8_t BitCount;
+};
+
+template <> struct FatArchTraits<MachO::fat_arch> {
+  static const uint64_t OffsetLimit = UINT32_MAX;
+  static const std::string StructName;
+  static const uint8_t BitCount = 32;
+};
+const std::string FatArchTraits<MachO::fat_arch>::StructName = "fat_arch";
+
+template <> struct FatArchTraits<MachO::fat_arch_64> {
+  static const uint64_t OffsetLimit = UINT64_MAX;
+  static const std::string StructName;
+  static const uint8_t BitCount = 64;
+};
+const std::string FatArchTraits<MachO::fat_arch_64>::StructName = "fat_arch_64";
+
+template <typename FatArchTy>
+static Expected<SmallVector<FatArchTy, 2>>
 buildFatArchList(ArrayRef<Slice> Slices) {
-  SmallVector<MachO::fat_arch, 2> FatArchList;
+  SmallVector<FatArchTy, 2> FatArchList;
   uint64_t Offset =
-      sizeof(MachO::fat_header) + Slices.size() * sizeof(MachO::fat_arch);
+      sizeof(MachO::fat_header) + Slices.size() * sizeof(FatArchTy);
 
   for (const auto &S : Slices) {
     Offset = alignTo(Offset, 1ull << S.getP2Alignment());
-    if (Offset > UINT32_MAX)
+    if (Offset > FatArchTraits<FatArchTy>::OffsetLimit)
       return createStringError(
           std::errc::invalid_argument,
-          ("fat file too large to be created because the offset "
-           "field in struct fat_arch is only 32-bits and the offset " +
-           Twine(Offset) + " for " + S.getBinary()->getFileName() +
+          ("fat file too large to be created because the offset field in "
+           "the struct " + Twine(FatArchTraits<FatArchTy>::StructName) +
+           " is only " + Twine(FatArchTraits<FatArchTy>::BitCount) + "-bits and the"
+           " offset " + Twine(Offset) + " for " + S.getBinary()->getFileName() +
            " for architecture " + S.getArchString() + "exceeds that.")
               .str()
               .c_str());
 
-    MachO::fat_arch FatArch;
+    FatArchTy FatArch;
     FatArch.cputype = S.getCPUType();
     FatArch.cpusubtype = S.getCPUSubType();
     FatArch.offset = Offset;
@@ -270,17 +293,16 @@ buildFatArchList(ArrayRef<Slice> Slices) {
   return FatArchList;
 }
 
-Error object::writeUniversalBinaryToStream(ArrayRef<Slice> Slices,
-                                           raw_ostream &Out) {
-  MachO::fat_header FatHeader;
-  FatHeader.magic = MachO::FAT_MAGIC;
-  FatHeader.nfat_arch = Slices.size();
-
-  Expected<SmallVector<MachO::fat_arch, 2>> FatArchListOrErr =
-      buildFatArchList(Slices);
+template <typename FatArchTy>
+static Error
+writeUniversalArchsToStream(MachO::fat_header FatHeader,
+                                    ArrayRef<Slice> Slices,
+                                    raw_ostream &Out) {
+  Expected<SmallVector<FatArchTy, 2>> FatArchListOrErr =
+      buildFatArchList<FatArchTy>(Slices);
   if (!FatArchListOrErr)
     return FatArchListOrErr.takeError();
-  SmallVector<MachO::fat_arch, 2> FatArchList = *FatArchListOrErr;
+  SmallVector<FatArchTy, 2> FatArchList = *FatArchListOrErr;
 
   if (sys::IsLittleEndianHost)
     MachO::swapStruct(FatHeader);
@@ -288,17 +310,17 @@ Error object::writeUniversalBinaryToStream(ArrayRef<Slice> Slices,
             sizeof(MachO::fat_header));
 
   if (sys::IsLittleEndianHost)
-    for (MachO::fat_arch &FA : FatArchList)
+    for (FatArchTy &FA : FatArchList)
       MachO::swapStruct(FA);
   Out.write(reinterpret_cast<const char *>(FatArchList.data()),
-            sizeof(MachO::fat_arch) * FatArchList.size());
+            sizeof(FatArchTy) * FatArchList.size());
 
   if (sys::IsLittleEndianHost)
-    for (MachO::fat_arch &FA : FatArchList)
+    for (FatArchTy &FA : FatArchList)
       MachO::swapStruct(FA);
 
   size_t Offset =
-      sizeof(MachO::fat_header) + sizeof(MachO::fat_arch) * FatArchList.size();
+      sizeof(MachO::fat_header) + sizeof(FatArchTy) * FatArchList.size();
   for (size_t Index = 0, Size = Slices.size(); Index < Size; ++Index) {
     MemoryBufferRef BufferRef = Slices[Index].getBinary()->getMemoryBufferRef();
     assert((Offset <= FatArchList[Index].offset) && "Incorrect slice offset");
@@ -311,8 +333,23 @@ Error object::writeUniversalBinaryToStream(ArrayRef<Slice> Slices,
   return Error::success();
 }
 
+Error object::writeUniversalBinaryToStream(ArrayRef<Slice> Slices,
+                                           raw_ostream &Out,
+                                           bool UseFat64) {
+  MachO::fat_header FatHeader;
+  FatHeader.magic = UseFat64 ? MachO::FAT_MAGIC_64 : MachO::FAT_MAGIC;
+  FatHeader.nfat_arch = Slices.size();
+
+  if (UseFat64) {
+    return writeUniversalArchsToStream<MachO::fat_arch_64>(FatHeader, Slices, Out);
+  } else {
+    return writeUniversalArchsToStream<MachO::fat_arch>(FatHeader, Slices, Out);
+  }
+}
+
 Error object::writeUniversalBinary(ArrayRef<Slice> Slices,
-                                   StringRef OutputFileName) {
+                                   StringRef OutputFileName,
+                                   bool UseFat64) {
   const bool IsExecutable = any_of(Slices, [](Slice S) {
     return sys::fs::can_execute(S.getBinary()->getFileName());
   });
@@ -324,7 +361,7 @@ Error object::writeUniversalBinary(ArrayRef<Slice> Slices,
   if (!Temp)
     return Temp.takeError();
   raw_fd_ostream Out(Temp->FD, false);
-  if (Error E = writeUniversalBinaryToStream(Slices, Out)) {
+  if (Error E = writeUniversalBinaryToStream(Slices, Out, UseFat64)) {
     if (Error DiscardError = Temp->discard())
       return joinErrors(std::move(E), std::move(DiscardError));
     return E;
diff --git a/llvm/test/tools/llvm-lipo/create-fat64.test b/llvm/test/tools/llvm-lipo/create-fat64.test
new file mode 100644
index 000000000000000..1c420899ed18a7d
--- /dev/null
+++ b/llvm/test/tools/llvm-lipo/create-fat64.test
@@ -0,0 +1,11 @@
+# RUN: yaml2obj %p/Inputs/i386-slice.yaml -o %t-i386.o
+# RUN: yaml2obj %p/Inputs/x86_64-slice.yaml -o %t-x86_64.o
+
+# RUN: llvm-lipo %t-i386.o %t-x86_64.o -create -output %t-universal-32.o
+# RUN: llvm-objdump -m --universal-headers %t-universal-32.o | FileCheck %s -check-prefixes=FAT32
+
+# RUN: llvm-lipo %t-i386.o %t-x86_64.o -create -fat64 -output %t-universal-64.o
+# RUN: llvm-objdump -m --universal-headers %t-universal-64.o | FileCheck %s -check-prefixes=FAT64
+
+FAT32: fat_magic FAT_MAGIC
+FAT64: fat_magic FAT_MAGIC_64
diff --git a/llvm/tools/llvm-lipo/LipoOpts.td b/llvm/tools/llvm-lipo/LipoOpts.td
index 866353573cba309..32bdca4a5da1607 100644
--- a/llvm/tools/llvm-lipo/LipoOpts.td
+++ b/llvm/tools/llvm-lipo/LipoOpts.td
@@ -58,3 +58,6 @@ def replace
 def output : Option<["-", "--"], "output", KIND_SEPARATE>,
              HelpText<"Create output file with specified name">;
 def o : JoinedOrSeparate<["-"], "o">, Alias<output>;
+
+def fat64 : Option<["-", "--"], "fat64", KIND_FLAG>,
+            HelpText<"Use 64 bits Universal Mach-O format">;
diff --git a/llvm/tools/llvm-lipo/llvm-lipo.cpp b/llvm/tools/llvm-lipo/llvm-lipo.cpp
index b1f202877b15e8d..b5d38027ccb991a 100644
--- a/llvm/tools/llvm-lipo/llvm-lipo.cpp
+++ b/llvm/tools/llvm-lipo/llvm-lipo.cpp
@@ -116,6 +116,7 @@ struct Config {
   std::string ArchType;
   std::string OutputFile;
   LipoAction ActionToPerform;
+  bool UseFat64;
 };
 
 static Slice createSliceFromArchive(LLVMContext &LLVMCtx, const Archive &A) {
@@ -223,6 +224,8 @@ static Config parseLipoOptions(ArrayRef<const char *> ArgsArr) {
                   Twine(AlignmentValue));
   }
 
+  C.UseFat64 = InputArgs.hasArg(LIPO_fat64);
+
   SmallVector<opt::Arg *, 1> ActionArgs(InputArgs.filtered(LIPO_action_group));
   if (ActionArgs.empty())
     reportError("at least one action should be specified");
@@ -598,7 +601,7 @@ buildSlices(LLVMContext &LLVMCtx, ArrayRef<OwningBinary<Binary>> InputBinaries,
 
 [[noreturn]] static void createUniversalBinary(
     LLVMContext &LLVMCtx, ArrayRef<OwningBinary<Binary>> InputBinaries,
-    const StringMap<const uint32_t> &Alignments, StringRef OutputFileName) {
+    const StringMap<const uint32_t> &Alignments, StringRef OutputFileName, bool UseFat64) {
   assert(InputBinaries.size() >= 1 && "Incorrect number of input binaries");
   assert(!OutputFileName.empty() && "Create expects a single output file");
 
@@ -609,7 +612,7 @@ buildSlices(LLVMContext &LLVMCtx, ArrayRef<OwningBinary<Binary>> InputBinaries,
   checkUnusedAlignments(Slices, Alignments);
 
   llvm::stable_sort(Slices);
-  if (Error E = writeUniversalBinary(Slices, OutputFileName))
+  if (Error E = writeUniversalBinary(Slices, OutputFileName, UseFat64))
     reportError(std::move(E));
 
   exit(EXIT_SUCCESS);
@@ -748,7 +751,7 @@ int llvm_lipo_main(int argc, char **argv, const llvm::ToolContext &) {
     break;
   case LipoAction::CreateUniversal:
     createUniversalBinary(LLVMCtx, InputBinaries, C.SegmentAlignments,
-                          C.OutputFile);
+                          C.OutputFile, C.UseFat64);
     break;
   case LipoAction::ReplaceArch:
     replaceSlices(LLVMCtx, InputBinaries, C.SegmentAlignments, C.OutputFile,

@github-actions
Copy link

github-actions bot commented Sep 28, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Member

@JDevlieghere JDevlieghere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small suggestion but overall this LGTM.

llvm/include/llvm/Object/MachOUniversalWriter.h Outdated Show resolved Hide resolved
Xcode `lipo` seems to support a non-document `-fat64` option that
creates Universal Mach-O archives using 64 bit versions of the
`fat_arch` header, which allows offsets larger than 32 bits to be
specified.

Modify `llvm-lipo` to support the same flag, and use the value of the
flag to use either 32 bits or 64 bits Mach-O headers.

The Mach-O universal writer allows specifying a new option to write
these 64 bits headers. The default is still using 32 bits.

`dsymutil` implemented support for a similar flag in
https://reviews.llvm.org/D146879.
@drodriguez drodriguez merged commit efb4651 into llvm:main Sep 30, 2023
2 of 3 checks passed
@drodriguez drodriguez deleted the lipo-fat64 branch September 30, 2023 22:25
@vitalybuka
Copy link
Collaborator

Please take a look at https://lab.llvm.org/buildbot/#/builders/74/builds/22357

@drodriguez
Copy link
Contributor Author

Please take a look at https://lab.llvm.org/buildbot/#/builders/74/builds/22357

Thanks. It seems to happen outside the lines I directly modified, and from the stack trace seems to happen in some internal buffer of raw_ostream. No idea how these changes are triggering something that far. I will try to setup the MSan tomorrow in a Linux machine, I have never done it before.

vitalybuka added a commit that referenced this pull request Oct 2, 2023
@vitalybuka
Copy link
Collaborator

I will try to setup the MSan tomorrow in a Linux machine, I have never done it before.

Usually, if it can't be fixed immediately, it should be reverted.

I guess 55d8f0c is the fix, fat_arch_64::reserved is uninitialized.
The fix should not affect correct code, but please feel free to improve if you have better fix.

@drodriguez
Copy link
Contributor Author

Sorry. I was trying to get the MSan working on my machine. It seems to do many builds before giving me any error.

I think your fix is the right thing. That reserved field seems uninitialized and I did not see the difference between the two different headers when I was changing the code. Thanks for fixing.

Sorry for any breakage I might have caused. I will try to be more careful in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants