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

runtime: js-wasm build broken #29632

Closed
bradfitz opened this issue Jan 9, 2019 · 5 comments
Closed

runtime: js-wasm build broken #29632

bradfitz opened this issue Jan 9, 2019 · 5 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone
Go1.12

Comments

@bradfitz
Copy link
Contributor

bradfitz commented Jan 9, 2019

https://go-review.googlesource.com/c/go/+/156657 broke the js-wasm build:

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

The TryBots passed on PS1 but post-cherry-pick it must've had conflicted with something else.

/cc @randall77 @aclements @cherrymui @neelance @tklauser
@bradfitz bradfitz added NeedsFix The path to resolution is known, but the work has not been done. release-blocker labels Jan 9, 2019
@bradfitz bradfitz added this to the Go1.12 milestone Jan 9, 2019
@cherrymui
Copy link
Member

The TryBots passed on PS1 but post-cherry-pick it must've had conflicted with something else.

I think I have seen this happened a few times on js-wasm. Maybe the js-wasm trybot doesn't run all the tests?

@bradfitz
Copy link
Contributor Author

bradfitz commented Jan 9, 2019

@cherrymui, indeed:

        addBuilder(BuildConfig{
                Name:     "js-wasm",
                HostType: "host-js-wasm",
                tryBot:   explicitTrySet("go"),
                ShouldRunDistTest: func(distTest string, isTry bool) bool {
                        if isTry {
                                if strings.HasPrefix(distTest, "test:") {
                                        return false
                                }
                                if strings.Contains(distTest, "/internal/") ||
                                        strings.Contains(distTest, "vendor/golang.org/x/arch") {
                                        return false
                                }
                                switch distTest {
                                case "cmd/go", "nolibgcc:crypto/x509":
                                        return false
                                }
                                return true
                        }
                        return true
                },

We could shard the js-wasm builder out wider and enable more tests. /cc @dmitshur

@cherrymui
Copy link
Member

On Wasm, the instruction after the call of sigpanic doesn't increment the PC. Subtracting 1 in Frame.Next makes it pointing to the previous PC, which is the function entry in this case.

"".f STEXT size=11 args=0x0 locals=0x80
	0x0000 00000 (/tmp/bug347.go:20)	TEXT	"".f(SB), ABIInternal, $128-0
	0x0000 00000 (/tmp/bug347.go:20)	Loop
	0x0000 00000 (/tmp/bug347.go:20)	Block
	0x0000 00000 (/tmp/bug347.go:20)	Block
	0x0000 00000 (/tmp/bug347.go:20)	Block
	0x0000 00000 (/tmp/bug347.go:20)	Block
	0x0000 00000 (/tmp/bug347.go:20)	Block
	0x0000 00000 (/tmp/bug347.go:20)	Block
	0x0000 00000 (/tmp/bug347.go:20)	Get	PC_B
	0x0000 00000 (/tmp/bug347.go:20)	BrTable
	0x0000 00000 (/tmp/bug347.go:20)	End
	0x0000 00000 (/tmp/bug347.go:20)	Get	SP
	0x0000 00000 (/tmp/bug347.go:20)	Get	g
	0x0000 00000 (/tmp/bug347.go:20)	I32WrapI64
	0x0000 00000 (/tmp/bug347.go:20)	I32Load	$16
	0x0000 00000 (/tmp/bug347.go:20)	I32LeU
	0x0000 00000 (/tmp/bug347.go:20)	If
	0x0000 00000 (/tmp/bug347.go:20)	Get	SP
	0x0000 00000 (/tmp/bug347.go:20)	I32Const	$8
	0x0000 00000 (/tmp/bug347.go:20)	I32Sub
	0x0000 00000 (/tmp/bug347.go:20)	Set	SP
	0x0000 00000 (/tmp/bug347.go:20)	Get	SP
	0x0000 00000 (/tmp/bug347.go:20)	I64Const	$"".f(SB)
	0x0000 00000 (/tmp/bug347.go:20)	I64Store	$0
	0x0000 00000 (/tmp/bug347.go:20)	I32Const	$0
	0x0000 00000 (/tmp/bug347.go:20)	Set	PC_B
	0x0000 00000 (/tmp/bug347.go:20)	Call	runtime.morestack_noctxt(SB)
	0x0000 00000 (/tmp/bug347.go:20)	If
	0x0000 00000 (/tmp/bug347.go:20)	I32Const	$1
	0x0000 00000 (/tmp/bug347.go:20)	Return
	0x0000 00000 (/tmp/bug347.go:20)	End
	0x0000 00000 (/tmp/bug347.go:20)	End
	0x0000 00000 (/tmp/bug347.go:20)	Get	SP
	0x0000 00000 (/tmp/bug347.go:20)	I32Const	$128
	0x0000 00000 (/tmp/bug347.go:20)	I32Sub
	0x0000 00000 (/tmp/bug347.go:20)	Set	SP
	0x0001 00001 (/tmp/bug347.go:20)	FUNCDATA	$0, gclocals·69c1753bd5f81501d95132d08af04464(SB)
	0x0001 00001 (/tmp/bug347.go:20)	FUNCDATA	$1, gclocals·0ce94cb13e33421890d23bb20b1573df(SB)
	0x0001 00001 (/tmp/bug347.go:20)	FUNCDATA	$3, gclocals·9fb7f0986f647f17cb53dda1484e0f7a(SB)
	0x0001 00001 (/tmp/bug347.go:20)	FUNCDATA	$4, "".f.stkobj(SB)
	0x0001 00001 (/tmp/bug347.go:22)	PCDATA	$2, $1
	0x0001 00001 (/tmp/bug347.go:22)	PCDATA	$0, $0
	0x0001 00001 (/tmp/bug347.go:22)	I64Const	$"".t(SB)
	0x0001 00001 (/tmp/bug347.go:22)	I32WrapI64
	0x0001 00001 (/tmp/bug347.go:22)	I64Load	$0
	0x0001 00001 (/tmp/bug347.go:22)	Set	R0
	0x0001 00001 (/tmp/bug347.go:22)	Get	R0
	0x0001 00001 (/tmp/bug347.go:22)	I64Eqz
	0x0001 00001 (/tmp/bug347.go:22)	If
	0x0001 00001 (/tmp/bug347.go:22)	Get	SP
	0x0001 00001 (/tmp/bug347.go:22)	I32Const	$8
	0x0001 00001 (/tmp/bug347.go:22)	I32Sub
	0x0001 00001 (/tmp/bug347.go:22)	Set	SP
	0x0001 00001 (/tmp/bug347.go:22)	Get	SP
	0x0001 00001 (/tmp/bug347.go:22)	I64Const	$"".f+1(SB)
	0x0001 00001 (/tmp/bug347.go:22)	I64Store	$0
	0x0001 00001 (/tmp/bug347.go:22)	I32Const	$0
	0x0001 00001 (/tmp/bug347.go:22)	Set	PC_B
	0x0001 00001 (/tmp/bug347.go:22)	Call	runtime.sigpanic(SB)
	0x0001 00001 (/tmp/bug347.go:22)	If
	0x0001 00001 (/tmp/bug347.go:22)	I32Const	$1
	0x0001 00001 (/tmp/bug347.go:22)	Return
	0x0001 00001 (/tmp/bug347.go:22)	End
	0x0002 00002 (/tmp/bug347.go:22)	End
	0x0002 00002 (/tmp/bug347.go:22)	Get	R0
	0x0002 00002 (/tmp/bug347.go:22)	I32WrapI64
	0x0002 00002 (/tmp/bug347.go:22)	I64Load	$0
	0x0002 00002 (/tmp/bug347.go:22)	Set	R0
	...

If I remember correctly, the stack trace of panic expects the PC of the panic is the call of sigpanic, so we don't advance the PC at that call. But runtime.Caller works differently...

Maybe we can advance the PC by 2 at sigpanic, then it would work for both cases?

@bradfitz bradfitz mentioned this issue Jan 9, 2019
Open
@gopherbot
Copy link

Change https://golang.org/cl/157157 mentions this issue: cmd/internal/obj/wasm: increment PC by 2 at sigpanic

@dmitshur
Copy link
Contributor

dmitshur commented Jan 9, 2019 •
edited

(Moved this comment to #29636 (comment).)

@golang golang locked and limited conversation to collaborators Jan 9, 2020
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.12
Development

No branches or pull requests
4 participants
@bradfitz @dmitshur @gopherbot @cherrymui