From d6294e00f029d93b8552827bce1f24f67458d3f2 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Thu, 18 Aug 2022 04:56:10 -0700 Subject: [PATCH] cmd/compile: fix devirtualization bug with unified IR 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 Run-TryBot: Matthew Dempsky Reviewed-by: Keith Randall TryBot-Result: Gopher Robot Reviewed-by: Cuong Manh Le --- .../internal/devirtualize/devirtualize.go | 19 ++++++++++++++ test/typeparam/mdempsky/21.go | 26 +++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 test/typeparam/mdempsky/21.go diff --git a/src/cmd/compile/internal/devirtualize/devirtualize.go b/src/cmd/compile/internal/devirtualize/devirtualize.go index f64ebc87d0..b620470b0e 100644 --- a/src/cmd/compile/internal/devirtualize/devirtualize.go +++ b/src/cmd/compile/internal/devirtualize/devirtualize.go @@ -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 index 0000000000..da10ae3ea3 --- /dev/null +++ b/test/typeparam/mdempsky/21.go @@ -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() {}