[SR-1023] Build Error: Relocation R_X86_64_PC32

[swift-ci]

Previous ID	SR-1023
Radar	None
Original Reporter	rlovelett (JIRA User)
Type	Bug
Status	Closed
Resolution	Done

Attachment: Download

Environment

Arch Linux (4.4.5-1-ARCH)
Clang version 3.7.1 (tags/RELEASE_371/final)
GNU ld (GNU Binutils) 2.26.0.20160302

Additional Detail from JIRA

Votes	4
Component/s
Labels	Bug
Assignee	rlovelett (JIRA)
Priority	Medium

md5: 887c0afbe82cca6279b355238f4a2f89

is duplicated by:

• SR-1294 Ubuntu 16.04 swift fails to link: protected relocation issue

relates to:

• SR-1316 swift-package-manager link doesn't seem to respect the gold linker flag on Ubuntu

Issue Description:

Linking libswiftCore.so using binutils 2.26 fails during the relocation of a protected symbol. This issue is not present in 2.25 of binutils.

[511/536] Linking CXX shared library lib/swift/linux/x86_64/libswiftCore.so
FAILED: : && /usr/bin/clang++  -fPIC -fno-stack-protector -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Werror=date-time -std=c++11 -fcolor-diagnostics -ffunction-sections -fdata-sections -Wdocumentation -Wimplicit-fallthrough -Wunreachable-code -Woverloaded-virtual -O3  -Wl,-z,defs   -target x86_64-unknown-linux-gnu -isysroot / -lpthread -ldl  -L/home/ryan/build/Ninja-ReleaseAssert/swift-linux-x86_64/./lib/swift/linux/x86_64 -L/home/ryan/build/Ninja-ReleaseAssert/swift-linux-x86_64/./bin/../lib/swift/linux/x86_64 -L/home/ryan/build/Ninja-ReleaseAssert/swift-linux-x86_64/./bin/../lib/swift/linux -shared -Wl,-soname,libswiftCore.so -o lib/swift/linux/x86_64/libswiftCore.so lib/swift/linux/x86_64/swift_begin.o stdlib/public/core/linux/x86_64/Swift.o lib/swift/linux/x86_64/swift_end.o -L/home/ryan/build/Ninja-ReleaseAssert/llvm-linux-x86_64/lib  -L/home/ryan/build/Ninja-ReleaseAssert/llvm-linux-x86_64/./lib lib/swift/linux/x86_64/libswiftRuntime.a lib/swift/linux/x86_64/libswiftStdlibStubs.a -licuuc -licui18n -lbsd -Wl,-rpath,"\$ORIGIN:/usr/lib/swift/linux" && :
/usr/bin/ld: stdlib/public/core/linux/x86_64/Swift.o: relocation R_X86_64_PC32 against protected symbol `_TMPSa' can not be used when making a shared object
/usr/bin/ld: final link failed: Bad value

Using git bisect I was able to determine the commit that introduced the behavior of binutils between 2.25 and 2.26 to be ca3fe95e469b9daec153caa2c90665f5daaec2b5.

ca3fe95e469b9daec153caa2c90665f5daaec2b5 is the first bad commit
commit ca3fe95e469b9daec153caa2c90665f5daaec2b5
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Thu Mar 5 06:34:39 2015 -0800

    Add extern_protected_data and set it for x86
    
    With copy relocation, address of protected data defined in the shared
    library may be external.  This patch adds extern_protected_data and
    changes _bfd_elf_symbol_refs_local_p to return false for protected data
    if extern_protected_data is true.
    
    bfd/
    
        PR ld/pr15228
        PR ld/pr17709
        * elf-bfd.h (elf_backend_data): Add extern_protected_data.
        * elf32-i386.c (elf_backend_extern_protected_data): New.
        Defined to 1.
        * elf64-x86-64.c (elf_backend_extern_protected_data): Likewise.
        * elflink.c (_bfd_elf_adjust_dynamic_copy): Don't error on
        copy relocs against protected symbols if extern_protected_data
        is true.
        (_bfd_elf_symbol_refs_local_p): Don't return true on protected
        non-function symbols if extern_protected_data is true.
        * elfxx-target.h (elf_backend_extern_protected_data): New.
        Default to 0.
        (elfNN_bed): Initialize extern_protected_data with
        elf_backend_extern_protected_data.
    
    ld/testsuite/
    
        PR ld/pr15228
        PR ld/pr17709
        * ld-i386/i386.exp (i386tests): Add a test for PR ld/17709.
        * ld-i386/pr17709-nacl.rd: New file.
        * ld-i386/pr17709.rd: Likewise.
        * ld-i386/pr17709a.s: Likewise.
        * ld-i386/pr17709b.s: Likewise.
        * ld-i386/protected3.d: Updated.
        * ld-i386/protected3.s: Likewise.
        * ld-x86-64/pr17709-nacl.rd: New file.
        * ld-x86-64/pr17709.rd: Likewise.
        * ld-x86-64/pr17709a.s: Likewise.
        * ld-x86-64/pr17709b.s: Likewise.
        * ld-x86-64/protected3.d: Updated.
        * ld-x86-64/protected3.s: Likewise.
        * ld-x86-64/x86-64.exp (x86_64tests): Add a test for PR ld/17709.

:040000 040000 7fc861c288f9bed44c6444d1b04e2f6e688aeeaf fca3f6ce979e7c00ed44c04f506880015235806d M  bfd
:040000 040000 a5e358abb99b2b4089765f16904f9ebc496c0f12 7ccba1e77448a0155e56e8155073b40804b00daa M  ld

To perform the bisect I used 2bd25930221dea4bf33c13a89c111514491440e2 as the last known good commit and 71090e7a9dde8d28ff5c4b44d6d83e270d1088e4 as the last known bad commit.

[jckarter]

Is there a way then for the compiler to tell ld "no, really, always resolve this symbol to the shared library's version", short of passing -Bsymbolic or something potentially invasive to non-Swift object files?

[jckarter]

rlovelett (JIRA User) Thanks for figuring out where the problem is!

[jckarter]

The fix for this is in the driver or (hopefully) in the compiler, not the runtime.

[swift-ci]

Comment by Ryan Lovelett (JIRA)

@jckarter can you expand on that? I know that this might not be a starter bug but I'd like to finally fix something around here instead of just filing bugs.

When you say the driver do you mean this? Also which compiler, do you mean the clang compiler or the actual Swift compiler?

[jckarter]

If you have something like this:

int foo() { bar(); }
int bar() { }

Then the default behavior for an ELF shared library is for the call to bar from foo to be dynamically resolved by the dynamic linker, on the off chance that the symbol might be overridden by some other shared library. This dynamic resolution incurs load-time and runtime overhead that we don't want for Swift symbols, and we use relocations that dynamic linkers don't support, so we use protected visibility, which is supposed to tell the linker to resolve references to the protected symbol statically. It sounds like GNU ld broke this intentionally because "copy" relocations could theoretically cause a definition to be copied out of its home dylib, but we shouldn't run into situations where that's necessary in Swift. We really want references to symbols within a single dynamic library to be statically resolved.

[swift-ci]

Comment by Ryan Lovelett (JIRA)

First thanks for explaining that. Very helpful. Not sure if I am capable of fixing this yet.

But I'm trying to read more about the topic. I wonder do you think this issue is related? https://llvm.org/bugs/show_bug.cgi?id=23104

[jckarter]

Yeah, that's the same issue.

[jckarter]

I'd start by asking H.J. or the GNU ld list what options we have for requiring local references to protected symbols. If -Bsymbolic is the only choice, then so be it.

[jckarter]

It occurs to me that another approach here might be, instead of using protected visibility, to use a hidden alias symbol for local references within an image. So for each public symbol we'd have:

; Public symbol
@_Tfoo = global i32 0
; Hidden alias
@_tfoo = hidden alias i32 *@_Tfoo

and we'd reference @_tfoo locally.

[jckarter]

The problem with that would be that, if the symbol were copied into the executable's region, we'd have inconsistent metadata pointers. I hope there's a way to make copy relocations illegal on a symbol.

[jckarter]

I asked the binutils list:

https://sourceware.org/ml/binutils/2016-03/msg00312.html
https://sourceware.org/ml/binutils/2016-03/msg00313.html

H.J. recommends invoking the linker with -z nocopyreloc.

[octoploid]

See also: https://llvm.org/bugs/show_bug.cgi?id=26580

[swift-ci]

Comment by Ryan Lovelett (JIRA)

While I can't say I know what works. I can say something that doesn't. I tried applying the following patch to send -z nocopyreloc through to the linker (as suggested).

diff --git a/cmake/modules/AddSwift.cmake b/cmake/modules/AddSwift.cmake
index 9234f51..69d959e 100644
--- a/cmake/modules/AddSwift.cmake
+++ b/cmake/modules/AddSwift.cmake
@@ -1164,6 +1164,8 @@ function(_add_swift_library_single target name)
     list(APPEND link_flags "-fuse-ld=gold")
   endif()
 
+  list(APPEND link_flags "-Wl,-z,nocopyreloc")
+
   # Configure plist creation for OS X.
   set(PLIST_INFO_PLIST "Info.plist" CACHE STRING "Plist name")
   if(APPLE AND SWIFTLIB_SINGLE_IS_STDLIB)

I'll concede this might not be the correct way to pass things through to the linker. But I did notice it appears on the arguments sent to clang++.

[510/544] Linking CXX shared library lib/swift/linux/x86_64/libswiftCore.so
FAILED: : && /home/ryan/Source/build/Ninja-ReleaseAssert/llvm-linux-x86_64/bin/clang++  -fPIC -fno-stack-protector -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Werror=date-time -std=c++11 -fcolor-diagnostics -ffunction-sections -fdata-sections -Wdocumentation -Wimplicit-fallthrough -Wunreachable-code -Woverloaded-virtual -O3  -Wl,-z,defs   -target x86_64-unknown-linux-gnu -isysroot / -lpthread -ldl -Wl,-z,nocopyreloc  -L/home/ryan/Source/build/Ninja-ReleaseAssert/swift-linux-x86_64/./lib/swift/linux/x86_64 -L/home/ryan/Source/build/Ninja-ReleaseAssert/swift-linux-x86_64/./bin/../lib/swift/linux/x86_64 -L/home/ryan/Source/build/Ninja-ReleaseAssert/swift-linux-x86_64/./bin/../lib/swift/linux -shared -Wl,-soname,libswiftCore.so -o lib/swift/linux/x86_64/libswiftCore.so lib/swift/linux/x86_64/swift_begin.o stdlib/public/core/linux/x86_64/Swift.o lib/swift/linux/x86_64/swift_end.o -L/home/ryan/Source/build/Ninja-ReleaseAssert/llvm-linux-x86_64/lib  -L/home/ryan/Source/build/Ninja-ReleaseAssert/llvm-linux-x86_64/./lib lib/swift/linux/x86_64/libswiftRuntime.a lib/swift/linux/x86_64/libswiftStdlibStubs.a -licuuc -licui18n -Wl,-rpath,"\$ORIGIN:/usr/lib/swift/linux" && :
/usr/bin/ld: stdlib/public/core/linux/x86_64/Swift.o: relocation R_X86_64_PC32 against protected symbol `_TMPSa' can not be used when making a shared object

