[lld-macho] Ensure __bss sections we output have file offset of zero

Summary:
llvm-mc emits `__bss` sections with an offset of zero, but we weren't expecting
that in our input, so we were copying non-zero data from the start of the file and
putting it in `__bss`, with obviously undesirable runtime results. (It appears that
the kernel will copy those nonzero bytes as long as the offset is nonzero, regardless
of whether S_ZERO_FILL is set.)

I debated on whether to make a special ZeroFillSection -- separate from a
regular InputSection -- but it seemed like too much work for now. But I'm happy
to refactor if anyone feels strongly about having it as a separate class.

Depends on D80857.

Reviewers: ruiu, pcc, MaskRay, smeenai, alexshap, gkm, Ktwu, christylee

Reviewed By: smeenai

Subscribers: llvm-commits

Tags: #llvm

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

diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index 071d738..46fe82f 100644 (file)
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -137,7 +137,8 @@ void InputFile::parseSections(ArrayRef<section_64> sections) {
     isec->file = this;
     isec->name = StringRef(sec.sectname, strnlen(sec.sectname, 16));
     isec->segname = StringRef(sec.segname, strnlen(sec.segname, 16));
-    isec->data = {buf + sec.offset, static_cast<size_t>(sec.size)};
+    isec->data = {isZeroFill(sec.flags) ? nullptr : buf + sec.offset,
+                  static_cast<size_t>(sec.size)};
     if (sec.align >= 32)
       error("alignment " + std::to_string(sec.align) + " of section " +
             isec->name + " is too large");

diff --git a/lld/MachO/InputSection.cpp b/lld/MachO/InputSection.cpp
index 327fd41..72d4892 100644 (file)
--- a/lld/MachO/InputSection.cpp
+++ b/lld/MachO/InputSection.cpp
@@ -28,8 +28,10 @@ uint64_t InputSection::getFileOffset() const {
 uint64_t InputSection::getVA() const { return parent->addr + outSecOff; }
 
 void InputSection::writeTo(uint8_t *buf) {
-  if (!data.empty())
-    memcpy(buf, data.data(), data.size());
+  if (getFileSize() == 0)
+    return;
+
+  memcpy(buf, data.data(), data.size());
 
   for (Reloc &r : relocs) {
     uint64_t va = 0;

diff --git a/lld/MachO/InputSection.h b/lld/MachO/InputSection.h
index 8b0e31d..96ae0cb 100644 (file)
--- a/lld/MachO/InputSection.h
+++ b/lld/MachO/InputSection.h
@@ -35,11 +35,17 @@ struct Reloc {
   llvm::PointerUnion<Symbol *, InputSection *> target;
 };
 
+inline bool isZeroFill(uint8_t flags) {
+  return (flags & llvm::MachO::SECTION_TYPE) == llvm::MachO::S_ZEROFILL;
+}
+
 class InputSection {
 public:
   virtual ~InputSection() = default;
   virtual uint64_t getSize() const { return data.size(); }
-  virtual uint64_t getFileSize() const { return getSize(); }
+  virtual uint64_t getFileSize() const {
+    return isZeroFill(flags) ? 0 : getSize();
+  }
   uint64_t getFileOffset() const;
   uint64_t getVA() const;
 

diff --git a/lld/MachO/SyntheticSections.cpp b/lld/MachO/SyntheticSections.cpp
index 7237aa6..f84a603 100644 (file)
--- a/lld/MachO/SyntheticSections.cpp
+++ b/lld/MachO/SyntheticSections.cpp
@@ -17,6 +17,7 @@
 #include "Writer.h"
 
 #include "lld/Common/ErrorHandler.h"
+#include "lld/Common/Memory.h"
 #include "llvm/Support/EndianStream.h"
 #include "llvm/Support/LEB128.h"
 
@@ -210,6 +211,9 @@ void StubHelperSection::setup() {
 ImageLoaderCacheSection::ImageLoaderCacheSection() {
   segname = segment_names::data;
   name = "__data";
+  uint8_t *arr = bAlloc.Allocate<uint8_t>(WordSize);
+  memset(arr, 0, WordSize);
+  data = {arr, WordSize};
 }
 
 LazyPointerSection::LazyPointerSection()

diff --git a/lld/MachO/Writer.cpp b/lld/MachO/Writer.cpp
index def82b5..0fcde15 100644 (file)
--- a/lld/MachO/Writer.cpp
+++ b/lld/MachO/Writer.cpp
@@ -416,7 +416,7 @@ void Writer::assignAddresses(OutputSegment *seg) {
     addr = alignTo(addr, section->align);
     fileOff = alignTo(fileOff, section->align);
     section->addr = addr;
-    section->fileOff = fileOff;
+    section->fileOff = isZeroFill(section->flags) ? 0 : fileOff;
     section->finalize();
 
     addr += section->getSize();

diff --git a/lld/test/MachO/bss.s b/lld/test/MachO/bss.s
new file mode 100644 (file)
index 0000000..7e7af12
--- /dev/null
+++ b/lld/test/MachO/bss.s
@@ -0,0 +1,38 @@
+# 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 --macho-segment %t | FileCheck %s
+
+## Check that __bss takes up zero file size and is at file offset zero.
+
+# CHECK:        Name: __bss
+# CHECK-NEXT:   Segment: __DATA
+# CHECK-NEXT:   Address:
+# CHECK-NEXT:   Size: 0x4
+# CHECK-NEXT:   Offset: 0
+# CHECK-NEXT:   Alignment: 0
+# CHECK-NEXT:   RelocationOffset: 0x0
+# CHECK-NEXT:   RelocationCount: 0
+# CHECK-NEXT:   Type: ZeroFill (0x1)
+# CHECK-NEXT:   Attributes [ (0x0)
+# CHECK-NEXT:   ]
+# CHECK-NEXT:   Reserved1: 0x0
+# CHECK-NEXT:   Reserved2: 0x0
+# CHECK-NEXT:   Reserved3: 0x0
+
+# CHECK:      Name: __DATA
+# CHECK-NEXT: Size:
+# CHECK-NEXT: vmaddr:
+# CHECK-NEXT: vmsize: 0x4
+# CHECK-NEXT: fileoff:
+# CHECK-NEXT: filesize: 0
+
+.globl _main
+
+.text
+_main:
+  movq $0, %rax
+  retq
+
+.bss
+.zero 4