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/go: reject use of PIE with -race #20038

Closed
aaronjwood opened this Issue Apr 19, 2017 · 18 comments

Comments

Projects
None yet
7 participants
@aaronjwood

aaronjwood commented Apr 19, 2017

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

1.7.4

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

OS: CoreOS 1010.6.0 (Linux 4.5.7)
Arch: x86_64

What did you do?

Built a binary with -race in our CI environment (also x86_64 Linux but not CoreOS), ran it on CoreOS, immediately got a segfault.

What did you expect to see?

No segfault :) Building this same binary without the race detector and running it on CoreOS works just fine.

What did you see instead?

==45515==ERROR: ThreadSanitizer failed to allocate 0x2790000 (41484288) bytes at address 1775919824880 (errno: 12)
unexpected fault address 0x0
fatal error: fault
[signal SIGSEGV: segmentation violation code=0x80 addr=0x0 pc=0x55d645f23ea5]

goroutine 1 [running, locked to thread]:
runtime.throw(0x0, 0x7fff2385c478)
/usr/local/go/src/runtime/panic.go:566 +0x97 fp=0x7fff2385c408 sp=0x7fff2385c3e8

goroutine 17 [syscall, locked to thread]:
runtime.goexit()
/usr/local/go/src/runtime/asm_amd64.s:2086 +0x1

One other important detail: this binary was run under Mesos 1.0.3. The only resource restrictions in place (applied via cgroups) were 4 GB of memory and 4 (relative) CPU shares.

@bradfitz bradfitz changed the title from Binary built with race detector segfaults on CoreOS 1010.6.0 with 4.5.7 kernel to runtime/race: binary built with race detector segfaults on CoreOS 1010.6.0 with 4.5.7 kernel Apr 19, 2017

@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz
Member

bradfitz commented Apr 19, 2017

@bradfitz bradfitz added this to the Go1.9Maybe milestone Apr 19, 2017

@davecheney

This comment has been minimized.

Show comment
Hide comment
@davecheney

davecheney Apr 19, 2017

Contributor
Contributor

davecheney commented Apr 19, 2017

@dvyukov

This comment has been minimized.

Show comment
Hide comment
@dvyukov

dvyukov Apr 19, 2017

Member

How much memory does the program use without race detector?

Member

dvyukov commented Apr 19, 2017

How much memory does the program use without race detector?

@aaronjwood

This comment has been minimized.

Show comment
Hide comment
@aaronjwood

aaronjwood Apr 19, 2017

@dvyukov I was doing some load testing (nothing too crazy) the other day and I found that it never reached 15 MB. I'd say it generally takes between 10-15 MB.

aaronjwood commented Apr 19, 2017

@dvyukov I was doing some load testing (nothing too crazy) the other day and I found that it never reached 15 MB. I'd say it generally takes between 10-15 MB.

@aaronjwood

This comment has been minimized.

Show comment
Hide comment
@aaronjwood

aaronjwood Apr 19, 2017

@davecheney does it sound strange that the race detector would require 8-16 GB of memory for a program that takes 15 or maybe 20 MB at the very most?

aaronjwood commented Apr 19, 2017

@davecheney does it sound strange that the race detector would require 8-16 GB of memory for a program that takes 15 or maybe 20 MB at the very most?

@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz

bradfitz Apr 19, 2017

Member

@aaronjwood, is your container limiting actual memory usage or also allocating virtual memory addresses?

Member

bradfitz commented Apr 19, 2017

@aaronjwood, is your container limiting actual memory usage or also allocating virtual memory addresses?

@dvyukov

This comment has been minimized.

Show comment
Hide comment
@dvyukov

dvyukov Apr 19, 2017

Member

Please provide repro instructions.

Member

dvyukov commented Apr 19, 2017

Please provide repro instructions.

@aaronjwood

This comment has been minimized.

Show comment
Hide comment
@aaronjwood

aaronjwood Apr 19, 2017

@dvyukov actually I have some updates on this. I found that it's not the race detector that's causing the issue, it's when we build the binary as a PIE executable AND use the race detector. Before when we were building we passed -buildmode=pie -race. I had taken both of those out when testing with and without the race detector so I wrongly assumed it was the race detector. This seems to be a combination of a PIE executable with the race detector. Building with -buildmode=pie by itself works just fine.

aaronjwood commented Apr 19, 2017

@dvyukov actually I have some updates on this. I found that it's not the race detector that's causing the issue, it's when we build the binary as a PIE executable AND use the race detector. Before when we were building we passed -buildmode=pie -race. I had taken both of those out when testing with and without the race detector so I wrongly assumed it was the race detector. This seems to be a combination of a PIE executable with the race detector. Building with -buildmode=pie by itself works just fine.

@aaronjwood aaronjwood changed the title from runtime/race: binary built with race detector segfaults on CoreOS 1010.6.0 with 4.5.7 kernel to runtime/race: binary built with race detector and -buildmode=pie segfaults on CoreOS 1010.6.0 with 4.5.7 kernel Apr 19, 2017

@dvyukov

This comment has been minimized.

Show comment
Hide comment
@dvyukov

dvyukov Apr 19, 2017

Member

Uh, that's not supported. Race runtime assumes that executable is loaded at around 0 address. I guess we need to prohibit this combination for now.

Member

dvyukov commented Apr 19, 2017

Uh, that's not supported. Race runtime assumes that executable is loaded at around 0 address. I guess we need to prohibit this combination for now.

@aaronjwood

This comment has been minimized.

Show comment
Hide comment
@aaronjwood

aaronjwood Apr 19, 2017

I see, I didn't realize that. If that's the case then I guess we can simply close this. Maybe some docs or a change like you suggested would help others that may run into this in the future.

aaronjwood commented Apr 19, 2017

I see, I didn't realize that. If that's the case then I guess we can simply close this. Maybe some docs or a change like you suggested would help others that may run into this in the future.

@bradfitz bradfitz added the NeedsFix label Apr 19, 2017

@bradfitz bradfitz modified the milestones: Go1.9, Go1.9Maybe Apr 19, 2017

@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz

bradfitz Apr 19, 2017

Member

Rejecting that combination in cmd/go SGTM.

Member

bradfitz commented Apr 19, 2017

Rejecting that combination in cmd/go SGTM.

@bradfitz bradfitz changed the title from runtime/race: binary built with race detector and -buildmode=pie segfaults on CoreOS 1010.6.0 with 4.5.7 kernel to cmd/go: reject use of PIE with -race Apr 21, 2017

@dajoo75

This comment has been minimized.

Show comment
Hide comment
@dajoo75

dajoo75 Apr 21, 2017

Contributor

I uploaded a fix for review: https://go-review.googlesource.com/c/41335/

Contributor

dajoo75 commented Apr 21, 2017

I uploaded a fix for review: https://go-review.googlesource.com/c/41335/

@ALTree

This comment has been minimized.

Show comment
Hide comment
@ALTree

ALTree Apr 21, 2017

Member

Is the gopherbot sick? It's not crossreferencing CLs, I uploaded a patch hours ago but it didn't post on the issue.

Member

ALTree commented Apr 21, 2017

Is the gopherbot sick? It's not crossreferencing CLs, I uploaded a patch hours ago but it didn't post on the issue.

@dajoo75

This comment has been minimized.

Show comment
Hide comment
@dajoo75

dajoo75 Apr 21, 2017

Contributor

Maybe on sick leave? ;-). Anyway, please ignore my fix if there was already one in the works.

Contributor

dajoo75 commented Apr 21, 2017

Maybe on sick leave? ;-). Anyway, please ignore my fix if there was already one in the works.

@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz

bradfitz Apr 21, 2017

Member

@ALTree, two problems with gopherbot. One was already known (not yet productionized properly) and the other is new with a fix+test coming.

Member

bradfitz commented Apr 21, 2017

@ALTree, two problems with gopherbot. One was already known (not yet productionized properly) and the other is new with a fix+test coming.

@gopherbot

This comment has been minimized.

Show comment
Hide comment
@gopherbot

gopherbot commented Apr 21, 2017

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

@gopherbot

This comment has been minimized.

Show comment
Hide comment
@gopherbot

gopherbot commented Apr 21, 2017

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

@gopherbot gopherbot closed this in 8db4d02 Apr 28, 2017

@golang golang locked and limited conversation to collaborators Apr 28, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.