[lld-macho] Fix segment filesize calculation

[int3]

Summary

The previous approach of adding up the file sizes of the
component sections ignored the fact that the sections did not have to be
contiguous in the file. As such, it was underestimating the true size.

I discovered this issue because codesign checks whether __LINKEDIT
extends to the end of the file. Since we were underestimating segment
sizes, this check failed.

[pcc]

I think you can use wc -c. See e.g. clang/test/Modules/rebuild.m.

[int3]

add test for total file size

[int3]

Thanks @pcc!

lld/test/MachO/segments.s

9

I should have made these CHECK-NEXT from the start...

[compnerd]

I think that the assert would be better than the comment, but not worth blocking on.

lld/MachO/Writer.cpp

137

I wonder if that should just be an assert instead.

[int3]

use assert

lld/MachO/Writer.cpp

137

Agreed :)

lld/MachO/Writer.cpp

if (seg->getSections().empty())
       return;
 
     c->vmaddr = seg->firstSection()->addr;
     c->vmsize =
         seg->lastSection()->addr + seg->lastSection()->getSize() - c->vmaddr;
     c->nsects = seg->numNonHiddenSections();
 
     for (OutputSection *osec : seg->getSections()) {
-      c->filesize += osec->getFileSize();
+      // This condition should hold unless we are dealing with a __bss section.
+      if (osec->fileOff >= seg->fileOff)
+        c->filesize = std::max(
+            c->filesize, osec->fileOff + osec->getFileSize() - seg->fileOff);
 
       if (osec->isHidden())
         continue;
 
       auto *sectHdr = reinterpret_cast<section_64 *>(buf);
       buf += sizeof(section_64);
 
       memcpy(sectHdr->sectname, osec->name.data(), osec->name.size());

 }
 
 void Writer::assignAddresses(OutputSegment *seg) {
   addr = alignTo(addr, PageSize);
   fileOff = alignTo(fileOff, PageSize);
   seg->fileOff = fileOff;
 
   for (auto *osec : seg->getSections()) {
+    if (!osec->isNeeded())
+      continue;
     addr = alignTo(addr, osec->align);
     fileOff = alignTo(fileOff, osec->align);
     osec->addr = addr;
     osec->fileOff = isZeroFill(osec->flags) ? 0 : fileOff;
     osec->finalize();
 
     addr += osec->getSize();
     fileOff += osec->getFileSize();

lld/test/MachO/section-headers.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
 # RUN: lld -flavor darwinnew -o %t %t.o
-# RUN: llvm-readobj --section-headers %t | FileCheck %s
+# RUN: llvm-readobj --section-headers --macho-segment %t | FileCheck %s
 
 # CHECK:      Name: __text
 # CHECK-NEXT: Segment: __TEXT
 # CHECK-NOT:  }
 # CHECK:      Alignment: 1
 # CHECK-NOT:  }
 # CHECK:      Type: Regular (0x0)
 # CHECK-NEXT: Attributes [ (0x800004)
 # CHECK-NEXT:   PureInstructions (0x800000)
 # CHECK-NEXT:   SomeInstructions (0x4)
 # CHECK-NEXT: ]
 
 # CHECK:      Name: __cstring
 # CHECK-NEXT: Segment: __TEXT
 # CHECK-NOT:  }
 # CHECK:      Alignment: 2
 # CHECK-NOT:  }
 # CHECK:      Type: CStringLiterals (0x2)
 # CHECK-NEXT: Attributes [ (0x0)
 # CHECK-NEXT: ]
 
 # CHECK:      Name: maxlen_16ch_name
 # CHECK-NEXT: Segment: __TEXT
-# CHECK-NOT:  }
-# CHECK:      Alignment: 3
+# CHECK-NEXT: Address:
+# CHECK-NEXT: Size: [[#%x, LAST_SEC_SIZE:]]
+# CHECK-NEXT: Offset: [[#%u, LAST_SEC_OFF:]]
+# CHECK-NEXT: Alignment: 3
 # CHECK-NOT:  }
 # CHECK:      Type: Regular (0x0)
 
+# CHECK-LABEL: Segment {
+# CHECK:       Name: __TEXT
+# CHECK-NEXT:  Size:
+# CHECK-NEXT:  vmaddr:
+# CHECK-NEXT:  vmsize:
+# CHECK-NEXT:  fileoff: 0
+# CHECK-NEXT:  filesize: [[#%u, LAST_SEC_SIZE + LAST_SEC_OFF]]
+
 .text
 .align 1
 .global _main
 _main:
   mov $0, %rax
   ret
 
 .section __TEXT,__cstring
 .align 2
 str:
   .asciz "Hello world!\n"
 
 .section __TEXT,maxlen_16ch_name
 .align 3

lld/test/MachO/segments.s

-# REQUIRES: x86
+# REQUIRES: x86, shell
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
 # RUN: lld -flavor darwinnew -o %t %t.o
-# RUN: llvm-readobj --macho-segment %t | FileCheck %s
+# RUN: (llvm-readobj --macho-segment %t; echo "Total file size"; wc -c %t) | FileCheck %s
 
 ## These two segments must always be present at the start of an executable.
 # CHECK-NOT:  Segment {
 # CHECK:      Segment {
-# CHECK:        Cmd: LC_SEGMENT_64
-# CHECK:        Name: __PAGEZERO
-# CHECK:        Size: 72
-# CHECK:        vmaddr: 0x0
-# CHECK:        vmsize: 0x100000000
-# CHECK:        fileoff: 0
-# CHECK:        filesize: 0
+# CHECK-NEXT:   Cmd: LC_SEGMENT_64
+# CHECK-NEXT:   Name: __PAGEZERO
+# CHECK-NEXT:   Size: 72
+# CHECK-NEXT:   vmaddr: 0x0
+# CHECK-NEXT:   vmsize: 0x100000000
+# CHECK-NEXT:   fileoff: 0
+# CHECK-NEXT:   filesize: 0
 ## The kernel won't execute a binary with the wrong protections for __PAGEZERO.
-# CHECK:        maxprot: ---
-# CHECK:        initprot: ---
-# CHECK:        nsects: 0
-# CHECK:        flags: 0x0
-# CHECK:      }
-# CHECK:      Segment {
-# CHECK:        Cmd: LC_SEGMENT_64
-# CHECK:        Name: __TEXT
-# CHECK:        Size: 152
-# CHECK:        vmaddr: 0x100000000
-# CHECK:        vmsize:
+# CHECK-NEXT:   maxprot: ---
+# CHECK-NEXT:   initprot: ---
+# CHECK-NEXT:   nsects: 0
+# CHECK-NEXT:   flags: 0x0
+# CHECK-NEXT: }
+# CHECK-NEXT: Segment {
+# CHECK-NEXT:   Cmd: LC_SEGMENT_64
+# CHECK-NEXT:   Name: __TEXT
+# CHECK-NEXT:   Size: 152
+# CHECK-NEXT:   vmaddr: 0x100000000
+# CHECK-NEXT:   vmsize:
 ## dyld3 assumes that the __TEXT segment starts from the file header
-# CHECK:        fileoff: 0
-# CHECK:        filesize:
-# CHECK:        maxprot: rwx
-# CHECK:        initprot: r-x
-# CHECK:        nsects: 1
-# CHECK:        flags: 0x0
-# CHECK:      }
+# CHECK-NEXT:   fileoff: 0
+# CHECK-NEXT:   filesize:
+# CHECK-NEXT:   maxprot: rwx
+# CHECK-NEXT:   initprot: r-x
+# CHECK-NEXT:   nsects: 1
+# CHECK-NEXT:   flags: 0x0
+# CHECK-NEXT: }
 
 ## Check that we handle max-length names correctly.
 # CHECK:      Cmd: LC_SEGMENT_64
 # CHECK-NEXT: Name: maxlen_16ch_name
 
-## This segment must always be present at the end of an executable.
+## This segment must always be present at the end of an executable, and cover
+## its last byte.
 # CHECK:      Name: __LINKEDIT
-# CHECK:      maxprot: rwx
-# CHECK:      initprot: r--
+# CHECK-NEXT: Size:
+# CHECK-NEXT: vmaddr:
+# CHECK-NEXT: vmsize:
+# CHECK-NEXT: fileoff: [[#%u, LINKEDIT_OFF:]]
+# CHECK-NEXT: filesize: [[#%u, LINKEDIT_SIZE:]]
+# CHECK-NEXT: maxprot: rwx
+# CHECK-NEXT: initprot: r--
 # CHECK-NOT:  Cmd: LC_SEGMENT_64
 
+# CHECK-LABEL: Total file size
+# CHECK-NEXT:  [[#%u, LINKEDIT_OFF + LINKEDIT_SIZE]]
+
 .text
 .global _main
 _main:
   mov $0, %rax
   ret
 
 .section maxlen_16ch_name,foo