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

Jump to bottom

cmd/go: buildmode=pie broken with -linkmode=internal and cgo #18968

Closed
rahulchaudhry opened this issue Feb 6, 2017 · 23 comments
Closed

cmd/go: buildmode=pie broken with -linkmode=internal and cgo #18968

rahulchaudhry opened this issue Feb 6, 2017 · 23 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone
Go1.10

Comments

@rahulchaudhry
Copy link
Contributor

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

go version go1.8rc3 linux/amd64

What did you do?

$ go test std -buildmode=pie -short

30 packages in the standard library fail with segfault. The segfault happens very early on (appears to be in the dynamic linker/loader). In some cases, simply trying to load a test binary in gdb crashes gdb.

It worked fine with go1.7, so this is a regression.

Tried this on Gentoo, Ubuntu, and Debian. Same failures on all three systems.

Also tested linux/386, linux/arm, android/arm, and android/arm64. No issues there. This is specific to amd64 (perhaps related to #18820).

Here's what the output looks like (stripping out the packages that passed):
signal: segmentation fault (core dumped)
FAIL context 0.165s
signal: segmentation fault (core dumped)
FAIL crypto/tls 0.258s
signal: segmentation fault (core dumped)
FAIL crypto/x509 0.134s
signal: segmentation fault (core dumped)
FAIL debug/elf 0.134s
signal: segmentation fault (core dumped)
FAIL encoding/gob 0.151s
signal: segmentation fault (core dumped)
FAIL encoding/json 0.212s
signal: segmentation fault (core dumped)
FAIL encoding/pem 0.172s
signal: segmentation fault (core dumped)
FAIL expvar 0.211s
signal: segmentation fault (core dumped)
FAIL log/syslog 0.152s
signal: segmentation fault (core dumped)
FAIL mime/multipart 0.145s
signal: segmentation fault (core dumped)
FAIL net 0.169s
signal: segmentation fault (core dumped)
FAIL net/http 0.272s
signal: segmentation fault (core dumped)
FAIL net/http/cgi 0.211s
signal: segmentation fault (core dumped)
FAIL net/http/cookiejar 0.221s
signal: segmentation fault (core dumped)
FAIL net/http/fcgi 0.182s
signal: segmentation fault (core dumped)
FAIL net/http/httptest 0.198s
signal: segmentation fault (core dumped)
FAIL net/http/httptrace 0.219s
signal: segmentation fault (core dumped)
FAIL net/http/httputil 0.198s
signal: segmentation fault (core dumped)
FAIL net/mail 0.146s
signal: segmentation fault (core dumped)
FAIL net/rpc 0.310s
signal: segmentation fault (core dumped)
FAIL net/rpc/jsonrpc 0.306s
signal: segmentation fault (core dumped)
FAIL net/smtp 0.170s
signal: segmentation fault (core dumped)
FAIL net/textproto 0.142s
signal: segmentation fault (core dumped)
FAIL net/url 0.201s
signal: segmentation fault (core dumped)
FAIL os/exec 0.205s
signal: segmentation fault (core dumped)
FAIL os/user 0.188s
signal: segmentation fault (core dumped)
FAIL runtime 0.206s
signal: segmentation fault (core dumped)
FAIL runtime/trace 0.147s
signal: segmentation fault (core dumped)
FAIL syscall 0.157s
signal: segmentation fault (core dumped)
FAIL vendor/golang_org/x/net/lex/httplex 0.149s
@bradfitz bradfitz changed the title cmd/go: buildmode=pie broken bigly cmd/go: buildmode=pie broken on Go 1.8 Feb 6, 2017
@bradfitz bradfitz added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 6, 2017
@bradfitz bradfitz added this to the Go1.9 milestone Feb 6, 2017
@bradfitz
Copy link
Contributor

bradfitz commented Feb 6, 2017

@ianlancetaylor, @crawshaw, is this a Go 1.8 worry?

@crawshaw
Copy link
Member

crawshaw commented Feb 6, 2017

I'm a little worried. Will prod at it a bit tonight.

Any chance that running one of these tests under gdb gives you a useful traceback?

@rahulchaudhry
Copy link
Contributor Author

This is the backtrace I see for package "context":

$ go test -buildmode=pie -c context
$ gdb context.test
(gdb) run
Program received signal SIGSEGV, Segmentation fault.
(gdb) backtrace
#0 0x00007ffff7de6695 in elf_machine_rela (reloc=0x555555680f90, reloc=0x555555680f90, skip_ifunc=, reloc_addr_arg=, version=, sym=0x55555572c118, map=0x7ffff7ffe1c8) at ../sysdeps/x86_64/dl-machine.h:410
#1 elf_dynamic_do_Rela (skip_ifunc=, lazy=, nrelative=, relsize=, reladdr=, map=0x7ffff7ffe1c8) at do-rel.h:137
#2 _dl_relocate_object (scope=, reloc_mode=, consider_profiling=, consider_profiling@entry=0) at dl-reloc.c:264
#3 0x00007ffff7dddaca in dl_main (phdr=, phdr@entry=0x555555554040, phnum=, phnum@entry=11, user_entry=user_entry@entry=0x7fffffffdbc8, auxv=) at rtld.c:2200
#4 0x00007ffff7df1525 in _dl_sysdep_start (start_argptr=start_argptr@entry=0x7fffffffdcb0, dl_main=dl_main@entry=0x7ffff7ddb910 <dl_main>) at ../elf/dl-sysdep.c:249
#5 0x00007ffff7ddecc8 in _dl_start_final (arg=0x7fffffffdcb0) at rtld.c:331
#6 _dl_start (arg=0x7fffffffdcb0) at rtld.c:557
#7 0x00007ffff7ddb2d8 in _start () at rtld.c:871
#8 0x0000000000000001 in ?? ()
#9 0x00007fffffffe06d in ?? ()
#10 0x0000000000000000 in ?? ()

@rahulchaudhry
Copy link
Contributor Author

I did a bisection for package "context".
It points to https://go-review.googlesource.com/#/c/30370: context: make DeadlineExceeded implement net.Error

This change somehow broke -buildmode=pie for this package. Maybe that's a hint (a local type implementing an interface it didn't before).

