runtime: wrong line number in panic call stack

[gopherbot]

by daviddengcn:

1. What is a short input program that triggers the error?
package main

func PanicInGoSrc() {
    panic("panic")
}

var a string = "abc"

func doPanic() error {
    if a == "" {
        return nil  // Line 11
    }
    
    defer PanicInGoSrc()
    
    return nil          // Line 16
}

func main() {
    doPanic()
}

2. What is the full compiler output?
panic: panic

goroutine 1 [running]:
main.PanicInGoSrc()
        F:/job/go/src/wrongpanicline.go:4 +0x43
main.doPanic(0x0, 0x0)
        F:/job/go/src/wrongpanicline.go:11 +0x44
main.main()
        F:/job/go/src/wrongpanicline.go:20 +0x1e

goroutine 2 [runnable]:
exit status 2

3. What version of the compiler are you using?  (Run it with the -V flag.)
go version go1.1.1 windows/386

The line number in main.doPanic is not 11, should be 16, this confuse me a lot before I
found this is a compiler bug.

[gopherbot]

Comment 1 by daviddengcn:

1. slightly modify the program, the line number changes (still wrong)
package main
func PanicInGoSrc() {
    panic("panic")
}
var a string = "abc"
func doPanic() {
    if a == "" {
        return // Line 11
    }
    
    defer PanicInGoSrc()
    
    return     // Line 16
}
func main() {
    doPanic()
}
2. compiler output
panic: panic
goroutine 1 [running]:
main.PanicInGoSrc()
        F:/job/go/src/wrongpanicline.go:4 +0x43
main.doPanic()
        F:/job/go/src/wrongpanicline.go:10 +0x21
main.main()
        F:/job/go/src/wrongpanicline.go:20 +0x1b
goroutine 2 [runnable]:
exit status 2

[robpike]

Comment 2:

This will likely be fixed by the pending changes to the representation of the PC/line
table.

Labels changed: added priority-later, removed priority-triage.

Status changed to Accepted.

[rsc]

Comment 3:

Actually it is unclear. Defer needs to be handled a bit better in the stack traces in
general. 
There are some good ideas on issue #5832, but there might be a real bug here, so I'm
leaving this one separate.

[minux]

Comment 4:

the problem here is that the function only has a single call to
runtime.deferreturn, so we can't differentiate different return
statements.
unless our pcln table could map a single pc to multiple line
(and then we also has the problem of determine which one
to use)
we should make cmd/gc emit one runtime.deferreturn for
every return statement.

[rsc]

Comment 5:

I don't think we have to show the specific return statement: the return is over.
We could show line 17 (the final }), for example. But we can also recognize the
deferreturn pattern and print a note about this being a deferred call.

[rsc]

Comment 6:

I looked into having separate deferreturns, but it's tricky. 
The CALL deferreturn is already tagged with the line number of the function's closing },
but that line number is not actually relevant to the stack trace. The deferred function
call was set up through abnormal means, and it is _returning to_ the CALL deferreturn
instruction, not called by it. The traceback does what it always does, which is to print
the line number of the instruction before the return PC (in normal operation, that's the
CALL). Here, because the call was simulated by the runtime package, the instruction
before the return PC is completely unrelated. The way the code was laid out, it happens
to be something on line 11.
The solution is to emit a real no-op instruction with the correct line number before the
CALL deferreturn, so that when the traceback looks at the line number of the instruction
before the return PC, it will get the right answer.
goroutine 1 [running]:
main.PanicInGoSrc()
    /Users/rsc/x.go:4 +0x43
main.doPanic()
    /Users/rsc/x.go:17 +0x23
main.main()
    /Users/rsc/x.go:20 +0x1b

Owner changed to @rsc.

Status changed to Started.

[rsc]

Comment 7:

This issue was closed by revision 7e97d39.

Status changed to Fixed.