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

runtime: js-wasm build broken #29632

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

Comments

Projects
None yet
4 participants
@bradfitz
Copy link
Member

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 this to the Go1.12 milestone Jan 9, 2019

@cherrymui

This comment has been minimized.

Copy link
Contributor

commented Jan 9, 2019

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

This comment has been minimized.

Copy link
Member Author

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

This comment has been minimized.

Copy link
Contributor

commented Jan 9, 2019

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?

@gopherbot

This comment has been minimized.

Copy link

commented Jan 9, 2019

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

@dmitshur

This comment has been minimized.

Copy link
Member

commented Jan 9, 2019

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

@gopherbot gopherbot closed this in 52cae27 Jan 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.