[JITLink] fix most i686 relocations to include addend

vtjnash · vtjnash · commit 0d56a9f39f17 · 2024-10-04T17:11:29.000Z
Most relocations involve adding the value at the location with a value
taken from elsewhere. Most of them were hard coded instead. Assume
signed, since the math is the same either way after truncation, but this
seems more likely to give reasonable looking intermediate results.
diff --git a/llvm/include/llvm/ExecutionEngine/JITLink/i386.h b/llvm/include/llvm/ExecutionEngine/JITLink/i386.h
@@ -39,12 +39,8 @@ enum EdgeKind_i386 : Edge::Kind {
   /// Represents a data/control flow instruction using PC-relative addressing
   /// to a target.
   ///
-  /// The fixup expression for this kind includes an implicit offset to account
-  /// for the PC (unlike the Delta edges) so that a PCRel32 with a target
-  /// T and addend zero is a call/branch to the start (offset zero) of T.
-  ///
   /// Fixup expression:
-  ///   Fixup <- Target - (Fixup + 4) + Addend : int32
+  ///   Fixup <- Target - Fixup + Addend : int32
   ///
   /// Errors:
   ///   - The result of the fixup expression must fit into an int32, otherwise
@@ -68,12 +64,8 @@ enum EdgeKind_i386 : Edge::Kind {
   /// Represents a data/control flow instruction using PC-relative addressing
   /// to a target.
   ///
-  /// The fixup expression for this kind includes an implicit offset to account
-  /// for the PC (unlike the Delta edges) so that a PCRel16 with a target
-  /// T and addend zero is a call/branch to the start (offset zero) of T.
-  ///
   /// Fixup expression:
-  ///   Fixup <- Target - (Fixup + 4) + Addend : int16
+  ///   Fixup <- Target - Fixup + Addend : int16
   ///
   /// Errors:
   ///   - The result of the fixup expression must fit into an int16, otherwise
@@ -86,7 +78,7 @@ enum EdgeKind_i386 : Edge::Kind {
   /// Delta from the fixup to the target.
   ///
   /// Fixup expression:
-  ///   Fixup <- Target - Fixup + Addend : int64
+  ///   Fixup <- Target - Fixup + Addend : int32
   ///
   /// Errors:
   ///   - The result of the fixup expression must fit into an int32, otherwise
@@ -130,12 +122,8 @@ enum EdgeKind_i386 : Edge::Kind {
   /// Represents a PC-relative call or branch to a target. This can be used to
   /// identify, record, and/or patch call sites.
   ///
-  /// The fixup expression for this kind includes an implicit offset to account
-  /// for the PC (unlike the Delta edges) so that a Branch32PCRel with a target
-  /// T and addend zero is a call/branch to the start (offset zero) of T.
-  ///
   /// Fixup expression:
-  ///   Fixup <- Target - (Fixup + 4) + Addend : int32
+  ///   Fixup <- Target - Fixup + Addend : int32
   ///
   /// Errors:
   ///   - The result of the fixup expression must fit into an int32, otherwise
@@ -164,7 +152,7 @@ enum EdgeKind_i386 : Edge::Kind {
   /// target may be recorded to allow manipulation at runtime.
   ///
   /// Fixup expression:
-  ///   Fixup <- Target - Fixup + Addend - 4 : int32
+  ///   Fixup <- Target - Fixup + Addend : int32
   ///
   /// Errors:
   ///   - The result of the fixup expression must fit into an int32, otherwise
@@ -180,7 +168,7 @@ enum EdgeKind_i386 : Edge::Kind {
   /// is within range of the fixup location.
   ///
   /// Fixup expression:
-  ///   Fixup <- Target - Fixup + Addend - 4: int32
+  ///   Fixup <- Target - Fixup + Addend : int32
   ///
   /// Errors:
   ///   - The result of the fixup expression must fit into an int32, otherwise
@@ -215,8 +203,7 @@ inline Error applyFixup(LinkGraph &G, Block &B, const Edge &E,
   }
 
   case i386::PCRel32: {
-    int32_t Value =
-        E.getTarget().getAddress() - (FixupAddress + 4) + E.getAddend();
+    int32_t Value = E.getTarget().getAddress() - FixupAddress + E.getAddend();
     *(little32_t *)FixupPtr = Value;
     break;
   }
@@ -231,8 +218,7 @@ inline Error applyFixup(LinkGraph &G, Block &B, const Edge &E,
   }
 
   case i386::PCRel16: {
-    int32_t Value =
-        E.getTarget().getAddress() - (FixupAddress + 4) + E.getAddend();
+    int32_t Value = E.getTarget().getAddress() - FixupAddress + E.getAddend();
     if (LLVM_LIKELY(isInt<16>(Value)))
       *(little16_t *)FixupPtr = Value;
     else
@@ -257,8 +243,7 @@ inline Error applyFixup(LinkGraph &G, Block &B, const Edge &E,
   case i386::BranchPCRel32:
   case i386::BranchPCRel32ToPtrJumpStub:
   case i386::BranchPCRel32ToPtrJumpStubBypassable: {
-    int32_t Value =
-        E.getTarget().getAddress() - (FixupAddress + 4) + E.getAddend();
+    int32_t Value = E.getTarget().getAddress() - FixupAddress + E.getAddend();
     *(little32_t *)FixupPtr = Value;
     break;
   }
diff --git a/llvm/lib/ExecutionEngine/JITLink/ELF_i386.cpp b/llvm/lib/ExecutionEngine/JITLink/ELF_i386.cpp
@@ -186,15 +186,29 @@ class ELFLinkGraphBuilder_i386 : public ELFLinkGraphBuilder<ELFT> {
     int64_t Addend = 0;
 
     switch (*Kind) {
-    case i386::EdgeKind_i386::Delta32: {
+    case i386::EdgeKind_i386::None:
+      break;
+    case i386::EdgeKind_i386::Pointer32:
+    case i386::EdgeKind_i386::PCRel32:
+    case i386::EdgeKind_i386::RequestGOTAndTransformToDelta32FromGOT:
+    case i386::EdgeKind_i386::Delta32:
+    case i386::EdgeKind_i386::Delta32FromGOT:
+    case i386::EdgeKind_i386::BranchPCRel32:
+    case i386::EdgeKind_i386::BranchPCRel32ToPtrJumpStub:
+    case i386::EdgeKind_i386::BranchPCRel32ToPtrJumpStubBypassable: {
       const char *FixupContent = BlockToFix.getContent().data() +
                                  (FixupAddress - BlockToFix.getAddress());
-      Addend = *(const support::ulittle32_t *)FixupContent;
+      Addend = *(const support::little32_t *)FixupContent;
       break;
     }
-    default:
+    case i386::EdgeKind_i386::Pointer16:
+    case i386::EdgeKind_i386::PCRel16: {
+      const char *FixupContent = BlockToFix.getContent().data() +
+                                 (FixupAddress - BlockToFix.getAddress());
+      Addend = *(const support::little16_t *)FixupContent;
       break;
     }
+    }
 
     Edge::OffsetT Offset = FixupAddress - BlockToFix.getAddress();
     Edge GE(*Kind, Offset, *GraphSymbol, Addend);
diff --git a/llvm/test/ExecutionEngine/JITLink/i386/ELF_i386_absolute_relocations_16.s b/llvm/test/ExecutionEngine/JITLink/i386/ELF_i386_absolute_relocations_16.s
@@ -22,4 +22,12 @@ main:
         .type   bar,@function
 bar:
         retw    $external_data
-        .size   bar, .-bar
+        .size   bar, .-bar
+
+# jitlink-check: decode_operand(baz, 0) = external_data + 23
+        .globl  baz
+        .align        2, 0x90
+        .type   baz,@function
+baz:
+        retw    $external_data+23
+        .size   baz, .-baz
diff --git a/llvm/test/ExecutionEngine/JITLink/i386/ELF_i386_absolute_relocations_32.s b/llvm/test/ExecutionEngine/JITLink/i386/ELF_i386_absolute_relocations_32.s
@@ -7,17 +7,25 @@
 # Test ELF 32 bit absolute relocations
 
         .text
-        .globl  main     
+        .globl  main
         .p2align        4, 0x90
         .type   main,@function
-main:                                   
+main:
         retl
         .size   main, .-main
 
 # jitlink-check: decode_operand(foo, 0) = external_data
-        .globl  foo     
+        .globl  foo
         .p2align        4, 0x90
         .type   foo,@function
 foo:
         movl    external_data, %eax
-        .size   foo, .-foo
+        .size   foo, .-foo
+
+# jitlink-check: decode_operand(bar, 0) = external_data + 4000
+        .globl  bar
+        .p2align        4, 0x90
+        .type   bar,@function
+bar:
+        movl    external_data + 4000, %eax
+        .size   bar, .-bar
diff --git a/llvm/test/ExecutionEngine/JITLink/i386/ELF_i386_pc_relative_relocations_32.s b/llvm/test/ExecutionEngine/JITLink/i386/ELF_i386_pc_relative_relocations_32.s
@@ -33,17 +33,18 @@ foo:
 
 
 # Tests PC relative relocation for negative offset from PC
-# jitlink-check: decode_operand(baz, 0) = fooz - next_pc(baz)
+# jitlink-check: decode_operand(baz, 0) = fooz - next_pc(baz) + 1
         .globl  fooz
         .p2align        4
         .type   fooz,@function
 fooz:
+    nop
     retl
         .size   fooz, .-fooz
 
         .globl  baz
         .p2align        4
         .type   baz,@function
 baz:
-    calll fooz
-        .size       baz, .-baz
+    calll fooz+1
+        .size       baz, .-baz
diff --git a/llvm/test/ExecutionEngine/JITLink/i386/ELF_i386_small_pic_relocations_got.s b/llvm/test/ExecutionEngine/JITLink/i386/ELF_i386_small_pic_relocations_got.s
@@ -19,29 +19,29 @@ main:
 # Test GOT32 handling.
 # 
 # We want to check both the offset to the GOT entry and its contents. 
-# jitlink-check: decode_operand(test_got, 4) = got_addr(elf_sm_pic_reloc_got.o, named_data1) - _GLOBAL_OFFSET_TABLE_
+# jitlink-check: decode_operand(test_got, 4) = got_addr(elf_sm_pic_reloc_got.o, named_data1) - _GLOBAL_OFFSET_TABLE_ + 42
 # jitlink-check: *{4}(got_addr(elf_sm_pic_reloc_got.o, named_data1)) = named_data1
 # 
-# jitlink-check: decode_operand(test_got+6, 4) = got_addr(elf_sm_pic_reloc_got.o, named_data2) - _GLOBAL_OFFSET_TABLE_
+# jitlink-check: decode_operand(test_got+6, 4) = got_addr(elf_sm_pic_reloc_got.o, named_data2) - _GLOBAL_OFFSET_TABLE_ + 5
 # jitlink-check: *{4}(got_addr(elf_sm_pic_reloc_got.o, named_data2)) = named_data2
 
         .globl test_got
         .p2align      4, 0x90
         .type   test_got,@function
 test_got:
-	leal    named_data1@GOT, %eax
-        leal    named_data2@GOT, %eax
+        leal    named_data1@GOT+42, %eax
+        leal    named_data2@GOT+5, %eax
         .size   test_got, .-test_got
 
 
 
 # Test GOTOFF64 handling.
-# jitlink-check: decode_operand(test_gotoff, 1) = named_func - _GLOBAL_OFFSET_TABLE_
+# jitlink-check: decode_operand(test_gotoff, 1) = named_func - _GLOBAL_OFFSET_TABLE_ + 99
         .globl test_gotoff
         .p2align     4, 0x90
         .type  test_gotoff,@function
 test_gotoff:
-        mov $named_func@GOTOFF, %eax
+        mov $named_func@GOTOFF+99, %eax
         .size   test_gotoff, .-test_gotoff
 
 
diff --git a/llvm/test/ExecutionEngine/JITLink/i386/ELF_i386_small_pic_relocations_plt.s b/llvm/test/ExecutionEngine/JITLink/i386/ELF_i386_small_pic_relocations_plt.s
@@ -27,12 +27,12 @@ main:
 # for position independent code, first, as there may be future use-cases
 # where we would want to disable the optimization.
 # 
-# jitlink-check: decode_operand(test_call_extern_plt, 0) = external_func - next_pc(test_call_extern_plt)
+# jitlink-check: decode_operand(test_call_extern_plt, 0) = external_func - next_pc(test_call_extern_plt) + 53
 # jitlink-check: *{4}(got_addr(elf_sm_pic_reloc_plt.o, external_func))= external_func
         .globl  test_call_extern_plt
         .p2align       4, 0x90
         .type   test_call_extern_plt,@function
 test_call_extern_plt:
-        call   external_func@plt
+        call   external_func@plt + 53
 
-        .size   test_call_extern_plt, .-test_call_extern_plt
+        .size   test_call_extern_plt, .-test_call_extern_plt
