Return an error when dsymutil might produce an invalid mach-o file.

[clayborg]

Summary

64 bit mach-o files have sections that only have 32 bit file offsets. If dsymutil tries to produce an invalid mach-o file, then error out with a good error string.

[aprantl]

llvm/tools/dsymutil/MachOUtils.cpp

325

what does error() return?

[JDevlieghere]

A little bike shedding about the error but otherwise this LGTM

llvm/tools/dsymutil/MachOUtils.cpp

325

[JDevlieghere]

llvm/tools/dsymutil/MachOUtils.cpp

325

It prints the error and returns false.

[clayborg]

Yeah, the error reporting was doing this pattern everywhere else, and I didn't want to break the pattern, or switch all of dsymutil over the llvm::Error...

[clayborg]

Remove old code that was incorrectly checking for a total 4GB byte size of the final produced file. This code didn't work as intended for universal mach-o files since it was ok for these files to be larger than 4GB, and it didn't work for skinny mach-o files since they can actually be larger than 4GB as long as no uint32_t file offset would exceed UINT32_MAX.

[JDevlieghere]

llvm/tools/dsymutil/dsymutil.cpp

732 ↗

(On Diff #414498)

So instead of the check below we now want to check the fat binary. Something like this:

uint64_t offset = 0;
for(size_t i = 0; i < NeedsTempFiles.size() - 1; ++i) {
  ErrorOr<vfs::Status> stat =
        Options.LinkOpts.VFS->status(OutputLocationOrErr->DWARFFile);
  if(!stat)
    break;
  offset += stat->getSize();
  if (offset > UINT32_MAX) 
    WithColor::error() << "the fat binary has a slice with an offset exceeds 4GB and will produce an invalid mach-o file.";
        return EXIT_FAILURE;
}

[JDevlieghere]

Requesting changes to detect issues in fat Mach-Os

[clayborg]

Add size detection for universal files with a slice that starts beyond 4GB, and clean up the error message as requested.

[JDevlieghere]

Thanks Greg. LGTM with "Mach-O" everywhere for consistency.

llvm/tools/dsymutil/dsymutil.cpp

758 ↗

(On Diff #414526)

llvm/tools/dsymutil/MachOUtils.cpp

for (unsigned i = 0; i < nsects; ++i) {
     }
     if (Obj.isLittleEndian() != sys::IsLittleEndianHost)
       MachO::swapStruct(Sect);
     Writer.W.OS.write(reinterpret_cast<char *>(&Sect), sizeof(Sect));
   }
 }
 
 // Write the __DWARF segment load command to the output file.
-static void createDwarfSegment(uint64_t VMAddr, uint64_t FileOffset,
+static bool createDwarfSegment(uint64_t VMAddr, uint64_t FileOffset,
                                uint64_t FileSize, unsigned NumSections,
                                MCAsmLayout &Layout, MachObjectWriter &Writer) {
   Writer.writeSegmentLoadCommand("__DWARF", NumSections, VMAddr,
                                  alignTo(FileSize, 0x1000), FileOffset,
                                  FileSize, /* MaxProt */ 7,
                                  /* InitProt =*/3);
 
   for (unsigned int i = 0, n = Layout.getSectionOrder().size(); i != n; ++i) {
     MCSection *Sec = Layout.getSectionOrder()[i];
     if (Sec->begin() == Sec->end() || !Layout.getSectionFileSize(Sec))
       continue;
 
     unsigned Align = Sec->getAlignment();
     if (Align > 1) {
       VMAddr = alignTo(VMAddr, Align);
       FileOffset = alignTo(FileOffset, Align);
+      if (FileOffset > UINT32_MAX)
+        return error("section " + Sec->getName() + "'s file offset exceeds 4GB and will produce an invalid mach-o file");
     }
     Writer.writeSection(Layout, *Sec, VMAddr, FileOffset, 0, 0, 0);
 
     FileOffset += Layout.getSectionAddressSize(Sec);
     VMAddr += Layout.getSectionAddressSize(Sec);
   }
+  return true;
 }
 
 static bool isExecutable(const object::MachOObjectFile &Obj) {
   if (Obj.is64Bit())
     return Obj.getHeader64().filetype != MachO::MH_OBJECT;
   else
     return Obj.getHeader().filetype != MachO::MH_OBJECT;
 }
if (DwarfVMAddr + DwarfSegmentSize > DwarfVMMax ||
     // address space. Look through segments to find a gap.
     DwarfVMAddr = GapForDwarf;
     if (DwarfVMAddr == UINT64_MAX)
       warn("not enough VM space for the __DWARF segment.",
            "output file streaming");
   }
 
   // Write the load command for the __DWARF segment.
-  createDwarfSegment(DwarfVMAddr, DwarfSegmentStart, DwarfSegmentSize,
-                     NumDwarfSections, Layout, Writer);
+  if (!createDwarfSegment(DwarfVMAddr, DwarfSegmentStart, DwarfSegmentSize,
+                          NumDwarfSections, Layout, Writer))
+    return false;
 
   assert(OutFile.tell() == LoadCommandSize + HeaderSize);
   OutFile.write_zeros(SymtabStart - (LoadCommandSize + HeaderSize));
   assert(OutFile.tell() == SymtabStart);
 
   // Transfer symbols.
   if (ShouldEmitSymtab) {
     OutFile << NewSymtab.str();