Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/link: failure of 'internal linking of -buildmode=pie' test on (some) Linuxes #17068

Closed
siebenmann opened this issue Sep 12, 2016 · 16 comments

Comments

Projects
None yet
5 participants
@siebenmann
Copy link

commented Sep 12, 2016

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version devel +4ba2a49 Sun Sep 11 23:38:44 2016 +0000 linux/amd64

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOOS="linux"

What did you do?

On 64-bit Fedora 24 and Ubuntu 16.04 (at least) go from source using the git tip fails currently with the error:

##### internal linking of -buildmode=pie
signal: segmentation fault
FAIL    reflect 0.005s
2016/09/11 23:24:09 Failed: exit status 1

According to git bisect, this problem first appears in commit 1a42d8f. This problem doesn't happen on Ubuntu 14.04 amd64, where the test passes:

##### internal linking of -buildmode=pie
ok      reflect 0.075s
@mwhudson

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2016

@mwhudson

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2016

However it does fail locally for me. Maybe it's a dependence on kernel version.

It seems that runtime.firstmoduledata lacks any relocations. In fact it seems that the linker lacks any facility to emit relative relocations at all, so I don't understand how this could possibly work – how can an executable with static data be PIE without relative relocations?

/cc @ianlancetaylor @crawshaw

@ianlancetaylor ianlancetaylor added this to the Go1.8 milestone Sep 12, 2016

@crawshaw

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2016

Something has gone very wrong. Somewhere in my patch series I must have broken something, and my old Ubuntu 14.04 isn't showing it. Looking into it.

@crawshaw

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2016

(I think this is some fallout from my backing off adding BuildmodePIE to the DynlinkingGo function in cmd/link.)

@crawshaw

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2016

Ah yes, I see the problem: it's me. When I was using DynlinkingGo, I was tripping the other condition for host linking without realizing, so all of my original poking around the output for the right relocations was done on a host linked binary. Then I changed it so I don't host link, but by that point I was just testing all my binaries worked, and my linux system is far too forgiving. So I have a bunch more code to write to generate all the appropriate relocations.

I'll send a CL disabling internal PIE for now, as this may take a few days.

gopherbot pushed a commit that referenced this issue Sep 12, 2016

cmd/link: disable internal PIE for now
There's more work to do.

Updates #17068

Change-Id: I4e16c0e8e9ac739e1fe266224c3769f6c5b2e070
Reviewed-on: https://go-review.googlesource.com/29076
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@siebenmann

This comment has been minimized.

Copy link
Author

commented Sep 12, 2016

Even after pulling the above commit I still see the same test failure:

##### internal linking of -buildmode=pie
signal: segmentation fault
FAIL    reflect 0.006s
2016/09/12 14:33:46 Failed: exit status 1
@crawshaw

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2016

