cmd/compile: assignments to _ are not evaluated for possible panics #5790

gopherbot · 2013-06-26T12:20:19Z

by capnm9:

This is probably a compiler bug:

http://play.golang.org/p/9CWVL3YTpq

package main
func main() {
    a := []int16{2, 0}
    _ = a[0] / a[1]
    print("still alive")
}


Additionally for int64 it panics on arm:

go run main.go
runtime: unknown pc 0x2 at frame 12
fatal error: invalid stack

goroutine 1 [running]:
[fp=0xb6f6fe50] runtime.gc(0x0)
    /d/go/src/pkg/runtime/mgc0.c:2017 +0x1b8
[fp=0xb6f6fe84] runtime.mallocgc(0x8, 0x0, 0x1, 0x1)
    /d/go/src/pkg/runtime/zmalloc_linux_arm.c:103 +0x1d0
[fp=0xb6f6fe98] runtime.mal(0x8)
    /d/go/src/pkg/runtime/zmalloc_linux_arm.c:665 +0x3c
[fp=0xb6f6feb4] copyin(0x2d478, 0xb6f6fed8, 0xb6f6fee4)
    /d/go/src/pkg/runtime/iface.c:152 +0x60
[fp=0xb6f6fed0] runtime.convT2E()
    /d/go/src/pkg/runtime/iface.c:220 +0x44
[fp=0xb6f6fee8] runtime.newErrorString(0x520c0, 0x16, 0xb6f6ff00)
    /d/go/src/pkg/runtime/error.go:74 +0x38
[fp=0xb6f6ff08] runtime.panicstring(0x520c0)
    /d/go/src/pkg/runtime/panic.c:488 +0x78
[fp=0xb6f6ff10] runtime.panicdivide()
    /d/go/src/pkg/runtime/vlrt_arm.c:40 +0x28
[fp=0xb6f6ff10] udiv()
    /d/go/src/pkg/runtime/vlop_arm.s:139 +0x9c
[fp=0xb6f6ff34] _divv(0x558a8, 0x131c0, 0x14824, 0x10312000, 0x0, ...)
    /d/go/src/pkg/runtime/vlrt_arm.c:332 +0x10c
[fp=0xb6f6ff58] _divv(0x0, 0x0, 0x0, 0x1f304, 0x2, ...)
    /d/go/src/pkg/runtime/vlrt_arm.c:332 +0x10c
[fp=0xb6f6ff7c] runtime.lock(0x217bc)
    /d/go/src/pkg/runtime/lock_futex.c:44 +0x4c

goroutine 2 [runnable]:
runtime.MHeap_Scavenger()
    /d/go/src/pkg/runtime/mheap.c:438
created by runtime.main
    /d/go/src/pkg/runtime/proc.c:166

ianlancetaylor · 2013-06-26T13:28:33Z

Comment 1:

If you assign to an actual variable, then the program panics.  The problem appears to be
that because you are assigning to _, the expression is not evaluated.  I suppose this
ought to be fixed but it's hardly likely to be a problem in real code.

Labels changed: added priority-someday, compilerbug, removed priority-triage.

gopherbot · 2013-06-26T14:12:44Z

Comment 2 by capnm9:

Looks like the expression is (at least partially) evaluated for int64
on 32 bit architecture.
 
It came up while I was looking at dave's benchmark (agree, hardly a real code):
for n := 0; n < b.N; n++ {
    for i, row := range a {
        for j, _ := range row {
            println(a[j][i])
            _ = a[i][j] / a[j][i]
        }
    }
}

gopherbot · 2013-06-26T22:23:06Z

Comment 3 by capnm9:

Ah, this is an unrelated arm ?runtime bug?
func main() {
    a := 0
    b := 1 / a
    _ = b
}
amd64:
panic: runtime error: integer divide by zero
[signal 0x8 code=0x1 addr=0x400c12 pc=0x400c12]
goroutine 1 [running]:
main.main()
    /tmp/main.go:7 +0x12
arm6:
runtime: unknown pc 0x1 at frame 12
fatal error: invalid stack
goroutine 1 [running]:
[fp=0xb6f8fe70] runtime.gc(0x0)
    /d/go/src/pkg/runtime/mgc0.c:2017 +0x1b8
[fp=0xb6f8fea4] runtime.mallocgc(0x8, 0x0, 0x1, 0x1)
    /d/go/src/pkg/runtime/zmalloc_linux_arm.c:103 +0x1d0
...

davecheney · 2013-06-26T22:42:16Z

Comment 4:

@capnm9 which hg id are you at?

gopherbot · 2013-06-26T23:58:04Z

Comment 5 by capnm9:

Both are c96951f239c0 tip. Go1.1.1 panics as expected
with divide by zero.
uname -a
Linux rpi 3.6.11+ #474 PREEMPT Thu Jun 13 17:14:42 BST 2013 armv6l GNU/Linux
Should I fill an another bug?

ianlancetaylor · 2013-06-27T00:00:12Z

Comment 6:

If the ARM problem still occurs after rsc commits 10360048 to the repository, then
please file a separate bug for that.  Thanks.

rsc · 2013-06-27T15:50:26Z

Comment 7:

Are assignments to _ required to be evaluated for possible panics? I don't remember, and
at first glance I don't see an answer in the spec.

Status changed to Thinking.

griesemer · 2013-06-27T16:08:13Z

Comment 8:

The spec is not explicit about it, but there's no word why it should not be evaluated.
For one, the function f in http://play.golang.org/p/3iGuytfm0F is called even though its
result is assigned to _ . Assignment to _ permits "throwing away" values we don't care
about, the values are still computed.

