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

build: get Alpine builders passing #19938

Open
bradfitz opened this issue Apr 12, 2017 · 22 comments

Comments

@bradfitz
Copy link
Member

commented Apr 12, 2017

Thanks to @jessfraz's setting up an Alpine Linux builder (#17891), we now get to deal with more bugs, like this one:

https://build.golang.org/log/09a1850a53ff39d403984ecc8c29114ecd0a4109

--- FAIL: TestCgoPprofPIE (8.39s)
	crash_cgo_test.go:285: signal: segmentation fault (core dumped)
FAIL
FAIL	runtime	16.270s

@ianlancetaylor?

@jessfraz

This comment has been minimized.

Copy link
Contributor

commented Apr 13, 2017

I feel like I know why this is happening... I can debug

@jessfraz

This comment has been minimized.

Copy link
Contributor

commented Apr 13, 2017

So it's because https://github.com/golang/go/blob/master/src/cmd/link/internal/amd64/obj.go#L70

ld.Thearch.Linuxdynld = "/lib64/ld-linux-x86-64.so.2"

on alpine this is:

ld.Thearch.Linuxdynld = "/lib/ld-musl-x86_64.so.1"

we should maybe have a way of using a different one? but alpine is not a GOOS so unsure what you all would want here

This is #18243

@jessfraz

This comment has been minimized.

Copy link
Contributor

commented Apr 13, 2017

There are other failures following that failure as well. Details below.

EDIT: These are all #14481

TestTestEmpty
--- FAIL: TestTestEmpty (1.82s)
    --- FAIL: TestTestEmpty/pkg (1.82s)
        go_test.go:260: running testgo [test -cover -coverpkg=. -race]
        go_test.go:279: standard error:
        go_test.go:280: # runtime/race
                race_linux_amd64.syso: In function `__sanitizer::InternalAlloc(unsigned long, __sanitizer::SizeClassAllocatorLocalCache<__sanitizer::SizeClassAllocator32<0ul, 140737488355328ull, 0ul, __sanitizer::SizeClassMap<3ul, 4ul, 8ul, 17ul, 64ul, 14ul>, 20ul, __sanitizer::TwoLevelByteMap<32768ull, 4096ull, __sanitizer::NoOpMapUnmapCallback>, __sanitizer::NoOpMapUnmapCallback> >*, unsigned long)':
                gotsan.cc:(.text+0x16e1): undefined reference to `__libc_malloc'
                race_linux_amd64.syso: In function `__sanitizer::GetArgv()':
                gotsan.cc:(.text+0x4543): undefined reference to `__libc_stack_end'
                race_linux_amd64.syso: In function `__sanitizer::InternalRealloc(void*, unsigned long, __sanitizer::SizeClassAllocatorLocalCache<__sanitizer::SizeClassAllocator32<0ul, 140737488355328ull, 0ul, __sanitizer::SizeClassMap<3ul, 4ul, 8ul, 17ul, 64ul, 14ul>, 20ul, __sanitizer::TwoLevelByteMap<32768ull, 4096ull, __sanitizer::NoOpMapUnmapCallback>, __sanitizer::NoOpMapUnmapCallback> >*)':
                gotsan.cc:(.text+0x7dd0): undefined reference to `__libc_realloc'
                race_linux_amd64.syso: In function `__sanitizer::ReExec()':
                gotsan.cc:(.text+0xed47): undefined reference to `__libc_stack_end'
                race_linux_amd64.syso: In function `__sanitizer::InternalFree(void*, __sanitizer::SizeClassAllocatorLocalCache<__sanitizer::SizeClassAllocator32<0ul, 140737488355328ull, 0ul, __sanitizer::SizeClassMap<3ul, 4ul, 8ul, 17ul, 64ul, 14ul>, 20ul, __sanitizer::TwoLevelByteMap<32768ull, 4096ull, __sanitizer::NoOpMapUnmapCallback>, __sanitizer::NoOpMapUnmapCallback> >*)':
                gotsan.cc:(.text+0x7438): undefined reference to `__libc_free'
                collect2: error: ld returned 1 exit status
    
        go_test.go:289: go [test -cover -coverpkg=. -race] failed unexpectedly: exit status 2
