[llvm] [RuntimeDyld][ELF] Fix unwanted sign extension. (PR #94482)

Alastair Houghton via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 5 08:11:50 PDT 2024


https://github.com/al45tair created https://github.com/llvm/llvm-project/pull/94482

Casting the result of `Section.getAddressWithOffset()` goes wrong if we are on a 32-bit platform whose addresses are regarded as signed; in that case, just doing
```
(uint64_t)Section.getAddressWithOffset(...)
```
or
```
reinterpret_cast<uint64_t>(Section.getAddressWithOffset(...))
```
will result in sign-extension.

We use these expressions when constructing branch stubs, which is before we know the final load address, so we can just switch to the `Section.getLoadAddressWithOffset(...)` method instead.

Doing that is also more consistent, since when calculating relative offsets for relocations, we use the load address anyway, so the code currently only works because `Section.Address` is equal to `Section.LoadAddress` at this point.

Fixes #94478.

>From 9ce81b14d2ef5b4efaae02f1c28caf5908c810e3 Mon Sep 17 00:00:00 2001
From: Alastair Houghton <ahoughton at apple.com>
Date: Wed, 5 Jun 2024 16:01:26 +0100
Subject: [PATCH] [RuntimeDyld][ELF] Fix unwanted sign extension.

Casting the result of `Section.getAddressWithOffset()` goes wrong if
we are on a 32-bit platform whose addresses are regarded as signed;
in that case, just doing
```
(uint64_t)Section.getAddressWithOffset(...)
```
or
```
reinterpret_cast<uint64_t>(Section.getAddressWithOffset(...))
```
will result in sign-extension.

We use these expressions when constructing branch stubs, which is
before we know the final load address, so we can just switch to the
`Section.getLoadAddressWithOffset(...)` method instead.

Doing that is also more consistent, since when calculating relative
offsets for relocations, we use the load address anyway, so the
code currently only works because `Section.Address` is equal to
`Section.LoadAddress` at this point.

Fixes #94478.
---
 .../RuntimeDyld/RuntimeDyldELF.cpp            | 32 ++++++++-----------
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp
index 0046220611203..f44a6a472cb63 100644
--- a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp
+++ b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp
@@ -1183,8 +1183,7 @@ void RuntimeDyldELF::resolveAArch64Branch(unsigned SectionID,
   StubMap::const_iterator i = Stubs.find(Value);
   if (i != Stubs.end()) {
     resolveRelocation(Section, Offset,
-                      (uint64_t)Section.getAddressWithOffset(i->second),
-                      RelType, 0);
+                      Section.getLoadAddressWithOffset(i->second), RelType, 0);
     LLVM_DEBUG(dbgs() << " Stub function found\n");
   } else if (!resolveAArch64ShortBranch(SectionID, RelI, Value)) {
     // Create a new stub function.
@@ -1217,8 +1216,7 @@ void RuntimeDyldELF::resolveAArch64Branch(unsigned SectionID,
       addRelocationForSection(REmovk_g0, Value.SectionID);
     }
     resolveRelocation(Section, Offset,
-                      reinterpret_cast<uint64_t>(Section.getAddressWithOffset(
-                          Section.getStubOffset())),
+                      Section.getLoadAddressWithOffset(Section.getStubOffset()),
                       RelType, 0);
     Section.advanceStubOffset(getMaxStubSize());
   }
@@ -1349,10 +1347,9 @@ RuntimeDyldELF::processRelocationRef(
       // Look for an existing stub.
       StubMap::const_iterator i = Stubs.find(Value);
       if (i != Stubs.end()) {
-        resolveRelocation(
-            Section, Offset,
-            reinterpret_cast<uint64_t>(Section.getAddressWithOffset(i->second)),
-            RelType, 0);
+        resolveRelocation(Section, Offset,
+                          Section.getLoadAddressWithOffset(i->second), RelType,
+                          0);
         LLVM_DEBUG(dbgs() << " Stub function found\n");
       } else {
         // Create a new stub function.
@@ -1367,10 +1364,10 @@ RuntimeDyldELF::processRelocationRef(
         else
           addRelocationForSection(RE, Value.SectionID);
 
-        resolveRelocation(Section, Offset, reinterpret_cast<uint64_t>(
-                                               Section.getAddressWithOffset(
-                                                   Section.getStubOffset())),
-                          RelType, 0);
+        resolveRelocation(
+            Section, Offset,
+            Section.getLoadAddressWithOffset(Section.getStubOffset()), RelType,
+            0);
         Section.advanceStubOffset(getMaxStubSize());
       }
     } else {
@@ -1609,8 +1606,7 @@ RuntimeDyldELF::processRelocationRef(
         if (i != Stubs.end()) {
           // Symbol function stub already created, just relocate to it
           resolveRelocation(Section, Offset,
-                            reinterpret_cast<uint64_t>(
-                                Section.getAddressWithOffset(i->second)),
+                            Section.getLoadAddressWithOffset(i->second),
                             RelType, 0);
           LLVM_DEBUG(dbgs() << " Stub function found\n");
         } else {
@@ -1652,10 +1648,10 @@ RuntimeDyldELF::processRelocationRef(
             addRelocationForSection(REl, Value.SectionID);
           }
 
-          resolveRelocation(Section, Offset, reinterpret_cast<uint64_t>(
-                                                 Section.getAddressWithOffset(
-                                                     Section.getStubOffset())),
-                            RelType, 0);
+          resolveRelocation(
+              Section, Offset,
+              Section.getLoadAddressWithOffset(Section.getStubOffset()),
+              RelType, 0);
           Section.advanceStubOffset(getMaxStubSize());
         }
         if (IsExtern || (AbiVariant == 2 && Value.SectionID != SectionID)) {



More information about the llvm-commits mailing list