@siebenmann could I impose on you to try undoing the second part of 1a42d8f and see if that fixes it? That is, turn cmd/link/internal/ld/lib.go:639 from

    if Buildmode == BuildmodeExe {

into

    if Buildmode == BuildmodeExe || Buildmode == BuildmodePIE {

I don't know why that would matter, but as the comment above it says, it's a very unusual state for a binary as far as glibc is concerned.

@siebenmann

This comment has been minimized.

Copy link
Author

commented Sep 12, 2016

Making this change still leaves the test failing with a segmentation fault.

@crawshaw

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2016

Oh, the comment on the code I just changed is a little misleading. It doesn't force external linking, it just sets it to external if no link mode was set.

For now I'll disable the test in cmd/dist.

@gopherbot

This comment has been minimized.

Copy link

commented Sep 12, 2016

CL https://golang.org/cl/29079 mentions this issue.

@siebenmann

This comment has been minimized.

Copy link
Author

commented Sep 12, 2016

With the change from this CL applied by hand, an all.bash build on my machine succeeds.

gopherbot pushed a commit that referenced this issue Sep 12, 2016

cmd/dist: disable test of internal PIE linking
Updates #17068

Change-Id: I61b75ec07ca8705a678677d262e11b16848cddf3
Reviewed-on: https://go-review.googlesource.com/29079
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com>
@crawshaw

This comment has been minimized.

Copy link
Contributor

commented Sep 13, 2016

CL 29079 was a wild red herring chase. Starting from the beginning:

On Ubuntu 14.04 I can successfully build and run a hello world program with

go build -buildmode=pie -ldflags=-linkmode=internal hello.go

On Ubuntu 16.04 it crashes. It also crashed on 14.04 if it is run under gdb. It gives a nice clear error when it does so:

(gdb) run
Starting program: /usr/local/google/home/crawshaw/go/src/junk 

Program received signal SIGSEGV, Segmentation fault.
runtime.moduledataverify1 (datap=0x5555555f8620 <runtime.firstmoduledata>) at /usr/local/google/home/crawshaw/go/src/runtime/symtab.go:259
259             if pcln32[0] != 0xfffffffb || pcln[4] != 0 || pcln[5] != 0 || pcln[6] != sys.PCQuantum || pcln[7] != sys.PtrSize {
(gdb) bt
#0  runtime.moduledataverify1 (datap=0x5555555f8620 <runtime.firstmoduledata>) at /usr/local/google/home/crawshaw/go/src/runtime/symtab.go:259
#1  0x000055555558af89 in runtime.moduledataverify () at /usr/local/google/home/crawshaw/go/src/runtime/symtab.go:247
#2  0x0000555555578e77 in runtime.schedinit () at /usr/local/google/home/crawshaw/go/src/runtime/proc.go:438
#3  0x000055555559bd91 in runtime.rt0_go () at /usr/local/google/home/crawshaw/go/src/runtime/asm_amd64.s:145

That combined with Ian's explanation on CL 29079 makes everything clear, because gdb loads PIE at a fixed and unusual address. A relocation that needs to be statically resolved as PC-relative is instead being statically resolved as something else, probably absolute to the beginning of the program.

@mwhudson

This comment has been minimized.

Copy link
Contributor

commented Sep 13, 2016

I don't see anything from Ian on CL 29079?

I think the problem is that not nearly enough relocations are being emitted:

(master)mwhudson@aeglos:/opt/opensource/go/src$ go test reflect -o r1.ext -c -buildmode=pie -ldflags=-linkmode=external
(master)mwhudson@aeglos:/opt/opensource/go/src$ go test reflect -o r1.int -c -buildmode=pie -ldflags=-linkmode=internal
(master)mwhudson@aeglos:/opt/opensource/go/src$ readelf -r r1.int | wc -l
2209
(master)mwhudson@aeglos:/opt/opensource/go/src$ readelf -r r1.ext | wc -l
35366

I got confused a bit by the lack of relative relocations but of course R_X86_64_64 should work fine, even if it's a bit inefficient. But there aren't enough of them. There's this in adddynrel:

        if s.Type != obj.SDATA && s.Type != obj.SRODATA {
            break
        }

which looks a bit suspicious but the naive thing of adding "&& Buildmode != BuildmodePIE" on the end makes the linker consume unbounded amounts of RAM...

@mwhudson

This comment has been minimized.

Copy link
Contributor

commented Sep 13, 2016

Ah you mean CL 29090. And I think that is a red herring: this isn't about code->data references, it's about data->data references.

@gopherbot

This comment has been minimized.

Copy link

commented Sep 14, 2016

CL https://golang.org/cl/29119 mentions this issue.

@gopherbot

This comment has been minimized.

Copy link

commented Sep 14, 2016

CL https://golang.org/cl/29118 mentions this issue.

@gopherbot gopherbot closed this in 5940a00 Sep 14, 2016

gopherbot pushed a commit that referenced this issue Sep 14, 2016

cmd/dist: re-enable internal PIE test
For #17068

Change-Id: I4e3ab166f08100292b779b651a9acfbfb44a55cd
Reviewed-on: https://go-review.googlesource.com/29119
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

@golang golang locked and limited conversation to collaborators Sep 14, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.