[lld-macho] Add some relocation validation logic

I considered making a `Target::validate()` method, but I wasn't sure how
I felt about the overhead of doing yet another switch-dispatch on the
relocation type, so I put the validation in `relocateOne` instead...
might be a bit of a micro-optimization, but `relocateOne` does assume
certain things about the relocations it gets, and this error handling
makes that explicit, so it's not a totally unreasonable code
organization.

Reviewed By: smeenai

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

diff --git a/lld/MachO/Arch/X86_64.cpp b/lld/MachO/Arch/X86_64.cpp
index 21ae3d2..1b58e70 100644 (file)
--- a/lld/MachO/Arch/X86_64.cpp
+++ b/lld/MachO/Arch/X86_64.cpp
@@ -6,6 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "InputFiles.h"
 #include "Symbols.h"
 #include "SyntheticSections.h"
 #include "Target.h"
@@ -24,7 +25,8 @@ namespace {
 struct X86_64 : TargetInfo {
   X86_64();
 
-  uint64_t getImplicitAddend(const uint8_t *loc, uint8_t type) const override;
+  uint64_t getImplicitAddend(MemoryBufferRef, const section_64 &,
+                             const relocation_info &) const override;
   void relocateOne(uint8_t *loc, uint8_t type, uint64_t val) const override;
 
   void writeStub(uint8_t *buf, const DylibSymbol &) const override;
@@ -38,19 +40,36 @@ struct X86_64 : TargetInfo {
 
 } // namespace
 
-uint64_t X86_64::getImplicitAddend(const uint8_t *loc, uint8_t type) const {
-  switch (type) {
+static std::string getErrorLocation(MemoryBufferRef mb, const section_64 &sec,
+                                    const relocation_info &rel) {
+  return ("invalid relocation at offset " + std::to_string(rel.r_address) +
+          " of " + sec.segname + "," + sec.sectname + " in " +
+          mb.getBufferIdentifier())
+      .str();
+}
+
+uint64_t X86_64::getImplicitAddend(MemoryBufferRef mb, const section_64 &sec,
+                                   const relocation_info &rel) const {
+  auto *buf = reinterpret_cast<const uint8_t *>(mb.getBufferStart());
+  const uint8_t *loc = buf + sec.offset + rel.r_address;
+  switch (rel.r_type) {
   case X86_64_RELOC_BRANCH:
   case X86_64_RELOC_SIGNED:
   case X86_64_RELOC_SIGNED_1:
   case X86_64_RELOC_SIGNED_2:
   case X86_64_RELOC_SIGNED_4:
   case X86_64_RELOC_GOT_LOAD:
+    if (!rel.r_pcrel)
+      fatal(getErrorLocation(mb, sec, rel) + ": relocations of type " +
+            std::to_string(rel.r_type) + " must be pcrel");
     return read32le(loc);
   case X86_64_RELOC_UNSIGNED:
+    if (rel.r_pcrel)
+      fatal(getErrorLocation(mb, sec, rel) + ": relocations of type " +
+            std::to_string(rel.r_type) + " must not be pcrel");
     return read64le(loc);
   default:
-    error("TODO: Unhandled relocation type " + std::to_string(type));
+    error("TODO: Unhandled relocation type " + std::to_string(rel.r_type));
     return 0;
   }
 }

diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index b72c350..50ec84f 100644 (file)
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -177,9 +177,7 @@ void InputFile::parseRelocations(const section_64 &sec,
     Reloc r;
     r.type = rel.r_type;
     r.pcrel = rel.r_pcrel;
-    uint32_t secRelOffset = rel.r_address;
-    uint64_t rawAddend =
-        target->getImplicitAddend(buf + sec.offset + secRelOffset, r.type);
+    uint64_t rawAddend = target->getImplicitAddend(mb, sec, rel);
 
     if (rel.r_extern) {
       r.target = symbols[rel.r_symbolnum];
@@ -202,13 +200,13 @@ void InputFile::parseRelocations(const section_64 &sec,
       // TODO: The offset of 4 is probably not right for ARM64, nor for
       //       relocations with r_length != 2.
       uint32_t targetOffset =
-          sec.addr + secRelOffset + 4 + rawAddend - targetSec.addr;
+          sec.addr + rel.r_address + 4 + rawAddend - targetSec.addr;
       r.target = findContainingSubsection(targetSubsecMap, &targetOffset);
       r.addend = targetOffset;
     }
 
-    InputSection *subsec = findContainingSubsection(subsecMap, &secRelOffset);
-    r.offset = secRelOffset;
+    r.offset = rel.r_address;
+    InputSection *subsec = findContainingSubsection(subsecMap, &r.offset);
     subsec->relocs.push_back(r);
   }
 }

diff --git a/lld/MachO/Target.h b/lld/MachO/Target.h
index 1af7c03..60c7c6f 100644 (file)
--- a/lld/MachO/Target.h
+++ b/lld/MachO/Target.h
@@ -9,6 +9,9 @@
 #ifndef LLD_MACHO_TARGET_H
 #define LLD_MACHO_TARGET_H
 
+#include "llvm/BinaryFormat/MachO.h"
+#include "llvm/Support/MemoryBuffer.h"
+
 #include <cstddef>
 #include <cstdint>
 
@@ -16,6 +19,8 @@ namespace lld {
 namespace macho {
 
 class DylibSymbol;
+class InputSection;
+struct Reloc;
 
 enum {
   // We are currently only supporting 64-bit targets since macOS and iOS are
@@ -30,8 +35,10 @@ class TargetInfo {
 public:
   virtual ~TargetInfo() = default;
 
-  virtual uint64_t getImplicitAddend(const uint8_t *loc,
-                                     uint8_t type) const = 0;
+  // Validate the relocation structure and get its addend.
+  virtual uint64_t
+  getImplicitAddend(llvm::MemoryBufferRef, const llvm::MachO::section_64 &,
+                    const llvm::MachO::relocation_info &) const = 0;
   virtual void relocateOne(uint8_t *loc, uint8_t type, uint64_t val) const = 0;
 
   // Write code for lazy binding. See the comments on StubsSection for more

diff --git a/lld/test/MachO/invalid/invalid-relocation.yaml b/lld/test/MachO/invalid/invalid-relocation.yaml
new file mode 100644 (file)
index 0000000..ed7c24e
--- /dev/null
+++ b/lld/test/MachO/invalid/invalid-relocation.yaml
@@ -0,0 +1,99 @@
+# REQUIRES: x86
+# RUN: yaml2obj %s -o %t.o
+# RUN: not lld -flavor darwinnew -o %t %t.o 2>&1 | FileCheck %s -DFILE=%t.o
+#
+# CHECK: error: invalid relocation at offset 1 of __TEXT,__text in [[FILE]]: relocations of type 0 must not be pcrel
+
+!mach-o
+FileHeader:
+  magic:           0xFEEDFACF
+  cputype:         0x01000007
+  cpusubtype:      0x00000003
+  filetype:        0x00000001
+  ncmds:           4
+  sizeofcmds:      280
+  flags:           0x00000000
+  reserved:        0x00000000
+LoadCommands:
+  - cmd:             LC_SEGMENT_64
+    cmdsize:         152
+    segname:         ''
+    vmaddr:          0
+    vmsize:          9
+    fileoff:         312
+    filesize:        9
+    maxprot:         7
+    initprot:        7
+    nsects:          1
+    flags:           0
+    Sections:
+      - sectname:        __text
+        segname:         __TEXT
+        addr:            0x0000000000000000
+        size:            9
+        offset:          0x00000138
+        align:           0
+        reloff:          0x00000144
+        nreloc:          1
+        flags:           0x80000000
+        reserved1:       0x00000000
+        reserved2:       0x00000000
+        reserved3:       0x00000000
+        content:         '000000000000000000'
+        relocations:
+          - address:         0x00000001
+            symbolnum:       1
+            pcrel:           true
+            length:          3
+            extern:          true
+            type:            0
+            scattered:       false
+            value:           0
+  - cmd:             LC_BUILD_VERSION
+    cmdsize:         24
+    platform:        1
+    minos:           659200
+    sdk:             0
+    ntools:          0
+  - cmd:             LC_SYMTAB
+    cmdsize:         24
+    symoff:          332
+    nsyms:           2
+    stroff:          364
+    strsize:         12
+  - cmd:             LC_DYSYMTAB
+    cmdsize:         80
+    ilocalsym:       0
+    nlocalsym:       0
+    iextdefsym:      0
+    nextdefsym:      2
+    iundefsym:       2
+    nundefsym:       0
+    tocoff:          0
+    ntoc:            0
+    modtaboff:       0
+    nmodtab:         0
+    extrefsymoff:    0
+    nextrefsyms:     0
+    indirectsymoff:  0
+    nindirectsyms:   0
+    extreloff:       0
+    nextrel:         0
+    locreloff:       0
+    nlocrel:         0
+LinkEditData:
+  NameList:
+    - n_strx:          1
+      n_type:          0x0F
+      n_sect:          1
+      n_desc:          0
+      n_value:         1
+    - n_strx:          6
+      n_type:          0x0F
+      n_sect:          1
+      n_desc:          0
+      n_value:         9
+  StringTable:
+    - ''
+    - _foo
+    - _main