TestTestRaceInstall
--- FAIL: TestTestRaceInstall (1.99s)
        go_test.go:260: running testgo [install -race -pkgdir=/tmp/gotest207241282/pkg std]
        go_test.go:279: standard error:
        go_test.go:280: # runtime/race
                race_linux_amd64.syso: In function `__sanitizer::InternalAlloc(unsigned long, __sanitizer::SizeClassAllocatorLocalCache<__sanitizer::SizeClassAllocator32<0ul, 140737488355328ull, 0ul, __sanitizer::SizeClassMap<3ul, 4ul, 8ul, 17ul, 64ul, 14ul>, 20ul, __sanitizer::TwoLevelByteMap<32768ull, 4096ull, __sanitizer::NoOpMapUnmapCallback>, __sanitizer::NoOpMapUnmapCallback> >*, unsigned long)':
                gotsan.cc:(.text+0x16e1): undefined reference to `__libc_malloc'
                race_linux_amd64.syso: In function `__sanitizer::GetArgv()':
                gotsan.cc:(.text+0x4543): undefined reference to `__libc_stack_end'
                race_linux_amd64.syso: In function `__sanitizer::InternalRealloc(void*, unsigned long, __sanitizer::SizeClassAllocatorLocalCache<__sanitizer::SizeClassAllocator32<0ul, 140737488355328ull, 0ul, __sanitizer::SizeClassMap<3ul, 4ul, 8ul, 17ul, 64ul, 14ul>, 20ul, __sanitizer::TwoLevelByteMap<32768ull, 4096ull, __sanitizer::NoOpMapUnmapCallback>, __sanitizer::NoOpMapUnmapCallback> >*)':
                gotsan.cc:(.text+0x7dd0): undefined reference to `__libc_realloc'
                race_linux_amd64.syso: In function `__sanitizer::ReExec()':
                gotsan.cc:(.text+0xed47): undefined reference to `__libc_stack_end'
                race_linux_amd64.syso: In function `__sanitizer::InternalFree(void*, __sanitizer::SizeClassAllocatorLocalCache<__sanitizer::SizeClassAllocator32<0ul, 140737488355328ull, 0ul, __sanitizer::SizeClassMap<3ul, 4ul, 8ul, 17ul, 64ul, 14ul>, 20ul, __sanitizer::TwoLevelByteMap<32768ull, 4096ull, __sanitizer::NoOpMapUnmapCallback>, __sanitizer::NoOpMapUnmapCallback> >*)':
                gotsan.cc:(.text+0x7438): undefined reference to `__libc_free'
                collect2: error: ld returned 1 exit status

        go_test.go:289: go [install -race -pkgdir=/tmp/gotest207241282/pkg std] failed unexpectedly: exit status 2
TestGoTestRaceInstallCgo
--- FAIL: TestGoTestRaceInstallCgo (1.90s)
        go_test.go:260: running testgo [tool -n cgo]
        go_test.go:275: standard output:
        go_test.go:276: /usr/src/go/pkg/tool/linux_amd64/cgo

        go_test.go:260: running testgo [test -race -i runtime/race]
        go_test.go:279: standard error:
        go_test.go:280: # runtime/race
                race_linux_amd64.syso: In function `__sanitizer::InternalAlloc(unsigned long, __sanitizer::SizeClassAllocatorLocalCache<__sanitizer::SizeClassAllocator32<0ul, 140737488355328ull, 0ul, __sanitizer::SizeClassMap<3ul, 4ul, 8ul, 17ul, 64ul, 14ul>, 20ul, __sanitizer::TwoLevelByteMap<32768ull, 4096ull, __sanitizer::NoOpMapUnmapCallback>, __sanitizer::NoOpMapUnmapCallback> >*, unsigned long)':
                gotsan.cc:(.text+0x16e1): undefined reference to `__libc_malloc'
                race_linux_amd64.syso: In function `__sanitizer::GetArgv()':
                gotsan.cc:(.text+0x4543): undefined reference to `__libc_stack_end'
                race_linux_amd64.syso: In function `__sanitizer::InternalRealloc(void*, unsigned long, __sanitizer::SizeClassAllocatorLocalCache<__sanitizer::SizeClassAllocator32<0ul, 140737488355328ull, 0ul, __sanitizer::SizeClassMap<3ul, 4ul, 8ul, 17ul, 64ul, 14ul>, 20ul, __sanitizer::TwoLevelByteMap<32768ull, 4096ull, __sanitizer::NoOpMapUnmapCallback>, __sanitizer::NoOpMapUnmapCallback> >*)':
                gotsan.cc:(.text+0x7dd0): undefined reference to `__libc_realloc'
                race_linux_amd64.syso: In function `__sanitizer::ReExec()':
                gotsan.cc:(.text+0xed47): undefined reference to `__libc_stack_end'
                race_linux_amd64.syso: In function `__sanitizer::InternalFree(void*, __sanitizer::SizeClassAllocatorLocalCache<__sanitizer::SizeClassAllocator32<0ul, 140737488355328ull, 0ul, __sanitizer::SizeClassMap<3ul, 4ul, 8ul, 17ul, 64ul, 14ul>, 20ul, __sanitizer::TwoLevelByteMap<32768ull, 4096ull, __sanitizer::NoOpMapUnmapCallback>, __sanitizer::NoOpMapUnmapCallback> >*)':
                gotsan.cc:(.text+0x7438): undefined reference to `__libc_free'
                collect2: error: ld returned 1 exit status

        go_test.go:289: go [test -race -i runtime/race] failed unexpectedly: exit status 2
