You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
package main
type T int
func (t T) M() {}
type I interface {
M()
M2()
}
var t T
var e interface{} = &t
var ok = false
var ch = make(chan int)
func main() {
_, ok = e.(I) // populate itab cache with a false result
go f() // get itab in a loop
var i I
for k := 0; k < 10000; k++ {
i, ok = e.(I) // read the cached itab
if ok {
println("iteration", k, "i =", i, "&t =", &t)
panic("conversion succeeded")
}
}
<-ch
}
func f() {
for i := 0; i < 10000; i++ {
f1()
}
ch <- 1
}
func f1() {
defer func() {
err := recover()
if err == nil {
panic("did not panic")
}
}()
i := e.(I) // triggers itab.init, for getting the panic string
_ = i
}
What did you expect to see?
Not panic.
We do conversion of e (containing &t) to I concurrently, one with ",ok" form (in f1), one without (in main). T doesn't implement I, so the conversion should fail, and the program should not panic.
What did you see instead?
Panics.
$ go run i.go
iteration 2778 i = (0x1161008,0x10ddc98) &t = 0x10ddc98
panic: conversion succeeded
goroutine 1 [running]:
main.main()
/tmp/i.go:27 +0x1a2
exit status 2
$ go run i.go
iteration 1681 i = (0x1161008,0x10ddc98) &t = 0x10ddc98
panic: conversion succeeded
goroutine 1 [running]:
main.main()
/tmp/i.go:27 +0x1a2
exit status 2
After some iterations, the conversion in main succeeded, with an I interface containing an &t, which is bad.
I think what happens here is:
In the first conversion (early in main), we populate the itab cache with a false result.
In f1, we get the same itab from the cache. As this is not ",ok" form, getitab calls m.init to find out what method is missing for the panic message. m.init writes m.fun[0] to T.M's code pointer as it sees the M method. Before return it sets m.fun[0] = 0 as it couldn't find the M2 method. There is a small window that m.fun[0] is not 0.
In the loop in main, we get the same itab from the cache, check its m.fun[0] field in getitab. If this happens in the middle of the window that m.fun[0] is not 0, it thinks the conversion succeeded.
I think we can fix this by not setting m.fun[0] immediately when we find the first method. Instead, store the code pointer in a variable, and set m.fun[0] only after we know the conversion succeeds. Will send a fix.
Found this while I'm porting itab caching to gccgo.
What version of Go are you using (
go version
)?tip (3cb92fc)
Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?darwin/amd64, also linux/amd64
What did you do?
https://play.golang.org/p/3QdnC62q1Vb
What did you expect to see?
Not panic.
We do conversion of
e
(containing&t
) toI
concurrently, one with ",ok" form (inf1
), one without (inmain
).T
doesn't implementI
, so the conversion should fail, and the program should not panic.What did you see instead?
Panics.
After some iterations, the conversion in
main
succeeded, with anI
interface containing an&t
, which is bad.I think what happens here is:
getitab
callsm.init
to find out what method is missing for the panic message.m.init
writesm.fun[0]
toT.M
's code pointer as it sees theM
method. Before return it setsm.fun[0] = 0
as it couldn't find theM2
method. There is a small window thatm.fun[0]
is not 0.m.fun[0]
field ingetitab
. If this happens in the middle of the window thatm.fun[0]
is not 0, it thinks the conversion succeeded.I think we can fix this by not setting
m.fun[0]
immediately when we find the first method. Instead, store the code pointer in a variable, and setm.fun[0]
only after we know the conversion succeeds. Will send a fix.Found this while I'm porting itab caching to gccgo.
cc @aclements @randall77
The text was updated successfully, but these errors were encountered: