<div dir="ltr"><div class="gmail_extra">LGTM</div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jan 21, 2015 at 10:03 AM, Will Newton <span dir="ltr"><<a href="mailto:will.newton@linaro.org" target="_blank">will.newton@linaro.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Add support for checking overflow when applying a R_AARCH64_ABS32<br>
relocation and add a test to ensure it behaves correctly.<br>
---<br>
 .../ELF/AArch64/AArch64RelocationHandler.cpp       | 20 ++++++--<br>
 test/elf/AArch64/rel-abs32-overflow.test           | 53 ++++++++++++++++++++++<br>
 2 files changed, 68 insertions(+), 5 deletions(-)<br>
 create mode 100644 test/elf/AArch64/rel-abs32-overflow.test<br>
<br>
diff --git a/lib/ReaderWriter/ELF/AArch64/AArch64RelocationHandler.cpp b/lib/ReaderWriter/ELF/AArch64/AArch64RelocationHandler.cpp<br>
index 924836d..85a3038 100644<br>
--- a/lib/ReaderWriter/ELF/AArch64/AArch64RelocationHandler.cpp<br>
+++ b/lib/ReaderWriter/ELF/AArch64/AArch64RelocationHandler.cpp<br>
@@ -16,6 +16,13 @@ using namespace elf;<br>
<br>
 #define PAGE(X) ((X) & ~0x0FFFL)<br>
<br>
+/// \brief Check X is in the interval (-2^(bits-1), 2^bits]<br>
+bool withinSignedUnsignedRange(int64_t X, int bits) {<br>
+  if (X < -(1L << (bits - 1)) || X >= (1L << bits))<br>
+    return false;<br>
+  return true;<br>
+}<br>
+<br>
 /// \brief R_AARCH64_ABS64 - word64: S + A<br>
 static void relocR_AARCH64_ABS64(uint8_t *location, uint64_t P, uint64_t S,<br>
                                  int64_t A) {<br>
@@ -41,9 +48,11 @@ static void relocR_AARCH64_PREL32(uint8_t *location, uint64_t P, uint64_t S,<br>
 }<br>
<br>
 /// \brief R_AARCH64_ABS32 - word32:  S + A<br>
-static void relocR_AARCH64_ABS32(uint8_t *location, uint64_t P, uint64_t S,<br>
-                                 int64_t A) {<br>
-  int32_t result = (int32_t)(S + A);<br>
+static std::error_code relocR_AARCH64_ABS32(uint8_t *location, uint64_t P,<br>
+                                            uint64_t S, int64_t A) {<br>
+  int64_t result = S + A;<br>
+  if (!withinSignedUnsignedRange(result, 32))<br></blockquote><div><br></div><div>Because you only pass 32 as an argument, I'd probably write (result != (int32_t)result), because it's easy to understand.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+    return make_out_of_range_reloc_error();<br>
   DEBUG_WITH_TYPE(<br>
       "AArch64", llvm::dbgs() << "\t\tHandle " << LLVM_FUNCTION_NAME << " -";<br>
       llvm::dbgs() << " S: 0x" << Twine::utohexstr(S);<br>
@@ -53,6 +62,7 @@ static void relocR_AARCH64_ABS32(uint8_t *location, uint64_t P, uint64_t S,<br>
   *reinterpret_cast<llvm::support::ulittle32_t *>(location) =<br>
       result |<br>
       (int32_t) * reinterpret_cast<llvm::support::little32_t *>(location);<br>
+  return std::error_code();<br>
 }<br>
<br>
 /// \brief R_AARCH64_ADR_PREL_PG_HI21 - Page(S+A) - Page(P)<br>
@@ -385,8 +395,8 @@ std::error_code AArch64TargetRelocationHandler::applyRelocation(<br>
                           ref.addend());<br>
     break;<br>
   case R_AARCH64_ABS32:<br>
-    relocR_AARCH64_ABS32(location, relocVAddress, targetVAddress, ref.addend());<br>
-    break;<br>
+    return relocR_AARCH64_ABS32(location, relocVAddress, targetVAddress,<br>
+                                ref.addend());<br>
   // Runtime only relocations. Ignore here.<br>
   case R_AARCH64_RELATIVE:<br>
   case R_AARCH64_IRELATIVE:<br>
diff --git a/test/elf/AArch64/rel-abs32-overflow.test b/test/elf/AArch64/rel-abs32-overflow.test<br>
new file mode 100644<br>
index 0000000..be65ebc<br>
--- /dev/null<br>
+++ b/test/elf/AArch64/rel-abs32-overflow.test<br>
@@ -0,0 +1,53 @@<br>
+# Check handling of R_AARCH64_ABS32 relocation overflow.<br>
+# RUN: yaml2obj -format=elf %s > %t-obj<br>
+# RUN: not lld -flavor gnu -target arm64 -o %t-exe %t-obj 2>&1 | FileCheck %s<br>
+<br>
+# CHECK: Relocation out of range in file {{.*}}: reference from data1+0 to data2+34359738369 of type 258 (R_AARCH64_ABS32)<br>
+# CHECK: Relocation out of range in file {{.*}}: reference from data2+0 to data1+34359738369 of type 258 (R_AARCH64_ABS32)<br>
+<br>
+!ELF<br>
+FileHeader: !FileHeader<br>
+  Class: ELFCLASS64<br>
+  Data: ELFDATA2LSB<br>
+  Type: ET_REL<br>
+  Machine: EM_AARCH64<br>
+<br>
+Sections:<br>
+- Name: .text<br>
+  Type: SHT_PROGBITS<br>
+  Content: "00000000"<br>
+  AddressAlign: 16<br>
+  Flags: [SHF_ALLOC, SHF_EXECINSTR]<br>
+- Name: .data<br>
+  Type: SHT_PROGBITS<br>
+  Content: "0000000000000000"<br>
+  AddressAlign: 16<br>
+  Flags: [SHF_ALLOC, SHF_WRITE]<br>
+<br>
+- Name: .rela.data<br>
+  Type: SHT_RELA<br>
+  Info: .data<br>
+  AddressAlign: 8<br>
+  Relocations:<br>
+    - Offset: 0x0<br>
+      Symbol: data2<br>
+      Type: R_AARCH64_ABS32<br>
+      Addend: 0x800000001<br>
+    - Offset: 0x4<br>
+      Symbol: data1<br>
+      Type: R_AARCH64_ABS32<br>
+      Addend: 0x800000001<br>
+<br>
+Symbols:<br>
+  Global:<br>
+    - Name: _start<br>
+      Section: .text<br>
+      Value: 0x0<br>
+      Size: 4<br>
+    - Name: data1<br>
+      Section: .data<br>
+      Size: 4<br>
+    - Name: data2<br>
+      Section: .data<br>
+      Value: 0x4<br>
+      Size: 4<br>
<span class="HOEnZb"><font color="#888888">--<br>
2.1.0<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</font></span></blockquote></div><br></div></div>