[PATCH] D107535: [ELF] Support copy relocation on non-default version symbols

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
MaskRay added a reviewer: peter.smith.
Herald added subscribers: arichardson, emaste.
Herald added a reviewer: alexander-shaposhnikov.
MaskRay requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Copy relocation on a non-default version symbol is unsupported and can crash at
runtime. Fortunately there is a one-line fix which works for most cases:
ensure `getSymbolsAt` unconditionally returns `ss`.

If two non-default version symbols are defined at the same place and both
are copy relocated, our implementation will copy relocated them into different
addresses and the pointer inequality may or may be an issue. In real world
the failing case is highly unlikely.

(
In glibc, sys_errlist at GLIBC_2.2.5 sys_errlist at GLIBC_2.3 sys_errlist at GLIBC_2.4
are defined at the same place, but it is unlikely they are all copy relocated in
one executable.
)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107535

Files:
  lld/ELF/Relocations.cpp
  lld/test/ELF/copy-rel-version.s


Index: lld/test/ELF/copy-rel-version.s
===================================================================
--- lld/test/ELF/copy-rel-version.s
+++ lld/test/ELF/copy-rel-version.s
@@ -1,15 +1,27 @@
-// REQUIRES: x86
-// RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
-// RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/copy-rel-version.s -o %t1.o
-// RUN: echo "v1 {}; v2 {};" > %t.ver
-// RUN: ld.lld %t1.o -shared -soname t1.so --version-script=%t.ver -o %t1.so
-// RUN: ld.lld %t.o %t1.so -o %t
-// RUN: llvm-readobj --symbols %t | FileCheck %s
+# REQUIRES: x86
+## Copy relocate a versioned symbol which has a versioned alias.
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t1.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %p/Inputs/copy-rel-version.s -o %t.o
+# RUN: echo 'v1 {}; v2 {};' > %t.ver
+# RUN: ld.lld %t.o -shared -soname t.so --version-script=%t.ver -o %t.so
+
+## Copy relocate the default version symbol.
+# RUN: ld.lld %t1.o %t.so -o %t1
+# RUN: llvm-readelf --dyn-syms %t1 | FileCheck %s --check-prefix=CHECK1
+
+# CHECK1:       1: {{.+}}             8 OBJECT  GLOBAL DEFAULT [[#]] foo at v2
+# CHECK1-EMPTY:
+
+## Copy relocate the non-default version symbol.
+# RUN: llvm-objcopy --redefine-sym foo=foo at v1 %t1.o %t2.o
+# RUN: ld.lld %t2.o %t.so -o %t2
+# RUN: llvm-readelf --dyn-syms %t2 | FileCheck %s --check-prefix=CHECK2
+
+# CHECK2:       1: [[ADDR:[0-9a-f]+]] 4 OBJECT  GLOBAL DEFAULT [[#]] foo at v1
+# CHECK2-NEXT:  2: [[ADDR]]           8 OBJECT  GLOBAL DEFAULT [[#]] foo at v2
+# CHECK2-EMPTY:
 
 .global _start
 _start:
   leaq foo, %rax
-
-// CHECK:      Name: foo (
-// CHECK-NEXT: Value:
-// CHECK-NEXT: Size: 8
Index: lld/ELF/Relocations.cpp
===================================================================
--- lld/ELF/Relocations.cpp
+++ lld/ELF/Relocations.cpp
@@ -527,6 +527,13 @@
     if (auto *alias = dyn_cast_or_null<SharedSymbol>(sym))
       ret.insert(alias);
   }
+
+  // The loop does not check SHT_GNU_verneed, so ret does not contain
+  // non-default version symbols. If ss has a non-default version, ret won't
+  // contain ss. Just add ss unconditionally. If a non-default version alias is
+  // separately copy relocated, it and ss will have different addresses.
+  // Fortunately this failing case is impractical.
+  ret.insert(&ss);
   return ret;
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D107535.364345.patch
Type: text/x-patch
Size: 2340 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210805/da2bf006/attachment.bin>