Status changed to Accepted.

griesemer · 2013-06-27T16:18:31Z

Comment 9:

Interestingly, while
    a, b := 1, 0
    _ = a/b
does not panic; the following does:
    a, b := 1, 0
    _, _ = 1, a/b
as does:
    a, b := 1, 0
    x, _ := 1, a/b
So gc is not consistent in this case. Again, I argue that the expressions on the rhs
must be evaluated if there are observable side-effects (perhaps excluding execution
time).

gopherbot · 2013-06-28T10:55:05Z

Comment 10 by capnm9:

@iant done https://golang.org/issue/5805
Panic in the runtime would be (sort of) consistent with
the static check:
    a := 1
    _ = a/0
./main.go:7: division by zero

rsc · 2013-07-30T13:41:03Z

Comment 11:

Labels changed: added go1.2maybe.

rsc · 2013-07-30T22:38:43Z

Comment 12:

Labels changed: added feature.

robpike · 2013-08-30T06:03:37Z

Comment 13:

Not for 1.2.

Labels changed: removed go1.2maybe.

rsc · 2013-11-27T18:45:42Z

Comment 14:

Labels changed: added go1.3maybe.

rsc · 2013-11-27T20:29:33Z

Comment 15:

Labels changed: removed feature.

rsc · 2013-12-04T01:29:04Z

Comment 16:

Labels changed: added release-none, removed go1.3maybe.

rsc · 2013-12-04T01:43:48Z

Comment 17:

Labels changed: added repo-main.

odeke-em · 2016-12-23T11:18:58Z

At least for the original bug report in #5790 (comment),
it panics as we'd like, on tip

$ go version
go version devel +8887be4 Thu Dec 22 22:43:44 2016 +0000 darwin/amd64
$ cat main.go  && go run main.go
package main

import "fmt"

func main() {
	b := []int16{2, 0}
	_ = b[0] / b[1]
	var x int
	fmt.Printf("still alive: %v\n", x)
}
panic: runtime error: integer divide by zero

goroutine 1 [running]:
main.main()
	/Users/emmanuelodeke/Desktop/openSrc/bugs/golang/5790/main.go:7 +0xc6
exit status 2

/cc @randall77 and the other compiler folks, was this bug fixed as one of the
gracious gifts/side effects of SSA?

I'll submit a CL to lock in this test/behavior, but for starters I've filed #18421 so that we can fix the condition
in which zerodivide errors in test/ were never being tested.

gopherbot · 2016-12-23T11:30:29Z

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

randall77 · 2016-12-23T17:11:20Z

Yes, this was probably fixed as a side effect of the SSA conversion.

gopherbot added accepted labels Dec 4, 2013

rsc added this to the Unplanned milestone Apr 10, 2015

rsc removed priority-someday labels Apr 10, 2015

rsc changed the title ~~cmd/gc: assignments to _ are not evaluated for possible panics~~ cmd/compile: assignments to _ are not evaluated for possible panics Jun 8, 2015

cmars mentioned this issue Jun 13, 2015

Benchmark & cmd cleanups cmars/basen#2

Merged

odeke-em mentioned this issue Dec 23, 2016

test/zerodivide.go: all expected zerodivide panic errors are skipped #18421

Closed

gopherbot closed this as completed in c8f1436 Dec 23, 2016

golang locked and limited conversation to collaborators Dec 23, 2017

gopherbot added the FrozenDueToAge label Dec 23, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/compile: assignments to _ are not evaluated for possible panics #5790

cmd/compile: assignments to _ are not evaluated for possible panics #5790

gopherbot commented Jun 26, 2013

ianlancetaylor commented Jun 26, 2013

gopherbot commented Jun 26, 2013

gopherbot commented Jun 26, 2013

davecheney commented Jun 26, 2013

gopherbot commented Jun 26, 2013

ianlancetaylor commented Jun 27, 2013

rsc commented Jun 27, 2013

griesemer commented Jun 27, 2013

griesemer commented Jun 27, 2013

gopherbot commented Jun 28, 2013

rsc commented Jul 30, 2013

rsc commented Jul 30, 2013

robpike commented Aug 30, 2013

rsc commented Nov 27, 2013

rsc commented Nov 27, 2013

rsc commented Dec 4, 2013

rsc commented Dec 4, 2013

odeke-em commented Dec 23, 2016

gopherbot commented Dec 23, 2016

randall77 commented Dec 23, 2016

cmd/compile: assignments to _ are not evaluated for possible panics #5790

cmd/compile: assignments to _ are not evaluated for possible panics #5790

Comments

gopherbot commented Jun 26, 2013

ianlancetaylor commented Jun 26, 2013

gopherbot commented Jun 26, 2013

gopherbot commented Jun 26, 2013

davecheney commented Jun 26, 2013

gopherbot commented Jun 26, 2013

ianlancetaylor commented Jun 27, 2013

rsc commented Jun 27, 2013

griesemer commented Jun 27, 2013

griesemer commented Jun 27, 2013

gopherbot commented Jun 28, 2013

rsc commented Jul 30, 2013

rsc commented Jul 30, 2013

robpike commented Aug 30, 2013

rsc commented Nov 27, 2013

rsc commented Nov 27, 2013

rsc commented Dec 4, 2013

rsc commented Dec 4, 2013

odeke-em commented Dec 23, 2016

gopherbot commented Dec 23, 2016

randall77 commented Dec 23, 2016