[PATCH] D84574: [lld-macho] Fix segment filesize calculation

[Jez Ng via Phabricator via llvm-commits]

int3 created this revision.
int3 added a reviewer: lld-macho.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

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.

Question: I would like to write a test for this particular invariant,
i.e. that the `__LINKEDIT` offset + filesize equals to the total file
size. But I'm not sure what's a good way of determining total file size
in the test suite -- `du` probably isn't available on Windows, and isn't
really portable between Linux and macOS either. Would I need to write a
utility tool for this, like `not`?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84574

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


Index: lld/test/MachO/section-headers.s
===================================================================
--- lld/test/MachO/section-headers.s
+++ 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
Index: lld/MachO/Writer.cpp
===================================================================
--- lld/MachO/Writer.cpp
+++ lld/MachO/Writer.cpp
@@ -134,7 +134,10 @@
     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;
@@ -451,6 +454,8 @@
   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;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D84574.280644.patch
Type: text/x-patch
Size: 1943 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200725/b0f7c965/attachment.bin>