From d3d5a3bceff0a2a7ff182ddb2d69ee5a5af1e645 Mon Sep 17 00:00:00 2001 From: Mateusz Poliwczak Date: Thu, 13 Feb 2025 11:25:31 +0100 Subject: [PATCH 1/8] cmd/compile: devirtualize interface calls with type assertions --- .../internal/devirtualize/devirtualize.go | 10 +- src/cmd/compile/internal/ir/expr.go | 49 +++++- src/cmd/compile/internal/typecheck/subr.go | 4 + ...scape_iface_with_devirt_type_assertions.go | 151 ++++++++++++++++++ 4 files changed, 205 insertions(+), 9 deletions(-) create mode 100644 test/escape_iface_with_devirt_type_assertions.go diff --git a/src/cmd/compile/internal/devirtualize/devirtualize.go b/src/cmd/compile/internal/devirtualize/devirtualize.go index 372d05809401ff..604a9edee006b8 100644 --- a/src/cmd/compile/internal/devirtualize/devirtualize.go +++ b/src/cmd/compile/internal/devirtualize/devirtualize.go @@ -40,14 +40,8 @@ func StaticCall(call *ir.CallExpr) { } sel := call.Fun.(*ir.SelectorExpr) - r := ir.StaticValue(sel.X) - if r.Op() != ir.OCONVIFACE { - return - } - recv := r.(*ir.ConvExpr) - - typ := recv.X.Type() - if typ.IsInterface() { + typ := ir.StaticType(sel.X) + if typ == nil { return } diff --git a/src/cmd/compile/internal/ir/expr.go b/src/cmd/compile/internal/ir/expr.go index 4a2e996569450d..ad419156439b7a 100644 --- a/src/cmd/compile/internal/ir/expr.go +++ b/src/cmd/compile/internal/ir/expr.go @@ -840,6 +840,37 @@ func IsAddressable(n Node) bool { return false } +var Implements = func(t, iface *types.Type) bool { + panic("unreachable") +} + +// StaticType is like StaticValue but for types. +func StaticType(n Node) *types.Type { + out, typs := staticValue(n, true) + + if out.Op() != OCONVIFACE { + return nil + } + + recv := out.(*ConvExpr) + + typ := recv.X.Type() + if typ.IsInterface() { + return nil + } + + // Make sure that every type assertion that involves interfaes is satisfied. + for _, t := range typs { + if t.IsInterface() { + if !Implements(typ, t) { + return nil + } + } + } + + return typ +} + // StaticValue analyzes n to find the earliest expression that always // evaluates to the same value as n, which might be from an enclosing // function. @@ -855,6 +886,16 @@ func IsAddressable(n Node) bool { // calling StaticValue on the "int(y)" expression returns the outer // "g()" expression. func StaticValue(n Node) Node { + v, t := staticValue(n, false) + if len(t) != 0 { + base.Fatalf("len(t) != 0; len(t) = %v", len(t)) + } + return v + +} + +func staticValue(n Node, forDevirt bool) (Node, []*types.Type) { + typeAssertTypes := []*types.Type{} for { switch n1 := n.(type) { case *ConvExpr: @@ -870,11 +911,17 @@ func StaticValue(n Node) Node { case *ParenExpr: n = n1.X continue + case *TypeAssertExpr: + if forDevirt && n1.Op() == ODOTTYPE { + typeAssertTypes = append(typeAssertTypes, n1.Type()) + n = n1.X + continue + } } n1 := staticValue1(n) if n1 == nil { - return n + return n, typeAssertTypes } n = n1 } diff --git a/src/cmd/compile/internal/typecheck/subr.go b/src/cmd/compile/internal/typecheck/subr.go index 3b22d260bf4faf..32e67a22199fee 100644 --- a/src/cmd/compile/internal/typecheck/subr.go +++ b/src/cmd/compile/internal/typecheck/subr.go @@ -612,6 +612,10 @@ func Implements(t, iface *types.Type) bool { return implements(t, iface, &missing, &have, &ptr) } +func init() { + ir.Implements = Implements +} + // ImplementsExplain reports whether t implements the interface iface. t can be // an interface, a type parameter, or a concrete type. If t does not implement // iface, a non-empty string is returned explaining why. diff --git a/test/escape_iface_with_devirt_type_assertions.go b/test/escape_iface_with_devirt_type_assertions.go new file mode 100644 index 00000000000000..300e077f136950 --- /dev/null +++ b/test/escape_iface_with_devirt_type_assertions.go @@ -0,0 +1,151 @@ +// errorcheck -0 -m + +// Copyright 2025 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. + +package escape + +import ( + "crypto/sha256" + "encoding" + "hash" + "io" +) + +type M interface{ M() } + +type A interface{ A() } + +type C interface{ C() } + +type Impl struct{} + +func (*Impl) M() {} // ERROR "can inline" + +func (*Impl) A() {} // ERROR "can inline" + +type CImpl struct{} + +func (CImpl) C() {} // ERROR "can inline" + +func t() { + var a M = &Impl{} // ERROR "&Impl{} does not escape" + + a.(M).M() // ERROR "devirtualizing a.\(M\).M" "inlining call" + a.(A).A() // ERROR "devirtualizing a.\(A\).A" "inlining call" + a.(*Impl).M() // ERROR "inlining call" + a.(*Impl).A() // ERROR "inlining call" + + v := a.(M) + v.M() // ERROR "devirtualizing v.M" "inlining call" + v.(A).A() // ERROR "devirtualizing v.\(A\).A" "inlining call" + v.(*Impl).A() // ERROR "inlining call" + v.(*Impl).M() // ERROR "inlining call" + + v2 := a.(A) + v2.A() // ERROR "devirtualizing v2.A" "inlining call" + v2.(M).M() // ERROR "devirtualizing v2.\(M\).M" "inlining call" + v2.(*Impl).A() // ERROR "inlining call" + v2.(*Impl).M() // ERROR "inlining call" + + a.(M).(A).A() // ERROR "devirtualizing a.\(M\).\(A\).A" "inlining call" + a.(A).(M).M() // ERROR "devirtualizing a.\(A\).\(M\).M" "inlining call" + + a.(M).(A).(*Impl).A() // ERROR "inlining call" + a.(A).(M).(*Impl).M() // ERROR "inlining call" + + { + var a C = &CImpl{} // ERROR "does not escape" + a.(any).(C).C() // ERROR "devirtualizing" "inlining" + a.(any).(*CImpl).C() // ERROR "inlining" + } +} + +// TODO: these type assertions could also be devirtualized. +func t2() { + { + var a M = &Impl{} // ERROR "&Impl{} escapes to heap" + if v, ok := a.(M); ok { + v.M() + } + } + { + var a M = &Impl{} // ERROR "&Impl{} escapes to heap" + if v, ok := a.(A); ok { + v.A() + } + } + { + var a M = &Impl{} // ERROR "&Impl{} escapes to heap" + v, ok := a.(M) + if ok { + v.M() + } + } + { + var a M = &Impl{} // ERROR "&Impl{} escapes to heap" + v, ok := a.(A) + if ok { + v.A() + } + } + { + var a M = &Impl{} // ERROR "does not escape" + v, ok := a.(*Impl) + if ok { + v.A() // ERROR "inlining" + v.M() // ERROR "inlining" + } + } + { + var a M = &Impl{} // ERROR "&Impl{} escapes to heap" + v, _ := a.(M) + v.M() + } + { + var a M = &Impl{} // ERROR "&Impl{} escapes to heap" + v, _ := a.(A) + v.A() + } + { + var a M = &Impl{} // ERROR "does not escape" + v, _ := a.(*Impl) + v.A() // ERROR "inlining" + v.M() // ERROR "inlining" + } +} + +//go:noinline +func testInvalidAsserts() { + { + var a M = &Impl{} // ERROR "escapes" + a.(C).C() // this will panic + a.(any).(C).C() // this will panic + } + { + var a C = &CImpl{} // ERROR "escapes" + a.(M).M() // this will panic + a.(any).(M).M() // this will panic + } + { + var a C = &CImpl{} // ERROR "does not escape" + + // this will panic + a.(M).(*Impl).M() // ERROR "inlining" + + // this will panic + a.(any).(M).(*Impl).M() // ERROR "inlining" + } +} + +func testSha256() { + h := sha256.New() // ERROR "inlining call" "does not escape" + h.Write(nil) // ERROR "devirtualizing" + h.(io.Writer).Write(nil) // ERROR "devirtualizing" + h.(hash.Hash).Write(nil) // ERROR "devirtualizing" + h.(encoding.BinaryUnmarshaler).UnmarshalBinary(nil) // ERROR "devirtualizing" + + h2 := sha256.New() // ERROR "escapes" "inlining call" + h2.(M).M() // this will panic +} From 68ee1cb04ec60d9d160aee6ec86ad954abad3a71 Mon Sep 17 00:00:00 2001 From: Mateusz Poliwczak Date: Thu, 13 Feb 2025 15:01:46 +0100 Subject: [PATCH 2/8] make sha256 test in crypto/sha256 package --- src/crypto/sha256/sha256_test.go | 14 ++++++++++++++ ...escape_iface_with_devirt_type_assertions.go | 18 ------------------ 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/src/crypto/sha256/sha256_test.go b/src/crypto/sha256/sha256_test.go index e1af9640e25547..04be3461075017 100644 --- a/src/crypto/sha256/sha256_test.go +++ b/src/crypto/sha256/sha256_test.go @@ -391,3 +391,17 @@ func BenchmarkHash1K(b *testing.B) { func BenchmarkHash8K(b *testing.B) { benchmarkSize(b, 8192) } + +func TestAllocatonsWithTypeAsserts(t *testing.T) { + cryptotest.SkipTestAllocations(t) + allocs := testing.AllocsPerRun(100, func() { + h := New() + h.Write([]byte{1, 2, 3}) + marshaled, _ := h.(encoding.BinaryMarshaler).MarshalBinary() + marshaled, _ = h.(encoding.BinaryAppender).AppendBinary(marshaled[:0]) + h.(encoding.BinaryUnmarshaler).UnmarshalBinary(marshaled) + }) + if allocs != 0 { + t.Fatalf("allocs = %v; want = 0", allocs) + } +} diff --git a/test/escape_iface_with_devirt_type_assertions.go b/test/escape_iface_with_devirt_type_assertions.go index 300e077f136950..cc81573b66d8d3 100644 --- a/test/escape_iface_with_devirt_type_assertions.go +++ b/test/escape_iface_with_devirt_type_assertions.go @@ -6,13 +6,6 @@ package escape -import ( - "crypto/sha256" - "encoding" - "hash" - "io" -) - type M interface{ M() } type A interface{ A() } @@ -138,14 +131,3 @@ func testInvalidAsserts() { a.(any).(M).(*Impl).M() // ERROR "inlining" } } - -func testSha256() { - h := sha256.New() // ERROR "inlining call" "does not escape" - h.Write(nil) // ERROR "devirtualizing" - h.(io.Writer).Write(nil) // ERROR "devirtualizing" - h.(hash.Hash).Write(nil) // ERROR "devirtualizing" - h.(encoding.BinaryUnmarshaler).UnmarshalBinary(nil) // ERROR "devirtualizing" - - h2 := sha256.New() // ERROR "escapes" "inlining call" - h2.(M).M() // this will panic -} From 0ec4e57651d18e348964abb5ccaad8f525d8b55e Mon Sep 17 00:00:00 2001 From: Mateusz Poliwczak Date: Thu, 13 Feb 2025 15:20:52 +0100 Subject: [PATCH 3/8] also follow OCONVIFACE --- src/cmd/compile/internal/ir/expr.go | 13 ++++++------- test/escape_iface_with_devirt_type_assertions.go | 8 ++++++++ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/cmd/compile/internal/ir/expr.go b/src/cmd/compile/internal/ir/expr.go index ad419156439b7a..075e4f122615ce 100644 --- a/src/cmd/compile/internal/ir/expr.go +++ b/src/cmd/compile/internal/ir/expr.go @@ -848,13 +848,7 @@ var Implements = func(t, iface *types.Type) bool { func StaticType(n Node) *types.Type { out, typs := staticValue(n, true) - if out.Op() != OCONVIFACE { - return nil - } - - recv := out.(*ConvExpr) - - typ := recv.X.Type() + typ := out.Type() if typ.IsInterface() { return nil } @@ -903,6 +897,11 @@ func staticValue(n Node, forDevirt bool) (Node, []*types.Type) { n = n1.X continue } + if forDevirt && n1.Op() == OCONVIFACE { + typeAssertTypes = append(typeAssertTypes, n1.Type()) + n = n1.X + continue + } case *InlinedCallExpr: if n1.Op() == OINLCALL { n = n1.SingleResult() diff --git a/test/escape_iface_with_devirt_type_assertions.go b/test/escape_iface_with_devirt_type_assertions.go index cc81573b66d8d3..59c3652c82f9f5 100644 --- a/test/escape_iface_with_devirt_type_assertions.go +++ b/test/escape_iface_with_devirt_type_assertions.go @@ -48,6 +48,14 @@ func t() { a.(M).(A).(*Impl).A() // ERROR "inlining call" a.(A).(M).(*Impl).M() // ERROR "inlining call" + any(a).(M).M() // ERROR "devirtualizing" "inlining call" + any(a).(A).A() // ERROR "devirtualizing" "inlining call" + any(a).(M).(any).(A).A() // ERROR "devirtualizing" "inlining call" + + c := any(a) + c.(A).A() // ERROR "devirtualizing" "inlining call" + c.(M).M() // ERROR "devirtualizing" "inlining call" + { var a C = &CImpl{} // ERROR "does not escape" a.(any).(C).C() // ERROR "devirtualizing" "inlining" From 96ba2f919e0fc0f4e7d4ba34c9be97d4a9d22410 Mon Sep 17 00:00:00 2001 From: Mateusz Poliwczak Date: Fri, 14 Feb 2025 15:08:45 +0100 Subject: [PATCH 4/8] update Change-Id: I72f42d4b8ccf83bcd74cb7a91bbf3dad328c496b --- .../internal/devirtualize/devirtualize.go | 4 +++ src/cmd/compile/internal/ir/expr.go | 30 ++++--------------- src/cmd/compile/internal/typecheck/subr.go | 4 --- 3 files changed, 9 insertions(+), 29 deletions(-) diff --git a/src/cmd/compile/internal/devirtualize/devirtualize.go b/src/cmd/compile/internal/devirtualize/devirtualize.go index 604a9edee006b8..5bfeffa4612f7c 100644 --- a/src/cmd/compile/internal/devirtualize/devirtualize.go +++ b/src/cmd/compile/internal/devirtualize/devirtualize.go @@ -45,6 +45,10 @@ func StaticCall(call *ir.CallExpr) { return } + if !typecheck.Implements(typ, sel.X.Type()) { + return + } + // If typ is a shape type, then it was a type argument originally // and we'd need an indirect call through the dictionary anyway. // We're unable to devirtualize this call. diff --git a/src/cmd/compile/internal/ir/expr.go b/src/cmd/compile/internal/ir/expr.go index 075e4f122615ce..1f1f336503e7fb 100644 --- a/src/cmd/compile/internal/ir/expr.go +++ b/src/cmd/compile/internal/ir/expr.go @@ -840,28 +840,15 @@ func IsAddressable(n Node) bool { return false } -var Implements = func(t, iface *types.Type) bool { - panic("unreachable") -} - -// StaticType is like StaticValue but for types. +// StaticType is like StaticValue, but also follows ODOTTYPE and OCONVIFACE. func StaticType(n Node) *types.Type { - out, typs := staticValue(n, true) + out := staticValue(n, true) typ := out.Type() if typ.IsInterface() { return nil } - // Make sure that every type assertion that involves interfaes is satisfied. - for _, t := range typs { - if t.IsInterface() { - if !Implements(typ, t) { - return nil - } - } - } - return typ } @@ -880,16 +867,11 @@ func StaticType(n Node) *types.Type { // calling StaticValue on the "int(y)" expression returns the outer // "g()" expression. func StaticValue(n Node) Node { - v, t := staticValue(n, false) - if len(t) != 0 { - base.Fatalf("len(t) != 0; len(t) = %v", len(t)) - } - return v + return staticValue(n, false) } -func staticValue(n Node, forDevirt bool) (Node, []*types.Type) { - typeAssertTypes := []*types.Type{} +func staticValue(n Node, forDevirt bool) Node { for { switch n1 := n.(type) { case *ConvExpr: @@ -898,7 +880,6 @@ func staticValue(n Node, forDevirt bool) (Node, []*types.Type) { continue } if forDevirt && n1.Op() == OCONVIFACE { - typeAssertTypes = append(typeAssertTypes, n1.Type()) n = n1.X continue } @@ -912,7 +893,6 @@ func staticValue(n Node, forDevirt bool) (Node, []*types.Type) { continue case *TypeAssertExpr: if forDevirt && n1.Op() == ODOTTYPE { - typeAssertTypes = append(typeAssertTypes, n1.Type()) n = n1.X continue } @@ -920,7 +900,7 @@ func staticValue(n Node, forDevirt bool) (Node, []*types.Type) { n1 := staticValue1(n) if n1 == nil { - return n, typeAssertTypes + return n } n = n1 } diff --git a/src/cmd/compile/internal/typecheck/subr.go b/src/cmd/compile/internal/typecheck/subr.go index 32e67a22199fee..3b22d260bf4faf 100644 --- a/src/cmd/compile/internal/typecheck/subr.go +++ b/src/cmd/compile/internal/typecheck/subr.go @@ -612,10 +612,6 @@ func Implements(t, iface *types.Type) bool { return implements(t, iface, &missing, &have, &ptr) } -func init() { - ir.Implements = Implements -} - // ImplementsExplain reports whether t implements the interface iface. t can be // an interface, a type parameter, or a concrete type. If t does not implement // iface, a non-empty string is returned explaining why. From 942f16ada911ffe2d29e8afdc13908250b9c0fe9 Mon Sep 17 00:00:00 2001 From: Mateusz Poliwczak Date: Fri, 14 Feb 2025 19:13:03 +0100 Subject: [PATCH 5/8] devirtualize v, ok := v.(T) Change-Id: I33ebc2734b404d61393f606950829b843b04111a --- src/cmd/compile/internal/ir/expr.go | 15 +++++-- src/cmd/compile/internal/noder/reader.go | 1 + ...scape_iface_with_devirt_type_assertions.go | 44 +++++++++++++------ 3 files changed, 44 insertions(+), 16 deletions(-) diff --git a/src/cmd/compile/internal/ir/expr.go b/src/cmd/compile/internal/ir/expr.go index 1f1f336503e7fb..47a2be98507172 100644 --- a/src/cmd/compile/internal/ir/expr.go +++ b/src/cmd/compile/internal/ir/expr.go @@ -892,21 +892,22 @@ func staticValue(n Node, forDevirt bool) Node { n = n1.X continue case *TypeAssertExpr: - if forDevirt && n1.Op() == ODOTTYPE { + if forDevirt { n = n1.X continue } } - n1 := staticValue1(n) + n1 := staticValue1(n, forDevirt) if n1 == nil { return n } + n = n1 } } -func staticValue1(nn Node) Node { +func staticValue1(nn Node, forDevirt bool) Node { if nn.Op() != ONAME { return nil } @@ -935,6 +936,14 @@ FindRHS: } } base.Fatalf("%v missing from LHS of %v", n, defn) + case OAS2DOTTYPE: + if !forDevirt { + return nil + } + defn := defn.(*AssignListStmt) + if defn.Lhs[0] == n { + rhs = defn.Rhs[0] + } default: return nil } diff --git a/src/cmd/compile/internal/noder/reader.go b/src/cmd/compile/internal/noder/reader.go index eca66487fa26da..bdfef70f216527 100644 --- a/src/cmd/compile/internal/noder/reader.go +++ b/src/cmd/compile/internal/noder/reader.go @@ -2941,6 +2941,7 @@ func (r *reader) multiExpr() []ir.Node { as.Def = true for i := range results { tmp := r.temp(pos, r.typ()) + tmp.Defn = as as.PtrInit().Append(ir.NewDecl(pos, ir.ODCL, tmp)) as.Lhs.Append(tmp) diff --git a/test/escape_iface_with_devirt_type_assertions.go b/test/escape_iface_with_devirt_type_assertions.go index 59c3652c82f9f5..abc72a9641d1ce 100644 --- a/test/escape_iface_with_devirt_type_assertions.go +++ b/test/escape_iface_with_devirt_type_assertions.go @@ -63,32 +63,31 @@ func t() { } } -// TODO: these type assertions could also be devirtualized. func t2() { { - var a M = &Impl{} // ERROR "&Impl{} escapes to heap" + var a M = &Impl{} // ERROR "does not escape" if v, ok := a.(M); ok { - v.M() + v.M() // ERROR "devirtualizing" "inlining call" } } { - var a M = &Impl{} // ERROR "&Impl{} escapes to heap" + var a M = &Impl{} // ERROR "does not escape" if v, ok := a.(A); ok { - v.A() + v.A() // ERROR "devirtualizing" "inlining call" } } { - var a M = &Impl{} // ERROR "&Impl{} escapes to heap" + var a M = &Impl{} // ERROR "does not escape" v, ok := a.(M) if ok { - v.M() + v.M() // ERROR "devirtualizing" "inlining call" } } { - var a M = &Impl{} // ERROR "&Impl{} escapes to heap" + var a M = &Impl{} // ERROR "does not escape" v, ok := a.(A) if ok { - v.A() + v.A() // ERROR "devirtualizing" "inlining call" } } { @@ -100,14 +99,14 @@ func t2() { } } { - var a M = &Impl{} // ERROR "&Impl{} escapes to heap" + var a M = &Impl{} // ERROR "does not escape" v, _ := a.(M) - v.M() + v.M() // ERROR "devirtualizing" "inlining call" } { - var a M = &Impl{} // ERROR "&Impl{} escapes to heap" + var a M = &Impl{} // ERROR "does not escape" v, _ := a.(A) - v.A() + v.A() // ERROR "devirtualizing" "inlining call" } { var a M = &Impl{} // ERROR "does not escape" @@ -115,6 +114,25 @@ func t2() { v.A() // ERROR "inlining" v.M() // ERROR "inlining" } + { + a := newM() // ERROR "does not escape" "inlining call" + callA(a) // ERROR "devirtualizing" "inlining call" + callIfA(a) // ERROR "devirtualizing" "inlining call" + } +} + +func newM() M { // ERROR "can inline" + return &Impl{} // ERROR "escapes" +} + +func callA(m M) { // ERROR "can inline" "leaking param" + m.(A).A() +} + +func callIfA(m M) { // ERROR "can inline" "leaking param" + if v, ok := m.(A); ok { + v.A() + } } //go:noinline From 753448067184680a245aca14212780413dca14ad Mon Sep 17 00:00:00 2001 From: Mateusz Poliwczak Date: Fri, 14 Feb 2025 19:29:37 +0100 Subject: [PATCH 6/8] remove added newline Change-Id: I9ccc3dfbc0311edb7ce16f28de6ed8e8197ac76a --- src/cmd/compile/internal/ir/expr.go | 1 - 1 file changed, 1 deletion(-) diff --git a/src/cmd/compile/internal/ir/expr.go b/src/cmd/compile/internal/ir/expr.go index 47a2be98507172..7eb7de239cab16 100644 --- a/src/cmd/compile/internal/ir/expr.go +++ b/src/cmd/compile/internal/ir/expr.go @@ -902,7 +902,6 @@ func staticValue(n Node, forDevirt bool) Node { if n1 == nil { return n } - n = n1 } } From 291add48a44e2fe88256ef2958eba01f42df7713 Mon Sep 17 00:00:00 2001 From: Mateusz Poliwczak Date: Fri, 14 Feb 2025 19:35:40 +0100 Subject: [PATCH 7/8] add test case Change-Id: Ic0453367df8032afedd048916c9292eafd75c2f3 --- test/escape_iface_with_devirt_type_assertions.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/escape_iface_with_devirt_type_assertions.go b/test/escape_iface_with_devirt_type_assertions.go index abc72a9641d1ce..bf7cdfc052a178 100644 --- a/test/escape_iface_with_devirt_type_assertions.go +++ b/test/escape_iface_with_devirt_type_assertions.go @@ -119,6 +119,14 @@ func t2() { callA(a) // ERROR "devirtualizing" "inlining call" callIfA(a) // ERROR "devirtualizing" "inlining call" } + + { + var a M = &Impl{} // ERROR "does not escape" + // Note the !ok condition, devirtualizing here is fine. + if v, ok := a.(M); !ok { + v.M() // ERROR "devirtualizing" "inlining call" + } + } } func newM() M { // ERROR "can inline" From f63e6fa6fdd8143137ca8c58fe9b666556fecef9 Mon Sep 17 00:00:00 2001 From: Mateusz Poliwczak Date: Sat, 15 Feb 2025 08:58:26 +0100 Subject: [PATCH 8/8] move StaticType to devirtualize pkg Change-Id: Iadcbf84a666a3e43ce2b460e064b111efa0f2022 --- .../internal/devirtualize/devirtualize.go | 89 ++++++++++++++++++- src/cmd/compile/internal/ir/expr.go | 38 +------- ...scape_iface_with_devirt_type_assertions.go | 47 ++++++++++ 3 files changed, 137 insertions(+), 37 deletions(-) diff --git a/src/cmd/compile/internal/devirtualize/devirtualize.go b/src/cmd/compile/internal/devirtualize/devirtualize.go index 5bfeffa4612f7c..5dd4d65b07be56 100644 --- a/src/cmd/compile/internal/devirtualize/devirtualize.go +++ b/src/cmd/compile/internal/devirtualize/devirtualize.go @@ -40,11 +40,16 @@ func StaticCall(call *ir.CallExpr) { } sel := call.Fun.(*ir.SelectorExpr) - typ := ir.StaticType(sel.X) + typ := staticType(sel.X) if typ == nil { return } + // Don't try to devirtualize calls that we statically know that would have failed at runtime. + // This can happen in such case: any(0).(interface {A()}).A(), this typechecks without + // any errors, but will cause a runtime panic. We statically know that int(0) does not + // implement that interface, thus we skip the devirtualization, as it is not possible + // to make a type assertion from interface{A()} to int (int does not implement interface{A()}). if !typecheck.Implements(typ, sel.X.Type()) { return } @@ -136,3 +141,85 @@ func StaticCall(call *ir.CallExpr) { // Desugar OCALLMETH, if we created one (#57309). typecheck.FixMethodCall(call) } + +func staticType(n ir.Node) *types.Type { + for { + switch n1 := n.(type) { + case *ir.ConvExpr: + if n1.Op() == ir.OCONVNOP || n1.Op() == ir.OCONVIFACE { + n = n1.X + continue + } + case *ir.InlinedCallExpr: + if n1.Op() == ir.OINLCALL { + n = n1.SingleResult() + continue + } + case *ir.ParenExpr: + n = n1.X + continue + case *ir.TypeAssertExpr: + n = n1.X + continue + } + + n1 := staticValue(n) + if n1 == nil { + if n.Type().IsInterface() { + return nil + } + return n.Type() + } + n = n1 + } +} + +func staticValue(nn ir.Node) ir.Node { + if nn.Op() != ir.ONAME { + return nil + } + + n := nn.(*ir.Name).Canonical() + if n.Class != ir.PAUTO { + return nil + } + + defn := n.Defn + if defn == nil { + return nil + } + + var rhs ir.Node +FindRHS: + switch defn.Op() { + case ir.OAS: + defn := defn.(*ir.AssignStmt) + rhs = defn.Y + case ir.OAS2: + defn := defn.(*ir.AssignListStmt) + for i, lhs := range defn.Lhs { + if lhs == n { + rhs = defn.Rhs[i] + break FindRHS + } + } + base.Fatalf("%v missing from LHS of %v", n, defn) + case ir.OAS2DOTTYPE: + defn := defn.(*ir.AssignListStmt) + if defn.Lhs[0] == n { + rhs = defn.Rhs[0] + } + default: + return nil + } + + if rhs == nil { + base.Fatalf("RHS is nil: %v", defn) + } + + if ir.Reassigned(n) { + return nil + } + + return rhs +} diff --git a/src/cmd/compile/internal/ir/expr.go b/src/cmd/compile/internal/ir/expr.go index 7eb7de239cab16..4a2e996569450d 100644 --- a/src/cmd/compile/internal/ir/expr.go +++ b/src/cmd/compile/internal/ir/expr.go @@ -840,18 +840,6 @@ func IsAddressable(n Node) bool { return false } -// StaticType is like StaticValue, but also follows ODOTTYPE and OCONVIFACE. -func StaticType(n Node) *types.Type { - out := staticValue(n, true) - - typ := out.Type() - if typ.IsInterface() { - return nil - } - - return typ -} - // StaticValue analyzes n to find the earliest expression that always // evaluates to the same value as n, which might be from an enclosing // function. @@ -867,11 +855,6 @@ func StaticType(n Node) *types.Type { // calling StaticValue on the "int(y)" expression returns the outer // "g()" expression. func StaticValue(n Node) Node { - return staticValue(n, false) - -} - -func staticValue(n Node, forDevirt bool) Node { for { switch n1 := n.(type) { case *ConvExpr: @@ -879,10 +862,6 @@ func staticValue(n Node, forDevirt bool) Node { n = n1.X continue } - if forDevirt && n1.Op() == OCONVIFACE { - n = n1.X - continue - } case *InlinedCallExpr: if n1.Op() == OINLCALL { n = n1.SingleResult() @@ -891,14 +870,9 @@ func staticValue(n Node, forDevirt bool) Node { case *ParenExpr: n = n1.X continue - case *TypeAssertExpr: - if forDevirt { - n = n1.X - continue - } } - n1 := staticValue1(n, forDevirt) + n1 := staticValue1(n) if n1 == nil { return n } @@ -906,7 +880,7 @@ func staticValue(n Node, forDevirt bool) Node { } } -func staticValue1(nn Node, forDevirt bool) Node { +func staticValue1(nn Node) Node { if nn.Op() != ONAME { return nil } @@ -935,14 +909,6 @@ FindRHS: } } base.Fatalf("%v missing from LHS of %v", n, defn) - case OAS2DOTTYPE: - if !forDevirt { - return nil - } - defn := defn.(*AssignListStmt) - if defn.Lhs[0] == n { - rhs = defn.Rhs[0] - } default: return nil } diff --git a/test/escape_iface_with_devirt_type_assertions.go b/test/escape_iface_with_devirt_type_assertions.go index bf7cdfc052a178..5a3d762aa16205 100644 --- a/test/escape_iface_with_devirt_type_assertions.go +++ b/test/escape_iface_with_devirt_type_assertions.go @@ -143,8 +143,55 @@ func callIfA(m M) { // ERROR "can inline" "leaking param" } } +//go:noinline +func newImplNoInline() *Impl { + return &Impl{} // ERROR "escapes" +} + +func t3() { + { + var a A = newImplNoInline() + if v, ok := a.(M); ok { + v.M() // ERROR "devirtualizing" "inlining call" + } + } + { + m := make(map[*Impl]struct{}) // ERROR "does not escape" + for v := range m { + var v A = v + if v, ok := v.(M); ok { + v.M() // ERROR "devirtualizing" "inlining call" + } + } + } + { + m := make(map[int]*Impl) // ERROR "does not escape" + for _, v := range m { + var v A = v + if v, ok := v.(M); ok { + v.M() // ERROR "devirtualizing" "inlining call" + } + } + } + { + m := make(map[int]*Impl) // ERROR "does not escape" + var v A = m[0] + if v, ok := v.(M); ok { + v.M() // ERROR "devirtualizing" "inlining call" + } + } + { + m := make(chan *Impl) + var v A = <-m + if v, ok := v.(M); ok { + v.M() // ERROR "devirtualizing" "inlining call" + } + } +} + //go:noinline func testInvalidAsserts() { + any(0).(interface{ A() }).A() // ERROR "escapes" { var a M = &Impl{} // ERROR "escapes" a.(C).C() // this will panic