[llvm-branch-commits] [lld] 588b95a - [ELF][AArch64] Fix unneeded thunk for branches to hidden undefined weak

[Tom Stellard via llvm-branch-commits]

Author: Fangrui Song
Date: 2022-05-19T16:12:26-07:00
New Revision: 588b95a2b88ef36b6ac294032ef24cda70c1cf44

URL: https://github.com/llvm/llvm-project/commit/588b95a2b88ef36b6ac294032ef24cda70c1cf44
DIFF: https://github.com/llvm/llvm-project/commit/588b95a2b88ef36b6ac294032ef24cda70c1cf44.diff

LOG: [ELF][AArch64] Fix unneeded thunk for branches to hidden undefined weak

Similar to D119787 for PPC64.

A hidden undefined weak may change its binding to local before some
`isUndefinedWeak` code, so some `isUndefinedWeak` code needs to be changed to
`isUndefined`. The undefined non-weak case has been errored, so just using
`isUndefined` is fine.

The Linux kernel recently has a usage that a branch from 0xffff800008491ee0
references a hidden undefined weak symbol `vfio_group_set_kvm`.
It relies on the behavior that a branch to undefined weak resolving to the next
instruction, otherwise it'd see spurious relocation out of range errors.

Fixes https://github.com/ClangBuiltLinux/linux/issues/1624

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

(cherry picked from commit 02eab52866775f1476028129d1b114235fddc127)

Added: 
    

Modified: 
    lld/ELF/Arch/AArch64.cpp
    lld/ELF/InputSection.cpp
    lld/test/ELF/aarch64-undefined-weak.s

Removed: 
    


################################################################################
diff  --git a/lld/ELF/Arch/AArch64.cpp b/lld/ELF/Arch/AArch64.cpp
index add819ff5a80..193956fa9991 100644
--- a/lld/ELF/Arch/AArch64.cpp
+++ b/lld/ELF/Arch/AArch64.cpp
@@ -256,9 +256,11 @@ void AArch64::writePlt(uint8_t *buf, const Symbol &sym,
 bool AArch64::needsThunk(RelExpr expr, RelType type, const InputFile *file,
                          uint64_t branchAddr, const Symbol &s,
                          int64_t a) const {
-  // If s is an undefined weak symbol and does not have a PLT entry then it
-  // will be resolved as a branch to the next instruction.
-  if (s.isUndefWeak() && !s.isInPlt())
+  // If s is an undefined weak symbol and does not have a PLT entry then it will
+  // be resolved as a branch to the next instruction. If it is hidden, its
+  // binding has been converted to local, so we just check isUndefined() here. A
+  // undefined non-weak symbol will have been errored.
+  if (s.isUndefined() && !s.isInPlt())
     return false;
   // ELF for the ARM 64-bit architecture, section Call and Jump relocations
   // only permits range extension thunks for R_AARCH64_CALL26 and

diff  --git a/lld/ELF/InputSection.cpp b/lld/ELF/InputSection.cpp
index 4b047f75ad69..490254c995c8 100644
--- a/lld/ELF/InputSection.cpp
+++ b/lld/ELF/InputSection.cpp
@@ -733,11 +733,13 @@ uint64_t InputSectionBase::getRelocTargetVA(const InputFile *file, RelType type,
     if (expr == R_ARM_PCA)
       // Some PC relative ARM (Thumb) relocations align down the place.
       p = p & 0xfffffffc;
-    if (sym.isUndefWeak()) {
+    if (sym.isUndefined()) {
       // On ARM and AArch64 a branch to an undefined weak resolves to the next
       // instruction, otherwise the place. On RISCV, resolve an undefined weak
       // to the same instruction to cause an infinite loop (making the user
       // aware of the issue) while ensuring no overflow.
+      // Note: if the symbol is hidden, its binding has been converted to local,
+      // so we just check isUndefined() here.
       if (config->emachine == EM_ARM)
         dest = getARMUndefinedRelativeWeakVA(type, a, p);
       else if (config->emachine == EM_AARCH64)

diff  --git a/lld/test/ELF/aarch64-undefined-weak.s b/lld/test/ELF/aarch64-undefined-weak.s
index cd5abf3a22ea..f9c8edbf73db 100644
--- a/lld/test/ELF/aarch64-undefined-weak.s
+++ b/lld/test/ELF/aarch64-undefined-weak.s
@@ -10,6 +10,9 @@
 // is not generated.
  .weak target
 
+.weak undefweak2
+.hidden undefweak2
+
  .text
  .global _start
 _start:
@@ -36,6 +39,9 @@ _start:
 // R_AARCH64_PLT32
  .word target at PLT - .
 
+bl_undefweak2:
+ bl undefweak2
+
 // CHECK: Disassembly of section .text:
 // CHECK-EMPTY:
 // CHECK-NEXT: 0000000010010120 <_start>:
@@ -51,3 +57,5 @@ _start:
 // CHECK-NEXT: 10010144: 00 00 00 00     .word   0x00000000
 // CHECK-NEXT: 10010148: 00 00 00 00     .word   0x00000000
 // CHECK-NEXT: 1001014c: 00 00           .short  0x0000
+// CHECK-LABEL: <bl_undefweak2>:
+// CHECK-NEXT:    bl {{.*}} <bl_undefweak2+0x4>