From 961818e0131aaa7468616a90ce9ebf00111ccdaa Mon Sep 17 00:00:00 2001 From: thepudds Date: Tue, 27 May 2025 13:32:36 -0400 Subject: [PATCH] cmd/compile/internal/walk: use original type for composite literals in addrTemp When creating a new *ir.Name or *ir.LinksymOffsetExpr to represent a composite literal stored in the read-only data section, we should use the original type of the expression that was found via ir.ReassignOracle.StaticValue. (This is needed because the StaticValue method can traverse through OCONVNOP operations to find its final result.) Otherwise, the compilation may succeed, but the linker might erroneously conclude that a type is not used and prune an itab when it should not, leading to a call at execution-time to runtime.unreachableMethod, which throws "fatal error: unreachable method called. linker bug?". The tests exercise both the case of a zero value struct literal that can be represented by the read-only runtime.zeroVal, which was the case of the simplified example from #73888, and also modifies that example to test the non zero value struct literal case. This CL makes two similar changes for those two cases. We can get either of the tests we are adding to fail independently if we only make a single corresponding change. Fixes #73888 Updates #71359 Change-Id: Ifd91f445cc168ab895cc27f7964a6557d5cc32e5 Reviewed-on: https://go-review.googlesource.com/c/go/+/676517 Reviewed-by: Keith Randall Reviewed-by: Keith Randall Auto-Submit: Keith Randall LUCI-TryBot-Result: Go LUCI Reviewed-by: Michael Knyszek --- src/cmd/compile/internal/ir/expr.go | 4 +++ src/cmd/compile/internal/walk/order.go | 4 +-- test/fixedbugs/issue73888.go | 34 ++++++++++++++++++++++++++ test/fixedbugs/issue73888b.go | 34 ++++++++++++++++++++++++++ 4 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 test/fixedbugs/issue73888.go create mode 100644 test/fixedbugs/issue73888b.go diff --git a/src/cmd/compile/internal/ir/expr.go b/src/cmd/compile/internal/ir/expr.go index cf56515a2c..8f7df4b458 100644 --- a/src/cmd/compile/internal/ir/expr.go +++ b/src/cmd/compile/internal/ir/expr.go @@ -853,6 +853,10 @@ func IsAddressable(n Node) bool { // // calling StaticValue on the "int(y)" expression returns the outer // "g()" expression. +// +// NOTE: StaticValue can return a result with a different type than +// n's type because it can traverse through OCONVNOP operations. +// TODO: consider reapplying OCONVNOP operations to the result. See https://go.dev/cl/676517. func StaticValue(n Node) Node { for { switch n1 := n.(type) { diff --git a/src/cmd/compile/internal/walk/order.go b/src/cmd/compile/internal/walk/order.go index 77322286c7..8ba8dd96cc 100644 --- a/src/cmd/compile/internal/walk/order.go +++ b/src/cmd/compile/internal/walk/order.go @@ -249,14 +249,14 @@ func (o *orderState) addrTemp(n ir.Node) ir.Node { if (v.Op() == ir.OSTRUCTLIT || v.Op() == ir.OARRAYLIT) && !base.Ctxt.IsFIPS() { if ir.IsZero(v) && 0 < v.Type().Size() && v.Type().Size() <= abi.ZeroValSize { // This zero value can be represented by the read-only zeroVal. - zeroVal := ir.NewLinksymExpr(v.Pos(), ir.Syms.ZeroVal, v.Type()) + zeroVal := ir.NewLinksymExpr(v.Pos(), ir.Syms.ZeroVal, n.Type()) vstat := typecheck.Expr(zeroVal).(*ir.LinksymOffsetExpr) return vstat } if isStaticCompositeLiteral(v) { // v can be directly represented in the read-only data section. lit := v.(*ir.CompLitExpr) - vstat := readonlystaticname(lit.Type()) + vstat := readonlystaticname(n.Type()) fixedlit(inInitFunction, initKindStatic, lit, vstat, nil) // nil init vstat = typecheck.Expr(vstat).(*ir.Name) return vstat diff --git a/test/fixedbugs/issue73888.go b/test/fixedbugs/issue73888.go new file mode 100644 index 0000000000..b3c1ff768f --- /dev/null +++ b/test/fixedbugs/issue73888.go @@ -0,0 +1,34 @@ +// run + +// 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 main + +type SourceRange struct { + x, y int +} + +func (r *SourceRange) String() string { + return "hello" +} + +type SourceNode interface { + SourceRange() +} + +type testNode SourceRange + +func (tn testNode) SourceRange() { +} + +func main() { + n := testNode(SourceRange{}) // zero value + Errorf(n) +} + +//go:noinline +func Errorf(n SourceNode) { + n.SourceRange() +} diff --git a/test/fixedbugs/issue73888b.go b/test/fixedbugs/issue73888b.go new file mode 100644 index 0000000000..b6e0289cc4 --- /dev/null +++ b/test/fixedbugs/issue73888b.go @@ -0,0 +1,34 @@ +// run + +// 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 main + +type SourceRange struct { + x, y int +} + +func (r *SourceRange) String() string { + return "hello" +} + +type SourceNode interface { + SourceRange() +} + +type testNode SourceRange + +func (tn testNode) SourceRange() { +} + +func main() { + n := testNode(SourceRange{1, 1}) // not zero value + Errorf(n) +} + +//go:noinline +func Errorf(n SourceNode) { + n.SourceRange() +} -- 2.50.0