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.

Sign up for GitHub

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/link/internal/ld: TestAbstractOriginSanity consistently failing on windows-amd64-longtest builder #45658

Closed
bcmills opened this issue Apr 21, 2021 · 10 comments
Closed

cmd/link/internal/ld: TestAbstractOriginSanity consistently failing on windows-amd64-longtest builder #45658

bcmills opened this issue Apr 21, 2021 · 10 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows release-blocker Soon This needs action soon. (recent regressions, service outages, unusual time-sensitive situations)
Milestone
Go1.17

Comments

@bcmills
Copy link
Contributor

bcmills commented Apr 21, 2021

2021-04-21T02:39:25-c33ced6/windows-amd64-longtest
2021-04-21T01:45:15-190cb93/windows-amd64-longtest
2021-04-21T00:54:01-c187443/windows-amd64-longtest
2021-04-21T00:53:48-faa4fa1/windows-amd64-longtest
2021-04-20T23:47:34-1c26843/windows-amd64-longtest
2021-04-20T23:41:54-e12b0af/windows-amd64-longtest
2021-04-20T22:47:50-48e3d92/windows-amd64-longtest
2021-04-20T18:34:56-77860ad/windows-amd64-longtest

--- FAIL: TestAbstractOriginSanity (15.02s)
    dwarf_test.go:124: build: # cmd/link/internal/ld/testdata/httptest/main
        runtime.abort: nosplit stack overflow
        	792	assumed on entry to runtime.callbackasm<1> (nosplit)
        	784	on entry to runtime.callbackasm1<0> (nosplit)
        	552	after runtime.callbackasm1<0> (nosplit) uses 232
        	544	on entry to runtime.cgocallback<0> (nosplit)
        	512	after runtime.cgocallback<0> (nosplit) uses 32
        	504	on entry to runtime.cgocallbackg<0> (nosplit)
        	472	after runtime.cgocallbackg<0> (nosplit) uses 32
        	464	on entry to runtime.cgocallbackg<1> (nosplit)
        	328	after runtime.cgocallbackg<1> (nosplit) uses 136
        	320	on entry to runtime.exitsyscall<1> (nosplit)
        	256	after runtime.exitsyscall<1> (nosplit) uses 64
        	248	on entry to runtime.casgstatus<1> (nosplit)
        	152	after runtime.casgstatus<1> (nosplit) uses 96
        	144	on entry to runtime.nanotime1<0> (nosplit)
        	136	on entry to runtime.nanotimeQPC<1> (nosplit)
        	96	after runtime.nanotimeQPC<1> (nosplit) uses 40
        	88	on entry to runtime.stdcall1<0> (nosplit)
        	72	after runtime.stdcall1<0> (nosplit) uses 16
        	64	on entry to runtime.stdcall<1> (nosplit)
        	16	after runtime.stdcall<1> (nosplit) uses 48
        	8	on entry to runtime.asmcgocall<0> (nosplit)
        	0	on entry to gosave_systemstack_switch<53> (nosplit)
        	-8	on entry to runtime.abort<0> (nosplit)
        
    dwarf_test.go:125: build error: exit status 2
FAIL
@bcmills bcmills added OS-Windows NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker Soon This needs action soon. (recent regressions, service outages, unusual time-sensitive situations) labels Apr 21, 2021
@bcmills bcmills added this to the Go1.17 milestone Apr 21, 2021
@bcmills
Copy link
Contributor Author

bcmills commented Apr 21, 2021

The first failure in the logs was at CL 311830 (CC @cherrymui, @dr2chase, @thanm).

@bcmills bcmills mentioned this issue Apr 21, 2021
Closed
@thanm
Copy link
Contributor

thanm commented Apr 21, 2021

@mknyszek

@thanm
Copy link
Contributor

thanm commented Apr 21, 2021

This is confusing -- I see

    	544	on entry to runtime.cgocallback<0> (nosplit)
    	512	after runtime.cgocallback<0> (nosplit) uses 32
    	504	on entry to runtime.cgocallbackg<0> (nosplit)
    	472	after runtime.cgocallbackg<0> (nosplit) uses 32

which makes it look as though runtime.cgocallbackg<0> is calling itself? Maybe I am reading this incorrectly.

@mknyszek
Copy link
Contributor

@thanm I think that's cgocallback (asm) calling cgocallbackg (Go).

Sigh. I think we need to find more places to save on nosplit stack. I already did a bit of a hack in nanotime on Windows to avoid a wrapper.

@cherrymui
Copy link
Member

I reapplied the disabled CL. It is no longer failing.

@bcmills
Copy link
Contributor Author

bcmills commented Apr 22, 2021

This same test is once again persistently failing as of CL 312429 (or perhaps CL 312191, but I think the former is much more likely).

@bcmills bcmills reopened this Apr 22, 2021
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/312650 mentions this issue: runtime: make nanotime1 ABIInternal on Windows

@mknyszek
Copy link
Contributor

mknyszek commented Apr 22, 2021 •
edited
Loading

FWIW I chose to fix forward here rather than roll-back, because the old code was pretty broken. I don't mind rolling back, though, if this takes a while to land.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/312669 mentions this issue: runtime: call cgocallbackg indirectly

gopherbot pushed a commit that referenced this issue Apr 22, 2021
@cherrymui
runtime: call cgocallbackg indirectly
d3853fb 
cgocallback calls cgocallbackg after switching the stack. Call it
indirectly to bypass the linker's nosplit check.

Apparently (at least on Windows) cgocallbackg can use quite a bit
stack space in a nosplit chain. We have been running over the
nosplit limit, or very close to the limit. Since it switches
stack in cgocallback, it is not meaningful to count frames above
cgocallback and below cgocallbackg together. Bypass the check.

For #45658.

Change-Id: Ie22017e3f82d2c1fcc37336696f2d02757856399
Reviewed-on: https://go-review.googlesource.com/c/go/+/312669
Trust: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
@mknyszek
Copy link
Contributor

https://golang.org/cl/312669 fixed the issue (longtest trybot passed). Thanks Cherry!

@cherrymui cherrymui mentioned this issue Feb 22, 2022
Open
@golang golang locked and limited conversation to collaborators Apr 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows release-blocker Soon This needs action soon. (recent regressions, service outages, unusual time-sensitive situations)
Projects
None yet
Milestone
Go1.17
Development

No branches or pull requests
5 participants
@mknyszek @bcmills @gopherbot @thanm @cherrymui