TestGoTestRaceFailures
--- FAIL: TestGoTestRaceFailures (0.69s)
        go_test.go:260: running testgo [test testrace]
        go_test.go:275: standard output:
        go_test.go:276: ok      testrace        0.002s

        go_test.go:260: running testgo [test -race testrace]
        go_test.go:275: standard output:
        go_test.go:276: FAIL    testrace [build failed]

        go_test.go:279: standard error:
        go_test.go:280: # runtime/race
                race_linux_amd64.syso: In function `__sanitizer::InternalAlloc(unsigned long, __sanitizer::SizeClassAllocatorLocalCache<__sanitizer::SizeClassAllocator32<0ul, 140737488355328ull, 0ul, __sanitizer::SizeClassMap<3ul, 4ul, 8ul, 17ul, 64ul, 14ul>, 20ul, __sanitizer::TwoLevelByteMap<32768ull, 4096ull, __sanitizer::NoOpMapUnmapCallback>, __sanitizer::NoOpMapUnmapCallback> >*, unsigned long)':
                gotsan.cc:(.text+0x16e1): undefined reference to `__libc_malloc'
                race_linux_amd64.syso: In function `__sanitizer::GetArgv()':
                gotsan.cc:(.text+0x4543): undefined reference to `__libc_stack_end'
                race_linux_amd64.syso: In function `__sanitizer::InternalRealloc(void*, unsigned long, __sanitizer::SizeClassAllocatorLocalCache<__sanitizer::SizeClassAllocator32<0ul, 140737488355328ull, 0ul, __sanitizer::SizeClassMap<3ul, 4ul, 8ul, 17ul, 64ul, 14ul>, 20ul, __sanitizer::TwoLevelByteMap<32768ull, 4096ull, __sanitizer::NoOpMapUnmapCallback>, __sanitizer::NoOpMapUnmapCallback> >*)':
                gotsan.cc:(.text+0x7dd0): undefined reference to `__libc_realloc'
                race_linux_amd64.syso: In function `__sanitizer::ReExec()':
                gotsan.cc:(.text+0xed47): undefined reference to `__libc_stack_end'
                race_linux_amd64.syso: In function `__sanitizer::InternalFree(void*, __sanitizer::SizeClassAllocatorLocalCache<__sanitizer::SizeClassAllocator32<0ul, 140737488355328ull, 0ul, __sanitizer::SizeClassMap<3ul, 4ul, 8ul, 17ul, 64ul, 14ul>, 20ul, __sanitizer::TwoLevelByteMap<32768ull, 4096ull, __sanitizer::NoOpMapUnmapCallback>, __sanitizer::NoOpMapUnmapCallback> >*)':
                gotsan.cc:(.text+0x7438): undefined reference to `__libc_free'
                collect2: error: ld returned 1 exit status

        go_test.go:299: testgo failed as expected: exit status 2
        go_test.go:365: TestRace did not fail
        go_test.go:366: pattern FAIL: TestRace not found in standard output
FAIL
@jessfraz

This comment has been minimized.

Copy link
Contributor

commented Apr 13, 2017

Lastly there is:

--- FAIL: TestGroupCleanup (0.00s)
        exec_linux_test.go:214: id command output: "uid=0(root) gid=0(root)", expected prefix: "uid=0(root) gid=0(root) groups=0(root)"
