From e5de4f4e9170697b98ae10963b15ab1bdd69c82c Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Mon, 19 Aug 2019 06:17:30 +0000 Subject: [PATCH] [MC] Don't emit .symver redirected symbols to the symbol table GNU as keeps the original symbol in the symbol table for defined @ and @@, but suppresses it in other cases (@@@ or undefined). The original symbol is usually undesired: In a shared object, the original symbol can be localized with a version script, but it is hard to remove/localize in an archive: 1) a post-processing step removes the undesired original symbol 2) consumers (executable) of the archive are built with the version script Moreover, it can cause linker issues like binutils PR/18703 if the original symbol name and the base name of the versioned symbol is the same (both ld.bfd and gold have some code to work around defined @ and @@). In lld, if it sees f and f@v1: --version-script =(printf 'v1 {};') => f and f@v1 --version-script =(printf 'v1 { f; };') => f@v1 and f@@v1 It can be argued that @@@ added on 2000-11-13 corrected the @ and @@ mistake. This patch catches some more multiple version errors (defined @ and @@), and consistently suppress the original symbol. This addresses all the problems listed above. If the user wants other aliases to the versioned symbol, they can copy the original symbol to other symbol names with .set directive, e.g. .symver f, f@v1 # emit f@v1 but not f into .symtab .set f_impl, f # emit f_impl into .symtab git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@369233 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/MC/ELFObjectWriter.cpp | 24 +-- test/LTO/X86/symver-asm.ll | 4 - test/LTO/X86/symver-asm2.ll | 3 - test/MC/ARM/arm-elf-symver.s | 27 ---- test/MC/ELF/multiple-different-symver.s | 6 - test/MC/ELF/symver-multiple-version.s | 20 +++ test/MC/ELF/symver.s | 174 +++++++-------------- test/MC/PowerPC/ppc64-localentry-symbols.s | 2 - 8 files changed, 90 insertions(+), 170 deletions(-) delete mode 100644 test/MC/ELF/multiple-different-symver.s create mode 100644 test/MC/ELF/symver-multiple-version.s diff --git a/lib/MC/ELFObjectWriter.cpp b/lib/MC/ELFObjectWriter.cpp index 10a9fe87aa2..111ba3fab39 100644 --- a/lib/MC/ELFObjectWriter.cpp +++ b/lib/MC/ELFObjectWriter.cpp @@ -1285,8 +1285,21 @@ void ELFObjectWriter::executePostLayoutBinding(MCAssembler &Asm, Alias->setBinding(Symbol.getBinding()); Alias->setOther(Symbol.getOther()); - if (!Symbol.isUndefined() && !Rest.startswith("@@@")) + // Record the rename. This serves two purposes: 1) detect multiple symbol + // version definitions, 2) consistently suppress the original symbol in the + // symbol table. GNU as keeps the original symbol for defined @ and @@, but + // suppresses in for other cases (@@@ or undefined). The original symbol is + // usually undesired and difficult to remove in an archive. Moreoever, it + // can cause linker issues like binutils PR/18703. If the user wants other + // aliases to the versioned symbol, they can copy the original symbol to + // other symbol names with .set directive. + auto R = Renames.try_emplace(&Symbol, Alias); + if (!R.second && R.first->second != Alias) { + Asm.getContext().reportError( + SMLoc(), llvm::Twine("multiple symbol versions defined for ") + + Symbol.getName()); continue; + } // FIXME: Get source locations for these errors or diagnose them earlier. if (Symbol.isUndefined() && Rest.startswith("@@") && @@ -1295,15 +1308,6 @@ void ELFObjectWriter::executePostLayoutBinding(MCAssembler &Asm, " must be defined"); continue; } - - if (Renames.count(&Symbol) && Renames[&Symbol] != Alias) { - Asm.getContext().reportError( - SMLoc(), llvm::Twine("multiple symbol versions defined for ") + - Symbol.getName()); - continue; - } - - Renames.insert(std::make_pair(&Symbol, Alias)); } for (const MCSymbol *&Sym : AddrsigSyms) { diff --git a/test/LTO/X86/symver-asm.ll b/test/LTO/X86/symver-asm.ll index 85d032ba1ab..6797527e3ad 100644 --- a/test/LTO/X86/symver-asm.ll +++ b/test/LTO/X86/symver-asm.ll @@ -21,7 +21,6 @@ module asm ".symver foo,foo@@VER1" define i32 @io_cancel_0_4() { ; CHECK-DAG: T io_cancel@@LIBAIO_0.4 -; CHECK-DAG: T io_cancel_0_4 ret i32 0 } @@ -29,19 +28,16 @@ define internal i32 @io_cancel_local_0_4() { ; INTERN: llvm.compiler.used {{.*}} @io_cancel_local_0_4 ; INTERN: define internal i32 @io_cancel_local_0_4() ; CHECK-DAG: t io_cancel_local@@LIBAIO_0.4 -; CHECK-DAG: t io_cancel_local_0_4 ret i32 0 } define weak i32 @io_cancel_weak_0_4() { ; CHECK-DAG: W io_cancel_weak@@LIBAIO_0.4 ; CHECK-DAG: W io_cancel_weak@@LIBAIO_0.4.1 -; CHECK-DAG: W io_cancel_weak_0_4 ret i32 0 } define i32 @"\01foo"() { ; CHECK-DAG: T foo@@VER1 -; CHECK-DAG: T foo ret i32 0 } diff --git a/test/LTO/X86/symver-asm2.ll b/test/LTO/X86/symver-asm2.ll index 42d6e54bd06..6b7a542dcb1 100644 --- a/test/LTO/X86/symver-asm2.ll +++ b/test/LTO/X86/symver-asm2.ll @@ -22,9 +22,6 @@ module asm ".symver _start3, foo@SOME_VERSION3" module asm ".local _start2" module asm ".weak _start3" -; CHECK-DAG: T _start -; CHECK-DAG: t _start2 -; CHECK-DAG: W _start3 ; CHECK-DAG: T foo@@SOME_VERSION ; CHECK-DAG: t foo@SOME_VERSION2 ; CHECK-DAG: W foo@SOME_VERSION3 diff --git a/test/MC/ARM/arm-elf-symver.s b/test/MC/ARM/arm-elf-symver.s index 83313c103a1..e1f45459766 100644 --- a/test/MC/ARM/arm-elf-symver.s +++ b/test/MC/ARM/arm-elf-symver.s @@ -60,24 +60,6 @@ global1: @ CHECK-NEXT: Section: .text @ CHECK-NEXT: } @ CHECK-NEXT: Symbol { -@ CHECK-NEXT: Name: defined1 -@ CHECK-NEXT: Value: 0x0 -@ CHECK-NEXT: Size: 0 -@ CHECK-NEXT: Binding: Local (0x0) -@ CHECK-NEXT: Type: None (0x0) -@ CHECK-NEXT: Other: 0 -@ CHECK-NEXT: Section: .text -@ CHECK-NEXT: } -@ CHECK-NEXT: Symbol { -@ CHECK-NEXT: Name: defined2 -@ CHECK-NEXT: Value: 0x0 -@ CHECK-NEXT: Size: 0 -@ CHECK-NEXT: Binding: Local (0x0) -@ CHECK-NEXT: Type: None (0x0) -@ CHECK-NEXT: Other: 0 -@ CHECK-NEXT: Section: .text -@ CHECK-NEXT: } -@ CHECK-NEXT: Symbol { @ CHECK-NEXT: Name: .text (0) @ CHECK-NEXT: Value: 0x0 @ CHECK-NEXT: Size: 0 @@ -113,13 +95,4 @@ global1: @ CHECK-NEXT: Other: 0 @ CHECK-NEXT: Section: .text @ CHECK-NEXT: } -@ CHECK-NEXT: Symbol { -@ CHECK-NEXT: Name: global1 -@ CHECK-NEXT: Value: 0x14 -@ CHECK-NEXT: Size: 0 -@ CHECK-NEXT: Binding: Global (0x1) -@ CHECK-NEXT: Type: None (0x0) -@ CHECK-NEXT: Other: 0 -@ CHECK-NEXT: Section: .text -@ CHECK-NEXT: } @ CHECK-NEXT: ] diff --git a/test/MC/ELF/multiple-different-symver.s b/test/MC/ELF/multiple-different-symver.s deleted file mode 100644 index c34626c0817..00000000000 --- a/test/MC/ELF/multiple-different-symver.s +++ /dev/null @@ -1,6 +0,0 @@ -// RUN: not llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o %t 2>&1 | FileCheck %s - -// CHECK: error: multiple symbol versions defined for foo - -.symver foo, foo@1 -.symver foo, foo@2 diff --git a/test/MC/ELF/symver-multiple-version.s b/test/MC/ELF/symver-multiple-version.s new file mode 100644 index 00000000000..bb296e8d0d9 --- /dev/null +++ b/test/MC/ELF/symver-multiple-version.s @@ -0,0 +1,20 @@ +# RUN: not llvm-mc -filetype=obj -triple x86_64 %s -o %t 2>&1 | FileCheck %s + +# CHECK: error: multiple symbol versions defined for defined1 +# CHECK-NEXT: error: multiple symbol versions defined for defined2 +# CHECK-NEXT: error: multiple symbol versions defined for defined3 +# CHECK-NEXT: error: multiple symbol versions defined for undef + +defined1: +defined2: +defined3: + +.symver defined1, defined1@1 +.symver defined1, defined1@2 +.symver defined2, defined2@1 +.symver defined2, defined2@2 +.symver defined3, defined@@@1 +.symver defined3, defined@@@2 + +.symver undef, undef@1 +.symver undef, undef@2 diff --git a/test/MC/ELF/symver.s b/test/MC/ELF/symver.s index a591d5cec7b..5d01596e720 100644 --- a/test/MC/ELF/symver.s +++ b/test/MC/ELF/symver.s @@ -1,124 +1,62 @@ -// RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o - | llvm-readobj -r --symbols | FileCheck %s +# RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o %t.o +# RUN: llvm-readelf -s %t.o | FileCheck --check-prefix=SYM %s +# RUN: llvm-readobj -r %t.o | FileCheck --check-prefix=REL %s -defined1: -defined2: -defined3: - .symver defined1, bar1@zed - .symver undefined1, bar2@zed +.globl global1_impl, global2_impl, global3_impl +local1_impl: +local2_impl: +local3_impl: +global1_impl: +global2_impl: +global3_impl: - .symver defined2, bar3@@zed +# SYM: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND +# SYM-NEXT: 0000000000000000 0 NOTYPE LOCAL DEFAULT 2 local1@zed +# SYM-NEXT: 0000000000000000 0 NOTYPE LOCAL DEFAULT 2 local2@@zed +# SYM-NEXT: 0000000000000000 0 NOTYPE LOCAL DEFAULT 2 local3@@zed +# SYM-NEXT: 0000000000000000 0 SECTION LOCAL DEFAULT 2 +.symver local1_impl, local1@zed +.symver local2_impl, local2@@zed +.symver local3_impl, local3@@@zed - .symver defined3, bar5@@@zed - .symver undefined3, bar6@@@zed +# SYM-NEXT: 0000000000000000 0 NOTYPE GLOBAL DEFAULT 2 global1@zed +# SYM-NEXT: 0000000000000000 0 NOTYPE GLOBAL DEFAULT 2 global2@@zed +# SYM-NEXT: 0000000000000000 0 NOTYPE GLOBAL DEFAULT 2 global3@@zed +.symver global1_impl, global1@zed +.symver global2_impl, global2@@zed +.symver global3_impl, global3@@@zed - .long defined1 - .long undefined1 - .long defined2 - .long defined3 - .long undefined3 +## undef3_impl@@@zed emits a non-default versioned undef3_impl@zed. +# SYM-NEXT: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND undef1@zed +# SYM-NEXT: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND undef3@zed +.symver undef1_impl, undef1@zed +.symver undef3_impl, undef3@@@zed - .global global1 - .symver global1, g1@@zed -global1: +## GNU as emits {local,global}{1,2}_impl (.symver acted as a copy) but not +## {local,global}3_impl or undef{1,2,3}_impl (.symver acted as a rename). +## We consistently treat .symver as a rename and suppress the original symbols. +## This is advantageous because the original symbols are usually undesired +## and can easily cause issues like binutils PR/18703. +## If they want to retain the original symbol, +# SYM-NOT: {{.}} -// CHECK: Relocations [ -// CHECK-NEXT: Section {{.*}} .rela.text { -// CHECK-NEXT: 0x0 R_X86_64_32 .text 0x0 -// CHECK-NEXT: 0x4 R_X86_64_32 bar2@zed 0x0 -// CHECK-NEXT: 0x8 R_X86_64_32 .text 0x0 -// CHECK-NEXT: 0xC R_X86_64_32 .text 0x0 -// CHECK-NEXT: 0x10 R_X86_64_32 bar6@zed 0x0 -// CHECK-NEXT: } -// CHECK-NEXT: ] - -// CHECK: Symbol { -// CHECK: Name: bar1@zed -// CHECK-NEXT: Value: 0x0 -// CHECK-NEXT: Size: 0 -// CHECK-NEXT: Binding: Local -// CHECK-NEXT: Type: None -// CHECK-NEXT: Other: 0 -// CHECK-NEXT: Section: .text -// CHECK-NEXT: } -// CHECK-NEXT: Symbol { -// CHECK-NEXT: Name: bar3@@zed -// CHECK-NEXT: Value: 0x0 -// CHECK-NEXT: Size: 0 -// CHECK-NEXT: Binding: Local -// CHECK-NEXT: Type: None -// CHECK-NEXT: Other: 0 -// CHECK-NEXT: Section: .text -// CHECK-NEXT: } -// CHECK-NEXT: Symbol { -// CHECK-NEXT: Name: bar5@@zed -// CHECK-NEXT: Value: 0x0 -// CHECK-NEXT: Size: 0 -// CHECK-NEXT: Binding: Local -// CHECK-NEXT: Type: None -// CHECK-NEXT: Other: 0 -// CHECK-NEXT: Section: .text -// CHECK-NEXT: } -// CHECK-NEXT: Symbol { -// CHECK-NEXT: Name: defined1 -// CHECK-NEXT: Value: 0x0 -// CHECK-NEXT: Size: 0 -// CHECK-NEXT: Binding: Local -// CHECK-NEXT: Type: None -// CHECK-NEXT: Other: 0 -// CHECK-NEXT: Section: .text -// CHECK-NEXT: } -// CHECK-NEXT: Symbol { -// CHECK-NEXT: Name: defined2 -// CHECK-NEXT: Value: 0x0 -// CHECK-NEXT: Size: 0 -// CHECK-NEXT: Binding: Local -// CHECK-NEXT: Type: None -// CHECK-NEXT: Other: 0 -// CHECK-NEXT: Section: .text -// CHECK-NEXT: } -// CHECK-NEXT: Symbol { -// CHECK-NEXT: Name: .text (0) -// CHECK-NEXT: Value: 0x0 -// CHECK-NEXT: Size: 0 -// CHECK-NEXT: Binding: Local -// CHECK-NEXT: Type: Section -// CHECK-NEXT: Other: 0 -// CHECK-NEXT: Section: .text -// CHECK-NEXT: } -// CHECK-NEXT: Symbol { -// CHECK-NEXT: Name: bar2@zed -// CHECK-NEXT: Value: 0x0 -// CHECK-NEXT: Size: 0 -// CHECK-NEXT: Binding: Global -// CHECK-NEXT: Type: None -// CHECK-NEXT: Other: 0 -// CHECK-NEXT: Section: Undefined -// CHECK-NEXT: } -// CHECK-NEXT: Symbol { -// CHECK-NEXT: Name: bar6@zed -// CHECK-NEXT: Value: 0x0 -// CHECK-NEXT: Size: 0 -// CHECK-NEXT: Binding: Global -// CHECK-NEXT: Type: None -// CHECK-NEXT: Other: 0 -// CHECK-NEXT: Section: Undefined -// CHECK-NEXT: } -// CHECK-NEXT: Symbol { -// CHECK-NEXT: Name: g1@@zed -// CHECK-NEXT: Value: 0x14 -// CHECK-NEXT: Size: 0 -// CHECK-NEXT: Binding: Global -// CHECK-NEXT: Type: None -// CHECK-NEXT: Other: 0 -// CHECK-NEXT: Section: .text -// CHECK-NEXT: } -// CHECK-NEXT: Symbol { -// CHECK-NEXT: Name: global1 -// CHECK-NEXT: Value: 0x14 -// CHECK-NEXT: Size: 0 -// CHECK-NEXT: Binding: Global -// CHECK-NEXT: Type: None -// CHECK-NEXT: Other: 0 -// CHECK-NEXT: Section: .text -// CHECK-NEXT: } -// CHECK-NEXT: ] +# REL: Relocations [ +# REL-NEXT: Section {{.*}} .rela.text { +# REL-NEXT: 0x0 R_X86_64_32 .text 0x0 +# REL-NEXT: 0x4 R_X86_64_32 .text 0x0 +# REL-NEXT: 0x8 R_X86_64_32 .text 0x0 +# REL-NEXT: 0xC R_X86_64_32 global1@zed 0x0 +# REL-NEXT: 0x10 R_X86_64_32 global2@@zed 0x0 +# REL-NEXT: 0x14 R_X86_64_32 global3@@zed 0x0 +# REL-NEXT: 0x18 R_X86_64_32 undef1@zed 0x0 +# REL-NEXT: 0x1C R_X86_64_32 undef3@zed 0x0 +# REL-NEXT: } +# REL-NEXT: ] +.long local1_impl +.long local2_impl +.long local3_impl +.long global1_impl +.long global2_impl +.long global3_impl +.long undef1_impl +.long undef3_impl diff --git a/test/MC/PowerPC/ppc64-localentry-symbols.s b/test/MC/PowerPC/ppc64-localentry-symbols.s index f1d5c5d0ab1..927247a80bf 100644 --- a/test/MC/PowerPC/ppc64-localentry-symbols.s +++ b/test/MC/PowerPC/ppc64-localentry-symbols.s @@ -1,8 +1,6 @@ # RUN: llvm-mc -filetype=obj -triple=powerpc64-unknown-freebsd13.0 %s -o %t # RUN: llvm-objdump -t %t | FileCheck %s -# CHECK: 0000000000000000 gw F .text 00000000 0x60 __impl_foo -# CHECK: 0000000000000000 g F .text 00000000 0x60 foo # CHECK: 0000000000000000 gw F .text 00000000 0x60 foo@FBSD_1.1 # CHECK: 0000000000000008 g F .text 00000000 0x60 func # CHECK: 0000000000000008 gw F .text 00000000 0x60 weak_func -- 2.50.1