[ELF] writeTrapInstr: Don't decrease p_memsz

[MaskRay]

When the last PT_LOAD segment is executable and includes BSS sections,
its p_memsz may exceed the aligned p_filesz. This change ensures p_memsz
is not reduced in such cases (e.g. --omagic).

In addition, disable this behavior when a SECTIONS command is specified.

Refined behavior introduced in https://reviews.llvm.org/D37369 (2017).

The -z separate-loadable-segments --omagic test adds coverage for the
option combination, even if it might be practical.

[llvmbot]

@llvm/pr-subscribers-lld-elf

@llvm/pr-subscribers-lld

Author: Fangrui Song (MaskRay)

Changes

When the last PT_LOAD segment is executable and includes BSS sections,
its p_memsz may exceed the aligned p_filesz. This change ensures p_memsz
is not reduced in such cases (e.g. --omagic).

In addition, disable this behavior when a SECTIONS command is specified.

Refined behavior introduced in https://reviews.llvm.org/D37369 (2017).

The -z separate-loadable-segments --omagic test adds coverage for the
option combination, even if it might be practical.

---

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

2 Files Affected:

• (modified) lld/ELF/Writer.cpp (+6-3)
• (modified) lld/test/ELF/fill-trap.s (+40-13)

diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index 28b24f90716b8..e2aebff20e174 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -2960,9 +2960,12 @@ template <class ELFT> void Writer<ELFT>::writeTrapInstr() {
       if (p->p_type == PT_LOAD)
         last = p.get();
 
-    if (last && (last->p_flags & PF_X))
-      last->p_memsz = last->p_filesz =
-          alignToPowerOf2(last->p_filesz, ctx.arg.maxPageSize);
+    if (last && (last->p_flags & PF_X)) {
+      last->p_filesz = alignToPowerOf2(last->p_filesz, ctx.arg.maxPageSize);
+      // p_memsz might be larger than the aligned p_filesz due to trailing BSS
+      // sections. Don't decrease it.
+      last->p_memsz = std::max(last->p_memsz, last->p_filesz);
+    }
   }
 }
 
diff --git a/lld/test/ELF/fill-trap.s b/lld/test/ELF/fill-trap.s
index 60276a1cc5af6..f111bb87a92a1 100644
--- a/lld/test/ELF/fill-trap.s
+++ b/lld/test/ELF/fill-trap.s
@@ -1,26 +1,28 @@
 # REQUIRES: x86
-# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+# RUN: rm -rf %t && split-file %s %t && cd %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64 a.s -o a.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64 bss.s -o bss.o
 
 ## -z noseparate-code is the default: text segment is not tail padded.
-# RUN: ld.lld %t.o -o %t
-# RUN: llvm-readobj -l %t | FileCheck %s --check-prefixes=CHECK,NOPAD
-# RUN: ld.lld %t.o -z noseparate-code -z common-page-size=512 -o %t
-# RUN: llvm-readobj -l %t | FileCheck %s --check-prefixes=CHECK,NOPAD
+# RUN: ld.lld a.o -o out
+# RUN: llvm-readobj -l out | FileCheck %s --check-prefixes=CHECK,NOPAD
+# RUN: ld.lld a.o -z noseparate-code -z common-page-size=512 -o out
+# RUN: llvm-readobj -l out | FileCheck %s --check-prefixes=CHECK,NOPAD
 
 ## -z separate-code pads the tail of text segment with traps.
 ## Make common-page-size smaller than max-page-size.
 ## Check that we use max-page-size instead of common-page-size for padding.
-# RUN: ld.lld %t.o -z separate-code -z common-page-size=512 -o %t
-# RUN: llvm-readobj -l %t | FileCheck %s --check-prefixes=CHECK,PAD
-# RUN: od -Ax -x -N16 -j0x1ff0 %t | FileCheck %s --check-prefix=FILL
+# RUN: ld.lld a.o -z separate-code -z common-page-size=512 -o out
+# RUN: llvm-readobj -l out | FileCheck %s --check-prefixes=CHECK,PAD
+# RUN: od -Ax -x -N16 -j0x1ff0 out | FileCheck %s --check-prefix=FILL
 
 ## -z separate-loadable-segments pads all segments, including the text segment.
-# RUN: ld.lld %t.o -z separate-loadable-segments -z common-page-size=512 -o %t
-# RUN: llvm-readobj -l %t | FileCheck %s --check-prefixes=CHECK,PAD
-# RUN: od -Ax -x -N16 -j0x1ff0 %t | FileCheck %s --check-prefix=FILL
+# RUN: ld.lld a.o -z separate-loadable-segments -z common-page-size=512 -o out
+# RUN: llvm-readobj -l out | FileCheck %s --check-prefixes=CHECK,PAD
+# RUN: od -Ax -x -N16 -j0x1ff0 out | FileCheck %s --check-prefix=FILL
 
-# RUN: ld.lld %t.o -z separate-code -z noseparate-code -z common-page-size=512 -o %t
-# RUN: llvm-readobj -l %t | FileCheck %s --check-prefixes=CHECK,NOPAD
+# RUN: ld.lld a.o -z separate-code -z noseparate-code -z common-page-size=512 -o out
+# RUN: llvm-readobj -l out | FileCheck %s --check-prefixes=CHECK,NOPAD
 
 # CHECK: ProgramHeader {
 # CHECK:   Type: PT_LOAD
@@ -39,6 +41,31 @@
 ## Check that executable page is filled with traps at its end.
 # FILL: 001ff0 cccc cccc cccc cccc cccc cccc cccc cccc
 
+## There is a single RWX segment. Test that p_memsz is not truncated to p_filesz.
+# RUN: ld.lld a.o bss.o -z separate-loadable-segments -T rwx.lds -z max-page-size=64 -o rwx
+# RUN: llvm-readelf -l rwx | FileCheck %s --check-prefix=RWX
+
+# RWX:       Type           Offset   VirtAddr           PhysAddr           FileSiz  MemSiz   Flg Align
+# RWX-NEXT:  LOAD           0x000080 0x0000000000000000 0x0000000000000000 0x000040 0x000404 RWE 0x40
+
+# RUN: ld.lld a.o bss.o -z separate-loadable-segments --omagic -o omagic
+# RUN: llvm-readelf -l omagic | FileCheck %s --check-prefix=OMAGIC
+
+# OMAGIC:     Type           Offset   VirtAddr           PhysAddr           FileSiz  MemSiz   Flg Align
+# OMAGIC-NEXT:LOAD           0x0000b0 0x00000000002000b0 0x00000000002000b0 0x000004 0x000404 RWE 0x4
+
+#--- a.s
 .globl _start
 _start:
   nop
+
+#--- bss.s
+.bss
+.space 1024
+
+#--- rwx.lds
+PHDRS { all PT_LOAD; }
+SECTIONS {
+  .text : {*(.text*)} :all
+  .bss : {*(.bss*)} :all
+}

[smithp35]

Definitely agree that we shouldn't truncate p_memsz.

I think its unlikely to use Omagic and separate-loadable-segments but it doesn't hurt to have the test case.