--- FAIL: TestGroupCleanupUserNamespace (0.00s)
        exec_linux_test.go:256: id command output: "uid=0(root) gid=0(root) groups=0(root),65534(nobody),65534(nobody),65534(nobody),65534(nobody),65534(nobody),65534(nobody),65534(nobody),65534(nobody),65534(nobody),65534(nobody)", expected one of ["uid=0(root) gid=0(root) groups=0(root)" "uid=0(root) gid=0(root) groups=0(root),65534(nobody)" "uid=0(root) gid=0(root) groups=0(root),65534(nogroup)" "uid=0(root) gid=0(root) groups=0(root),65534"]
FAIL

which I can open a CL to fix

@gopherbot

This comment has been minimized.

Copy link

commented Apr 13, 2017

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

gopherbot pushed a commit that referenced this issue Apr 13, 2017
syscall: fix TestGroupCleanup{UserNamespace} on Alpine
This updates TestGroupCleanup and TestGroupCleanupUserNamespace to pass in the
Alpine builder.

Updates #19938

Change-Id: Iacbfd73782eccd57f872f9e85726c6024529c277
Reviewed-on: https://go-review.googlesource.com/40692
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@bradfitz

This comment has been minimized.

Copy link
Member Author

commented Apr 25, 2017

Should we run the Alpine builder with CGO_ENABLED=0 perhaps?

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Apr 25, 2017

@bradfitz That seems reasonable until we have a fix for #18243.

@gopherbot

This comment has been minimized.

Copy link

commented Apr 25, 2017

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

gopherbot pushed a commit that referenced this issue Apr 25, 2017
runtime: ignore TestCgoPprofPIE test failures on Alpine
Updates #19938
Updates #18243

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

This comment has been minimized.

Copy link

commented Apr 25, 2017

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

gopherbot pushed a commit that referenced this issue Apr 25, 2017
runtime: ignore TestCgoPprofPIE test failures on Alpine (take 2)
s/arm64/amd64/ in previous typo CL 41628

Updates #19938
Updates #18243

Change-Id: I282244ee3c94535f229a87b6246382385ff64428
Reviewed-on: https://go-review.googlesource.com/41675
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

@bradfitz bradfitz changed the title runtime: TestCgoPprofPIE fails on Alpine build: get Alpine builders passing Apr 26, 2017

@bradfitz bradfitz self-assigned this Apr 26, 2017

@bradfitz

This comment has been minimized.

Copy link
Member Author

commented Apr 26, 2017

I'm slightly repurposing this bug.

There are existing bugs tracking fixing actual Alpine problems, such as:

#18243 -- internal linking broken
#14481 -- race doesn't work

This bug is only about getting the builders in "ok" state, even if that means temporarily disabling some tests.

To that end, I just tried setting GO_EXTLINK_ENABLED=1 on the Alpine builder in https://go-review.googlesource.com/c/41798/ but that seems to have caused a new error:

https://build.golang.org/log/a3288076e1057e053548837886fb064f2cf5d921

--- FAIL: TestSimpleDeadlock (60.10s)
	crash_test.go:106: testprog SimpleDeadlock exit status: exit status 2
	crash_test.go:205: output does not start with "fatal error: all goroutines are asleep - deadlock!\n":
		SIGQUIT: quit
		PC=0x454a31 m=0 sigcode=0
		
		goroutine 0 [idle]:
		runtime.futex(0x787990, 0x0, 0x0, 0x0, 0x0, 0x7fb672d977c0, 0x0, 0x0, 0x7ffeae1c2008, 0x40e211, ...)
			/tmp/workdir/go/src/runtime/sys_linux_amd64.s:422 +0x21
		runtime.futexsleep(0x787990, 0x0, 0xffffffffffffffff)
			/tmp/workdir/go/src/runtime/os_linux.go:45 +0x62
		runtime.notesleep(0x787990)
			/tmp/workdir/go/src/runtime/lock_futex.go:150 +0x81
		runtime.stopm()
			/tmp/workdir/go/src/runtime/proc.go:1637 +0xe5
		runtime.findrunnable(0xc42001c000, 0x0)
			/tmp/workdir/go/src/runtime/proc.go:2092 +0x5c7
		runtime.schedule()
			/tmp/workdir/go/src/runtime/proc.go:2212 +0x134
		runtime.park_m(0xc420000180)
			/tmp/workdir/go/src/runtime/proc.go:2275 +0xb6
		runtime.mcall(0x7ffeae1c21a0)
			/tmp/workdir/go/src/runtime/asm_amd64.s:276 +0x5b
		
		goroutine 1 [select (no cases)]:
		main.SimpleDeadlock()
			/tmp/workdir/go/src/runtime/testdata/testprog/deadlock.go:39 +0x20
		main.main()
			/tmp/workdir/go/src/runtime/testdata/testprog/main.go:34 +0x1da
		
		goroutine 17 [syscall, locked to thread]:
		runtime.goexit()
			/tmp/workdir/go/src/runtime/asm_amd64.s:2349 +0x1
		
		rax    0xca
		rbx    0x787880
		rcx    0x454a33
		rdx    0x0
		rdi    0x787990
		rsi    0x0
		rbp    0x7ffeae1c1fd0
		rsp    0x7ffeae1c1f88
		r8     0x0
		r9     0x0
		r10    0x0
		r11    0x286
		r12    0xc420096fa3
		r13    0x8
		r14    0xffffffffffffffff
		r15    0x8
		rip    0x454a31
		rflags 0x286
		cs     0x33
		fs     0x0
		gs     0x0
--- FAIL: TestInitDeadlock (60.00s)
...
(same)
...
panic: test timed out after 3m0s

I guess the external linker disables deadlock detection?

@gopherbot

This comment has been minimized.

Copy link

commented Apr 26, 2017

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

@bradfitz

This comment has been minimized.

Copy link
Member Author

commented Apr 26, 2017

I cleared the four builds from the dashboard built with GO_EXTLINK_ENABLED=1.

They're now all back to failing with:

https://build.golang.org/log/4d41ef62d744100b3271757b084f95166abddbb5

##### ../misc/cgo/test
unexpected fault address 0x7fb6ae1a9b90
fatal error: fault
[signal SIGSEGV: segmentation violation code=0x2 addr=0x7fb6ae1a9b90 pc=0x7fb6ae1a9b90]

goroutine 19 [running]:
runtime.throw(0x5dc6d5, 0x5)
        /tmp/workdir/go/src/runtime/panic.go:596 +0x95 fp=0xc420046768 sp=0xc420046748
runtime.sigpanic()
        /tmp/workdir/go/src/runtime/signal_unix.go:355 +0x282 fp=0xc4200467b8 sp=0xc420046768
created by _/tmp/workdir/go/misc/cgo/test.runTestSetgid
        /tmp/workdir/go/misc/cgo/test/setgid_linux.go:26 +0x6f

From local changes with go test -v, I see that crash is from === RUN TestSetgid

@bradfitz

This comment has been minimized.

Copy link
Member Author

commented Apr 26, 2017

And if I skip TestSetgid, it just moves to a few tests later:

##### ../misc/cgo/test
=== RUN   TestSetgid
--- SKIP: TestSetgid (0.00s)
        setgid_linux.go:40: temp skip
=== RUN   Test6997
--- PASS: Test6997 (0.50s)
=== RUN   TestBuildID
--- PASS: TestBuildID (0.00s)
=== RUN   Test9400
unexpected fault address 0x7f38ab094af8
fatal error: fault
[signal SIGSEGV: segmentation violation code=0x2 addr=0x7f38ab094af8 pc=0x7f38ab094af8]
 
goroutine 9 [running, locked to thread]:
runtime.throw(0x5dc739, 0x5)
        /tmp/workdir/go/src/runtime/panic.go:596 +0x95 fp=0xc420046768 sp=0xc420046748
runtime.sigpanic()
        /tmp/workdir/go/src/runtime/signal_unix.go:355 +0x282 fp=0xc4200467b8 sp=0xc420046768
created by _/tmp/workdir/go/misc/cgo/test.test9400
        /tmp/workdir/go/misc/cgo/test/issue9400_linux.go:30 +0xa2
gopherbot pushed a commit to golang/build that referenced this issue Apr 26, 2017
dashboard: revert Alpine GO_EXTLINK_ENABLED=1 change
See golang/go#19938 (comment)

Made things worse.

Updates golang/go#19938

Change-Id: I6c3e798c0b61ea5ee2c43af0aea5ee56e0f52032
Reviewed-on: https://go-review.googlesource.com/41812
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@bradfitz

This comment has been minimized.

Copy link
Member Author

commented Apr 30, 2017

/cc @mwhudson

@mwhudson

This comment has been minimized.

Copy link
Contributor

commented Apr 30, 2017

How can I reproduce this? (no promises, but I might be able to have a quick poke at it)

@bradfitz

This comment has been minimized.

Copy link
Member Author

commented Apr 30, 2017

gomote if you have access (email me if not) or run all.bash in an Alpine Docker container: https://github.com/golang/build/blob/master/env/linux-x86-alpine/Dockerfile

@chlunde

This comment has been minimized.

Copy link
Contributor

commented May 21, 2017

Regarding the TestSetgid and Test9400 crashes, here's my understanding of the issue:

Alt. A: Increasing stack guard area

As a temporary workaround, it is possible to set/increase the guard area by setting _StackSystem = 2048 in src/runtime/stack.go, or maybe even higher.

--- a/src/runtime/sigtab_linux_generic.go
+++ b/src/runtime/sigtab_linux_generic.go
@@ -50,7 +50,7 @@ var sigtable = [...]sigTabT{
        /* 31 */ {_SigThrow, "SIGSYS: bad system call"},
        /* 32 */ {_SigSetStack + _SigUnblock, "signal 32"}, /* SIGCANCEL; see issue 6997 */
        /* 33 */ {_SigSetStack + _SigUnblock, "signal 33"}, /* SIGSETXID; see issues 3871, 9400, 12498 */
-       /* 34 */ {_SigNotify, "signal 34"},
+       /* 34 */ {_SigSetStack + _SigUnblock, "signal 34"}, /* SIGSYNCCALL; see issue 19938 */
diff --git a/src/runtime/stack.go b/src/runtime/stack.go
index 562427a..4eb3368 100644
--- a/src/runtime/stack.go
+++ b/src/runtime/stack.go
@@ -65,7 +65,8 @@ const (
        // to each stack below the usual guard area for OS-specific
        // purposes like signal handling. Used on Windows, Plan 9,
        // and Darwin/ARM because they do not use a separate stack.
-       _StackSystem = sys.GoosWindows*512*sys.PtrSize + sys.GoosPlan9*512 + sys.GoosDarwin*sys.GoarchArm*1024
+       _StackSystem = sys.GoosWindows*512*sys.PtrSize + sys.GoosPlan9*512 + sys.GoosDarwin*sys.GoarchArm*1024 + 2048 // TODO: conditional on cgo + musl?

I assume this is not something we want, even if it could be made conditional on cgo+musl. It does seem to work, but test9400 would also need an update:

	issue9400_linux.go:55: entry 756 of test pattern is wrong; 0x7ff92884d045 != 0x123456789abcdef

Alt. B: Modifying musl

If I modify musl to

./src/thread/synccall.c:	struct sigaction sa = { .sa_flags = SA_ONSTACK|SA_RESTART, .sa_handler = handler };

this test case does not crash.

There are probably more handlers in musl which needs a look, for example:

  • src/time/timer_create.c
  • src/thread/pthread_cancel.c
    and perhaps
  • src/process/posix_spawn.c

Do we think it would be reasonable for musl to use SA_ONSTACK|SA_RESTART by default, or might that break other programs? Are there any other alternatives that will not break other programs? One idea I have is that we might be able to set SA_ONSTACK|SA_RESTART on the relevant handlers with SIG_DFL, and then request that musl changes behaviour and check for that property on these signals, and reuse the same flags on its own handler. I believe musl clobbers the handler anyway, so it should not break any existing behaviour.

PS! To get this error message to print: fatal error: unexpected signal during runtime execution, I had to make some changes, but I do not know if they are safe.

diff --git a/src/runtime/signal_unix.go b/src/runtime/signal_unix.go
index e0ea724..c20528e 100644
--- a/src/runtime/signal_unix.go
+++ b/src/runtime/signal_unix.go
@@ -326,10 +326,13 @@ func sigtrampgo(sig uint32, info *siginfo, ctx unsafe.Pointer) {
 // the signal handler. The effect is that the program will act as
 // though the function that got the signal simply called sigpanic
 // instead.
+//go:nosplit
 func sigpanic() {
        g := getg()
        if !canpanic(g) {
-               throw("unexpected signal during runtime execution")
+               systemstack(func() {
+                       throw("unexpected signal during runtime execution")
+               })
        }
 
        switch g.sig {
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented May 23, 2017

The use of SA_RESTART is not too important (but see #20400) but every signal handler in a Go program must use SA_ONSTACK. Since SA_ONSTACK does nothing if the thread does not call sigaltstack, and since a thread that calls sigaltstack presumably wants to actually use it, I think it would be reasonable for musl to use SA_ONSTACK when it temporarily installs a signal handler. Of course, I am not a musl maintainer.

@richfelker

This comment has been minimized.

Copy link

commented Jun 1, 2017

I'm not entirely opposed to using SA_ONSTACK for the implementation-internal signals, but also cautious, as it means the signal handlers may end up running on an application-provided stack rather than the main one. The signal handlers in musl themselves use very little stack space, so the culprit causing the crash is almost certainly the kernel saving sigcontext, which can be very large on some archs. Could you post to the musl mailing list explaining the issue so we can discuss it somewhere the musl community sees and has an opportunity to give feedback if there are concerns about SA_ONSTACK?

@bradfitz bradfitz modified the milestones: Go1.10, Go1.9Maybe Jun 28, 2017

@gopherbot

This comment has been minimized.

Copy link

commented Jul 19, 2017

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

@bradfitz bradfitz modified the milestones: Go1.10, Unplanned Nov 15, 2017

@gopherbot

This comment has been minimized.

Copy link

commented Nov 18, 2017

Change https://golang.org/cl/78575 mentions this issue: dashboard: remove alpine builder

gopherbot pushed a commit to golang/build that referenced this issue Nov 18, 2017
dashboard: remove alpine builder
It's doing no good being red all the time, especially as nobody is
working on it.

Updates golang/go#22689
Updates golang/go#19938

Change-Id: Icb947878d85e920f24ea458eb0f319844ca5bd60
Reviewed-on: https://go-review.googlesource.com/78575
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
tklauser added a commit to tklauser/go that referenced this issue Dec 7, 2018
cmd/dist, cmd/link: allow passing default dynamic linker/loader
Add an environment variable to make.bash to allow setting the default
dynamic linker/loader. This fixes alpine builds to use
/lib/ld-musl-x86_64.so.1:

    $ ldd ../bin/go
        /lib/ld-musl-x86_64.so.1 (0x5608dfadd000)
        libc.musl-x86_64.so.1 => /lib/ld-musl-x86_64.so.1 (0x5608dfadd000)

Also re-enable the internal linker tests that were previously disabled
for alpine (CL 41759, CL 41678).

Fixes golang#18243
Updates golang#19938

This resurrects CL 50070 which was authored by Jessie Frazelle.

Co-authored-by: Jessie Frazelle <me@jessfraz.com>

Change-Id: I132b5282045a3d60c8568e3b002a7f075eac2d93
tklauser added a commit to tklauser/go that referenced this issue Dec 20, 2018
cmd/dist, cmd/link: allow passing default dynamic linker/loader
Add an environment variable to make.bash to allow setting the default
dynamic linker/loader. This fixes alpine builds to use
/lib/ld-musl-x86_64.so.1:

  $ readelf -l ../bin/go | grep 'interpreter:' | sed -e 's/^.*interpreter: \(.*\)[]]/\1/'
  /lib/ld-musl-x86_64.so.1

Also re-enable the internal linker tests that were previously disabled
for alpine (CL 41759, CL 41678).

Fixes golang#18243
Updates golang#19938

This resurrects CL 50070 which was authored by Jessie Frazelle.

Change-Id: I132b5282045a3d60c8568e3b002a7f075eac2d93
@gopherbot

This comment has been minimized.

Copy link

commented Feb 27, 2019

Change https://golang.org/cl/163977 mentions this issue: cmd/dist, cmd/link: allow passing default dynamic linker/loader

gopherbot pushed a commit that referenced this issue Mar 1, 2019
cmd/dist, cmd/link: allow passing default dynamic linker/loader
Add an environment variable to make.bash to allow setting the default
dynamic linker/loader. This fixes alpine builds to use
/lib/ld-musl-x86_64.so.1:

  $ readelf -l ../bin/go | grep 'interpreter:' | sed -e 's/^.*interpreter: \(.*\)[]]/\1/'
  /lib/ld-musl-x86_64.so.1

Also re-enable the internal linker tests that were previously disabled
for alpine (CL 41759, CL 41678).

Fixes #18243
Updates #19938

This resurrects CL 50070 authored by Jessie Frazelle.

Change-Id: I132b5282045a3d60c8568e3b002a7f075eac2d93
Reviewed-on: https://go-review.googlesource.com/c/163977
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.