[JITLink][MachO] Fix handling of non-extern UNSIGNED pair of SUBTRACT…

…OR relocs. When processing a MachO SUBTRACTOR/UNSIGNED pair, if the UNSIGNED target is non-extern then check the r_symbolnum field of the relocation to find the targeted section and use the section's address to find 'ToSymbol'. Previously 'ToSymbol' was found by loading the initial value stored at the fixup location and treating this as an address to search for. This is incorrect, however: the initial value includes the addend and will point to the wrong block if the addend is less than zero or greater than the block size. rdar://65756694
YuriPlyakhin · Jul 19, 2020 · f7a5715 · f7a5715
1 parent ef66e3d
commit f7a5715
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 12 deletions.
diff --git a/llvm/lib/ExecutionEngine/JITLink/MachO_arm64.cpp b/llvm/lib/ExecutionEngine/JITLink/MachO_arm64.cpp
@@ -148,10 +148,11 @@ class MachOLinkGraphBuilder_arm64 : public MachOLinkGraphBuilder {
       else
         return ToSymbolOrErr.takeError();
     } else {
-      if (auto ToSymbolOrErr = findSymbolByAddress(FixupValue))
-        ToSymbol = &*ToSymbolOrErr;
-      else
-        return ToSymbolOrErr.takeError();
+      auto ToSymbolSec = findSectionByIndex(UnsignedRI.r_symbolnum - 1);
+      if (!ToSymbolSec)
+        return ToSymbolSec.takeError();
+      ToSymbol = getSymbolByAddress(ToSymbolSec->Address);
+      assert(ToSymbol && "No symbol for section");
       FixupValue -= ToSymbol->getAddress();
     }
 

diff --git a/llvm/lib/ExecutionEngine/JITLink/MachO_x86_64.cpp b/llvm/lib/ExecutionEngine/JITLink/MachO_x86_64.cpp
@@ -150,10 +150,11 @@ class MachOLinkGraphBuilder_x86_64 : public MachOLinkGraphBuilder {
       else
         return ToSymbolOrErr.takeError();
     } else {
-      if (auto ToSymbolOrErr = findSymbolByAddress(FixupValue))
-        ToSymbol = &*ToSymbolOrErr;
-      else
-        return ToSymbolOrErr.takeError();
+      auto ToSymbolSec = findSectionByIndex(UnsignedRI.r_symbolnum - 1);
+      if (!ToSymbolSec)
+        return ToSymbolSec.takeError();
+      ToSymbol = getSymbolByAddress(ToSymbolSec->Address);
+      assert(ToSymbol && "No symbol for section");
       FixupValue -= ToSymbol->getAddress();
     }
 

diff --git a/llvm/test/ExecutionEngine/JITLink/X86/MachO_x86-64_relocations.s b/llvm/test/ExecutionEngine/JITLink/X86/MachO_x86-64_relocations.s
@@ -193,19 +193,19 @@ anon_func_addr_quad:
 
 # X86_64_RELOC_SUBTRACTOR Quad/Long in named storage with anonymous minuend
 #
-# jitlink-check: *{8}anon_minuend_quad1 = section_addr(macho_reloc.o, __data) - anon_minuend_quad1 + 2
+# jitlink-check: *{8}anon_minuend_quad1 = section_addr(macho_reloc.o, __data) - anon_minuend_quad1 - 2
 # Only the form "B: .quad LA - B + C" is tested. The form "B: .quad B - LA + C" is
 # invalid because the subtrahend can not be local.
         .globl  anon_minuend_quad1
         .p2align  3
 anon_minuend_quad1:
-        .quad Lanon_data - anon_minuend_quad1 + 2
+        .quad Lanon_data - anon_minuend_quad1 - 2
 
-# jitlink-check: *{4}anon_minuend_long1 = (section_addr(macho_reloc.o, __data) - anon_minuend_long1 + 2)[31:0]
+# jitlink-check: *{4}anon_minuend_long1 = (section_addr(macho_reloc.o, __data) - anon_minuend_long1 - 2)[31:0]
         .globl  anon_minuend_long1
         .p2align  2
 anon_minuend_long1:
-        .long Lanon_data - anon_minuend_long1 + 2
+        .long Lanon_data - anon_minuend_long1 - 2
 
 # Check X86_64_RELOC_SUBTRACTOR Quad/Long in named storage with minuend and subtrahend.
 # Both forms "A: .quad A - B + C" and "A: .quad B - A + C" are tested.