[lld] d32e325 - [lld-macho] Fix segment filesize calculation

[Jez Ng via llvm-commits]

Author: Jez Ng
Date: 2020-07-28T10:02:19-07:00
New Revision: d32e32500f92602ccedcf967df2915da6f3803d2

URL: https://github.com/llvm/llvm-project/commit/d32e32500f92602ccedcf967df2915da6f3803d2
DIFF: https://github.com/llvm/llvm-project/commit/d32e32500f92602ccedcf967df2915da6f3803d2.diff

LOG: [lld-macho] Fix segment filesize calculation

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.

Reviewed By: #lld-macho, compnerd

Differential Revision: https://reviews.llvm.org/D84574

Added: 
    

Modified: 
    lld/MachO/Writer.cpp
    lld/test/MachO/section-headers.s
    lld/test/MachO/segments.s

Removed: 
    


################################################################################
diff  --git a/lld/MachO/Writer.cpp b/lld/MachO/Writer.cpp
index 03000a7f437e..c9070e90f97e 100644
--- a/lld/MachO/Writer.cpp
+++ b/lld/MachO/Writer.cpp
@@ -134,7 +134,11 @@ class LCSegment : public LoadCommand {
     c->nsects = seg->numNonHiddenSections();
 
     for (OutputSection *osec : seg->getSections()) {
-      c->filesize += osec->getFileSize();
+      if (!isZeroFill(osec->flags)) {
+        assert(osec->fileOff >= seg->fileOff);
+        c->filesize = std::max(
+            c->filesize, osec->fileOff + osec->getFileSize() - seg->fileOff);
+      }
 
       if (osec->isHidden())
         continue;
@@ -454,6 +458,8 @@ void Writer::assignAddresses(OutputSegment *seg) {
   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;

diff  --git a/lld/test/MachO/section-headers.s b/lld/test/MachO/section-headers.s
index 9fafc5a912b0..fdfdbed63245 100644
--- a/lld/test/MachO/section-headers.s
+++ b/lld/test/MachO/section-headers.s
@@ -1,7 +1,7 @@
 # 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
@@ -25,11 +25,21 @@
 
 # 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

diff  --git a/lld/test/MachO/segments.s b/lld/test/MachO/segments.s
index acb0f1e90101..e0f127fabe55 100644
--- a/lld/test/MachO/segments.s
+++ b/lld/test/MachO/segments.s
@@ -1,49 +1,58 @@
-# 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: