]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: fix devirtualization bug with unified IR
authorMatthew Dempsky <mdempsky@google.com>
Thu, 18 Aug 2022 11:56:10 +0000 (04:56 -0700)
committerMatthew Dempsky <mdempsky@google.com>
Thu, 18 Aug 2022 17:26:32 +0000 (17:26 +0000)
As a consistency check in devirtualization, when we determine `i` (of
interface type `I`) always has dynamic type `T`, we insert a type
assertion `i.(T)`. This emits an itab check for `go:itab.T,I`, but
it's always true (and so SSA optimizes it away).

However, if `I` is instead the generic interface type `I[T]`, then
`go:itab.T,I[int]` and `go:itab.T,I[go.shape.int]` are equivalent but
distinct itabs. And notably, we'll have originally created the
interface value using the former; but the (non-dynamic) TypeAssertExpr
created by devirtualization would ultimately emit a comparison against
the latter. This comparison would then evaluate false, leading to a
spurious type assertion panic at runtime.

The comparison is just meant as an extra safety check, so it should be
safe to just disable. But for now, it's simpler/safer to just punt on
devirtualization in this case. (The non-unified frontend doesn't
devirtualize this either.)

Change-Id: I6a8809bcfebc9571f32e289fa4bc6a8b0d21ca46
Reviewed-on: https://go-review.googlesource.com/c/go/+/424774
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
src/cmd/compile/internal/devirtualize/devirtualize.go
test/typeparam/mdempsky/21.go [new file with mode: 0644]

index f64ebc87d0298adab223d8db598565d13bf91205..b620470b0e83a61a12ad77cd0b4f1b02e95bd74c 100644 (file)
@@ -74,6 +74,25 @@ func Call(call *ir.CallExpr) {
                        }
                        return
                }
+
+               // Further, if sel.X's type has a shape type, then it's a shaped
+               // interface type. In this case, the (non-dynamic) TypeAssertExpr
+               // we construct below would attempt to create an itab
+               // corresponding to this shaped interface type; but the actual
+               // itab pointer in the interface value will correspond to the
+               // original (non-shaped) interface type instead. These are
+               // functionally equivalent, but they have distinct pointer
+               // identities, which leads to the type assertion failing.
+               //
+               // TODO(mdempsky): We know the type assertion here is safe, so we
+               // could instead set a flag so that walk skips the itab check. For
+               // now, punting is easy and safe.
+               if sel.X.Type().HasShape() {
+                       if base.Flag.LowerM != 0 {
+                               base.WarnfAt(call.Pos(), "cannot devirtualize %v: shaped interface %v", call, sel.X.Type())
+                       }
+                       return
+               }
        }
 
        dt := ir.NewTypeAssertExpr(sel.Pos(), sel.X, nil)
diff --git a/test/typeparam/mdempsky/21.go b/test/typeparam/mdempsky/21.go
new file mode 100644 (file)
index 0000000..da10ae3
--- /dev/null
@@ -0,0 +1,26 @@
+// run
+
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Test that devirtualization doesn't introduce spurious type
+// assertion failures due to shaped and non-shaped interfaces having
+// distinct itabs.
+
+package main
+
+func main() {
+       F[int]()
+}
+
+func F[T any]() {
+       var i I[T] = X(0)
+       i.M()
+}
+
+type I[T any] interface{ M() }
+
+type X int
+
+func (X) M() {}