[PATCH] D85086: [ELF] --oformat=binary: use LMA to compute file offsets

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
MaskRay added reviewers: grimar, jhenderson, psmith.
Herald added subscribers: llvm-commits, krytarowski, arichardson, emaste.
Herald added a reviewer: espindola.
Herald added a project: LLVM.
MaskRay requested review of this revision.

--oformat=binary is rare (used in a few places in FreeBSD, see `stand/i386/mbr/Makefile` `LDFLAGS_BIN`)
The result should be identical to a normal output transformed by `objcopy -O binary`.

The current implementation ignores addresses and lays out sections by
respecting output section alignments. It can fail when an output section
address is specified, e.g. `.rodata ALIGN(16) :` (PR33651).

Fix PR33651 by respecting LMA. The code is similar to
`tools/llvm-objcop/ELF/Object.cpp` BinaryWriter::finalize after D71035 <https://reviews.llvm.org/D71035> and D79229 <https://reviews.llvm.org/D79229>.
Unforunately for an output section without PT_LOAD, we assume its LMA is equal
to its VMA. So the result is still incorrect when an output section LMA
(`AT(...)`) is specified

Also drop `alignTo(off, config->wordsize)`. GNU ld does not round up the file size.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85086

Files:
  lld/ELF/Writer.cpp
  lld/test/ELF/oformat-binary-ttext.s
  lld/test/ELF/oformat-binary.s


Index: lld/test/ELF/oformat-binary.s
===================================================================
--- lld/test/ELF/oformat-binary.s
+++ lld/test/ELF/oformat-binary.s
@@ -3,8 +3,8 @@
 
 # RUN: ld.lld -o %t.out %t --oformat binary
 # RUN: od -t x1 -v %t.out | FileCheck %s
-# CHECK: 000000 90 11 22 00 00 00 00 00
-# CHECK-NOT: 00000010
+# CHECK:      0000000 90 11 22
+# CHECK-NEXT: 0000003
 
 ## Check case when linkerscript is used.
 # RUN: echo "SECTIONS { . = 0x1000; }" > %t.script
@@ -15,6 +15,31 @@
 # RUN: ld.lld -o %t2.out --script %t.script %t --oformat binary
 # RUN: od -t x1 -v %t2.out | FileCheck %s
 
+## LMA(.text)=0x100, LMA(.mysec)=0x108. The minimum LMA of all non-empty sections is 0x100.
+## We place an output section at its LMA minus 0x100.
+# RUN: echo 'SECTIONS { .text 0x100 : {*(.text)} .mysec ALIGN(8) : {*(.mysec*)} }' > %talign.lds
+# RUN: ld.lld -T %talign.lds %t --oformat binary -o %talign
+# RUN: od -Ax -t x1 %talign | FileCheck %s --check-prefix=ALIGN --ignore-case
+
+# ALIGN:      000000 90 00 00 00 00 00 00 00 11 22
+# ALIGN-NEXT: 00000a
+
+## The empty section .data is ignored when computing the file size.
+# RUN: echo 'SECTIONS { .text : {*(.text .mysec*)} .data 0x100 : {keep = .;}}' > %tempty.lds
+# RUN: ld.lld -T %tempty.lds %t --oformat binary -o %tempty
+# RUN: od -t x1 %tempty | FileCheck %s
+
+## NOBITS sections are ignored as well.
+# RUN: echo 'SECTIONS { .text : {*(.text .mysec*)} .data 0x100 (NOLOAD) : {BYTE(0)}}' > %tnobits.lds
+# RUN: ld.lld -T %tnobits.lds %t --oformat binary -o %tnobits
+# RUN: od -t x1 %tnobits | FileCheck %s
+
+## FIXME .mysec should be placed at file offset 1.
+## This does not work because for a section without PT_LOAD, we consider LMA = VMA.
+# RUN: echo 'SECTIONS { .text : {*(.text)} .mysec 0x8 : AT(1) {*(.mysec*)} }' > %tlma.lds
+# RUN: ld.lld -T %tlma.lds %t --oformat binary -o %tlma
+# RUN: od -Ax -t x1 %tlma | FileCheck %s --check-prefix=ALIGN --ignore-case
+
 # RUN: not ld.lld -o /dev/null %t --oformat foo 2>&1 \
 # RUN:   | FileCheck %s --check-prefix ERR
 # ERR: unknown --oformat value: foo
Index: lld/test/ELF/oformat-binary-ttext.s
===================================================================
--- lld/test/ELF/oformat-binary-ttext.s
+++ lld/test/ELF/oformat-binary-ttext.s
@@ -4,9 +4,8 @@
 # RUN: ld.lld -N -Ttext 0x100 -o %t.out %t --oformat binary
 # RUN: od -t x1 -v %t.out | FileCheck %s --check-prefix=BIN
 
-# BIN:      0000000 90 00 00 00 00 00 00 00
-# BIN-NEXT: 0000010
-# BIN-NOT:  0000020
+# BIN:      0000000 90
+# BIN-NEXT: 0000001
 
 ## The same but without OMAGIC.
 # RUN: ld.lld -Ttext 0x100 -o %t.out %t --oformat binary
Index: lld/ELF/Writer.cpp
===================================================================
--- lld/ELF/Writer.cpp
+++ lld/ELF/Writer.cpp
@@ -2547,11 +2547,21 @@
 }
 
 template <class ELFT> void Writer<ELFT>::assignFileOffsetsBinary() {
-  uint64_t off = 0;
+  // Compute the minimum LMA of all non-empty non-NOBITS sections as minAddr.
+  uint64_t minAddr = UINT64_MAX;
   for (OutputSection *sec : outputSections)
-    if (sec->flags & SHF_ALLOC)
-      off = setFileOffset(sec, off);
-  fileSize = alignTo(off, config->wordsize);
+    if (sec->type != SHT_NOBITS && (sec->flags & SHF_ALLOC) && sec->size > 0) {
+      sec->offset = sec->getLMA();
+      minAddr = std::min(minAddr, sec->offset);
+    }
+
+  // Sections are laided out at LMA minus minAddr.
+  fileSize = 0;
+  for (OutputSection *sec : outputSections)
+    if (sec->type != SHT_NOBITS && (sec->flags & SHF_ALLOC) && sec->size > 0) {
+      sec->offset -= minAddr;
+      fileSize = std::max(fileSize, sec->offset + sec->size);
+    }
 }
 
 static std::string rangeToString(uint64_t addr, uint64_t len) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D85086.282431.patch
Type: text/x-patch
Size: 3734 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200802/4478ba3d/attachment.bin>