cmd/compile: nil pointer dereference in walk

[dsnet]

Using ffa2bd2. The offending commit is probably something recent (last 2 weeks).

I'm seeing this compiler failure building a (proprietary) test:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x5b43b2]

goroutine 1 [running]:
panic(0xa2e120, 0xc42000e0a0)
    src/runtime/panic.go:502 +0x1b2
cmd/compile/internal/gc.(*Mpint).Cmp(0x0, 0xc42007b1d0, 0xc424c325a0)
    src/cmd/compile/internal/gc/mpint.go:252 +0x22
cmd/compile/internal/gc.walkexpr(0xc424c32630, 0xc44340f498, 0xc424c32900)
    src/cmd/compile/internal/gc/walk.go:1521 +0xa23c
cmd/compile/internal/gc.walkexpr(0xc424c327e0, 0xc44340f498, 0x0)
    src/cmd/compile/internal/gc/walk.go:738 +0x1261
cmd/compile/internal/gc.walkstmt(0xc424c327e0, 0xc424c32a20)
    src/cmd/compile/internal/gc/walk.go:192 +0xe59
cmd/compile/internal/gc.walkstmtlist(0xc439669900, 0x11, 0x20)
    src/cmd/compile/internal/gc/walk.go:80 +0x4d
cmd/compile/internal/gc.walk(0xc424c439e0)
    src/cmd/compile/internal/gc/walk.go:65 +0x1e4
cmd/compile/internal/gc.compile(0xc424c439e0)
    src/cmd/compile/internal/gc/pgen.go:391 +0x1d4
cmd/compile/internal/gc.funccompile(0xc424c439e0)
    src/cmd/compile/internal/gc/dcl.go:1258 +0x186
cmd/compile/internal/gc.Main()
    src/cmd/compile/internal/gc/main.go:484 +0x2065
cmd/compile/internal/amd64.Main()
    src/cmd/compile/internal/amd64/galign.go:93 +0x2fa
main.main()
    src/cmd/compile/main.go:33 +0x2a3

\cc @randall77 @mdempsky @ianlancetaylor

I'm still trying to strip the offending code into a smaller reproduction. Any advice on how to isolate the problem is welcome.

[bradfitz]

Which test? (which source code reproduces this?)

/cc @josharian @davecheney

[randall77]

Probably this: e6f9f39

@martisch

[dsnet]

I confirm that the offending commit is e6f9f39.

[davecheney]

Thanks for confirming. @dsnet, would you please revert that commit.

On Thu, Sep 1, 2016 at 9:11 AM, Joe Tsai notifications@github.com wrote:

I confirm that the offending commit is e6f9f39
e6f9f39
.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#16940 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAAcA1R9gUyJeOkqU3MTirlmtngONXQlks5qlgoegaJpZM4JyIc4
.

[josharian]

How urgent is this, @dsnet? Can we try rolling forward?

[josharian]

AFK but my guess from my recollection of that CL is that the error stems from something like make([]T, n) where n is a named int type.

[dsnet]

Not urgent. It was 1 build failure out of many thousands (passing).

I should also note that I'm discovering these issues from work intended to discover bugs earlier on in the development cycle.

[davecheney]

I would prefer to continue the practice of reverting changes rather than
trying to muscle through.

On Thu, Sep 1, 2016 at 10:55 AM, Joe Tsai notifications@github.com wrote:

Not urgent. It was 1 build failure out of many thousands.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#16940 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAAcA_48GV_J93Zl5J8PIKeVMVNWuXwkks5qliKCgaJpZM4JyIc4
.

[josharian]

Reproduction:

package src

func f(n T) []byte { return make([]byte, 0, T(0i)) }

type T complex64

Will send a fix tomorrow if @martisch doesn't beat me to it.

As for "muscling through", new code will frequently introduce bugs. Reverting on every single bug, during the open dev period, will add a lot of overhead. I am all for reverting when it's during the freeze, or it causes significant breakage that might impede other developers or cause unnoticed further build breakage, or the bug is subtle or unknown and might take a long time to track down, etc. This is a small bug with a minor impact and a minor fix. Let's just fix it.

It was 1 build failure out of many thousands.

Ouch.

[bradfitz]

I'm fine rolling forward here if the fix is known and everybody can work and build.golang.org isn't all red and impeding workflows.

This just broke on some internal Google code on a test run of tip Go, not affecting anybody, so yay... @dsnet's early testing of Go 1.8 is already bearing fruit.

[martisch]

Working on the fix and new test cases.

[dsnet]

CC me on the fix and I'll test the change on the failing test case.

[gopherbot]

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