[jckarter]

The argument's been ongoing on the binutils list; it sounds like this may be a mistake on their part they'll backpedal. In the meantime, it sounds like we also need to pass -z noextern-protected-data to the linker as well to prevent it from treating a library's own protected symbols as external.

[swift-ci]

Comment by Ryan Lovelett (JIRA)

That does get it to compile now; thanks for that. 👍

diff --git a/cmake/modules/AddSwift.cmake b/cmake/modules/AddSwift.cmake
index 4010aac..8bb7c10 100644
--- a/cmake/modules/AddSwift.cmake
+++ b/cmake/modules/AddSwift.cmake
@@ -1164,6 +1164,9 @@ function(_add_swift_library_single target name)
     list(APPEND link_flags "-fuse-ld=gold")
   endif()
 
+  list(APPEND link_flags "-Wl,-z,nocopyreloc")
+  list(APPEND link_flags "-Wl,-z,noextern-protected-data")
+
   # Configure plist creation for OS X.
   set(PLIST_INFO_PLIST "Info.plist" CACHE STRING "Plist name")
   if(APPLE AND SWIFTLIB_SINGLE_IS_STDLIB)

I too have been following that thread on the binutils list that you started. I agree that it appears that someone does not think that should have been merged. Though I don't have enough visibility into the project to determine if the person who thinks it shouldn't be there actually has the authority to revert it.

My larger question is what is the Swift position on this? Is Swift going to accept patches to work-around these issues or is it just not going to support Ubuntu 16.04 until binutils is properly patched?

Let's say it does get reverted/patched upstream. That doesn't seem like a guarentee that'll make it into Ubuntu 16.04. I targeted that release because it is due in ~21 days and is going to ship with some version of binutils-2.26; likely one that will not link properly.