]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile/internal/walk: use original type for composite literals in addrTemp
authorthepudds <thepudds1460@gmail.com>
Tue, 27 May 2025 17:32:36 +0000 (13:32 -0400)
committerGopher Robot <gobot@golang.org>
Tue, 27 May 2025 20:52:47 +0000 (13:52 -0700)
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 <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
src/cmd/compile/internal/ir/expr.go
src/cmd/compile/internal/walk/order.go
test/fixedbugs/issue73888.go [new file with mode: 0644]
test/fixedbugs/issue73888b.go [new file with mode: 0644]

index cf56515a2c164715900c989286279d13253e4387..8f7df4b458570e24edcb63b30e0c65a261da6b12 100644 (file)
@@ -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) {
index 77322286c77f3eb49dd5724d41c61ded091af27e..8ba8dd96cc41c6241a4bec36e5a931d00cf0babc 100644 (file)
@@ -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 (file)
index 0000000..b3c1ff7
--- /dev/null
@@ -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 (file)
index 0000000..b6e0289
--- /dev/null
@@ -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()
+}