Did another bisect for package "encoding/gob".
It points to https://go-review.googlesource.com/#/c/28971: cmd/link: attempt to rationalize linkmode init

@rsc
Copy link
Contributor

rsc commented Feb 6, 2017

From the stack trace it looks like the dynamic loader cannot even get the Go binary started. That is, it's not the generated Go code or the assembly that is the problem but something in the binary format itself, probably a bad relocation of some kind.

/cc @ianlancetaylor in case this is obvious.

@rsc rsc modified the milestones: Go1.8, Go1.9 Feb 6, 2017
@ianlancetaylor
Copy link
Contributor

It works in 1.7 because it uses external linking.

It fails in 1.8 because it uses internal linking.

@rsc
Copy link
Contributor

rsc commented Feb 6, 2017

Should -buildmode=pie imply external linking?

@ianlancetaylor
Copy link
Contributor

It used to. In 1.8 @crawshaw added support for internal linking of PIE. But I thought it was disabled because of #17068. Trying to figure out the current state.

@ianlancetaylor
Copy link
Contributor

ianlancetaylor commented Feb 6, 2017 •
edited

Looks like it was re-enabled again. Now trying to figure out whether it was always broken or whether it broke sometime after that.

The specific problem is that the offset field of the jump slot relocations is always zero.

@crawshaw
Copy link
Member

crawshaw commented Feb 6, 2017

Internal linking of PIE should be off by default. If it got turned back on by one of my other changes, that was a mistake.

@ianlancetaylor
Copy link
Contributor

It was turned on by https://golang.org/cl/28971.

@crawshaw
Copy link
Member

crawshaw commented Feb 6, 2017

I think for 1.8 we should have it off unless -linkmode is set explicitly. (I'll send a CL in a moment.)

@gopherbot
Copy link

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

@crawshaw
Copy link
Member

crawshaw commented Feb 6, 2017

OK, CL 36417 does the obvious conservative thing for 1.8 and defaults to external linking. But let's keep this bug opened for 1.9.

gopherbot pushed a commit that referenced this issue Feb 6, 2017
@crawshaw
cmd/link: use external linking for PIE by default
ab067cd 
Now `go test -buildmode=pie std -short` passes on linux/amd64.

Updates #18968

Change-Id: Ide21877713e00edc64c1700c950016d6bff8de0e
Reviewed-on: https://go-review.googlesource.com/36417
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@ianlancetaylor ianlancetaylor mentioned this issue Feb 6, 2017
Closed
@ianlancetaylor ianlancetaylor modified the milestones: Go1.9, Go1.8 Feb 6, 2017
@ianlancetaylor ianlancetaylor changed the title cmd/go: buildmode=pie broken on Go 1.8 cmd/go: buildmode=pie broken with -linkmode=internal and cgo Feb 6, 2017
@ianlancetaylor ianlancetaylor removed their assignment Feb 6, 2017
@gopherbot
Copy link

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

gopherbot pushed a commit that referenced this issue Feb 7, 2017
@crawshaw @rsc
[release-branch.go1.8] cmd/link: use external linking for PIE by default
f0749fe 
Now `go test -buildmode=pie std -short` passes on linux/amd64.

Updates #18968

Change-Id: Ide21877713e00edc64c1700c950016d6bff8de0e
Reviewed-on: https://go-review.googlesource.com/36417
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/36421
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Minux Ma <minux@golang.org>
@crawshaw
Copy link
Member

crawshaw commented Feb 14, 2017 •
edited

I bisected this using the net package tests, which use cgo. It broke with 5940a00. So my fix for #17068 was incomplete.

@ianlancetaylor
Copy link
Contributor

@crawshaw I didn't try to analyze it, beyond observing that the offset for the JMP_SLOT relocations is zero. The bug occurs when using internal linking with cgo. That is, the internal linker does not correctly handle some ELF relocations.

@rsc
Copy link
Contributor

rsc commented Jun 22, 2017

This is not an ongoing concern until we try to push PIE builds back to internal linking. Perhaps we can add a test that go test net -linkmode=pie works on linux/amd64 and close this issue.

@gopherbot
Copy link

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

@bradfitz bradfitz modified the milestones: Go1.10, Go1.9 Jun 26, 2017
@bradfitz bradfitz added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jun 26, 2017
@bradfitz bradfitz assigned rsc and unassigned crawshaw Jun 26, 2017
prattmic added a commit to prattmic/go that referenced this issue Jul 8, 2017
@prattmic
cmd/link: skip R_ADDR relocs in .rela.plt for internal PIE
ba6637b 
ld.addpltsym adds an R_X86_64_JMP_SLOT dynamic relocation to .rela.plt
and uses Addaddrplus to reference the GOT in Elf64_Rela.r_offset.

Addaddrplus results in an R_ADDR relocation, which here we transform
into an R_X86_64_64 dynamic relocation. This is wrong for several
reasons:

1. .rela.plt is not a writable, relro section. It is mapped read-only,
   causing the dynamic linker to segfault when it tried to handle the
   relocation. This was the immediate cause of internal PIE cgo
   crashes.

2. Relocations targetting other reloc sections are, as far as I can
   tell, undefined behavior in the ELF spec and are unlikely to be a
   good idea.

3. Even if the relocation did work, it isn't what we want. The
   relocation, if successfully handled, would have put an absolute
   address as the JMP_SLOT offset, but it should be the offset from the
   beginning of the binary, just like any other relocation. What we want
   is a statically resolved R_ADDR relocation, just as is used below for
   the R_X86_64_64 relocation.

Skipping the .rela.plt allows reloc() to handle these R_ADDR
relocations.

With this CL, internal PIE cgo binaries work.

Fixes golang#18968

Change-Id: Ie74e6fe249e88150baa0e340b1cb128cf7f28673
@gopherbot
Copy link

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

gopherbot pushed a commit that referenced this issue Jul 9, 2017
@prattmic @ianlancetaylor
cmd/link: skip R_ADDR relocs in .rela.plt for internal PIE
123fd46 
ld.addpltsym adds an R_X86_64_JMP_SLOT dynamic relocation to .rela.plt
and uses Addaddrplus to reference the GOT in Elf64_Rela.r_offset.

Addaddrplus results in an R_ADDR relocation, which here we transform
into an R_X86_64_64 dynamic relocation. This is wrong for several
reasons:

1. .rela.plt is not a writable, relro section. It is mapped read-only,
   causing the dynamic linker to segfault when it tried to handle the
   relocation. This was the immediate cause of internal PIE cgo
   crashes.

2. Relocations targetting other reloc sections are, as far as I can
   tell, undefined behavior in the ELF spec and are unlikely to be a
   good idea.

3. Even if the relocation did work, it isn't what we want. The
   relocation, if successfully handled, would have put an absolute
   address as the JMP_SLOT offset, but it should be the offset from the
   beginning of the binary, just like any other relocation. What we want
   is a statically resolved R_ADDR relocation, just as is used below for
   the R_X86_64_64 relocation.

Skipping the .rela.plt allows reloc() to handle these R_ADDR
relocations.

With this CL, internal PIE cgo binaries work.

Updates #18968

Change-Id: Ie74e6fe249e88150baa0e340b1cb128cf7f28673
Reviewed-on: https://go-review.googlesource.com/47837
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/54790 mentions this issue: cmd/go, cmd/link: enable buildmode=pie on darwin/amd64

gopherbot pushed a commit that referenced this issue Aug 14, 2017
@hirochachacha @ianlancetaylor
cmd/go, cmd/link: enable buildmode=pie on darwin/amd64
b7c600d 
Change some configurations to enable the feature. Also add the test.
This CL doesn't include internal linking support which is tentatively
disabled due to #18968. We could do that another day.

Fixes #21220

Change-Id: I601d2d78446d36332acc70be0d5b9461ac635208
Reviewed-on: https://go-review.googlesource.com/54790
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/57231 mentions this issue: cmd/go: -buildmode=pie forces external linking mode on all systems

gopherbot pushed a commit that referenced this issue Aug 21, 2017
@ianlancetaylor
cmd/go: -buildmode=pie forces external linking mode on all systems
eccd3ef 
The go tool assumed that -buildmode=pie implied internal linking on
linux-amd64. However, that was changed by CL 36417 for issue #18968.

Fixes #21452

Change-Id: I8ed13aea52959cc5c53223f4c41ba35329445545
Reviewed-on: https://go-review.googlesource.com/57231
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Avelino <t@avelino.xxx>
Reviewed-by: Rob Pike <r@golang.org>
@rsc
Copy link
Contributor

rsc commented Dec 1, 2017

buildmode=pie now forces external linking on all systems, so this sounds fixed.

@rsc rsc closed this as completed Dec 1, 2017
@golang golang locked and limited conversation to collaborators Dec 1, 2018
@rsc rsc removed their assignment Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Milestone
Go1.10
Development

No branches or pull requests
6 participants
@bradfitz @rsc @crawshaw @ianlancetaylor @gopherbot @rahulchaudhry