]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: allow multi-field structs to be stored directly in interfaces
authorKeith Randall <khr@golang.org>
Sun, 15 Jun 2025 03:10:50 +0000 (20:10 -0700)
committerKeith Randall <khr@golang.org>
Fri, 14 Nov 2025 20:37:44 +0000 (12:37 -0800)
If the struct is a bunch of 0-sized fields and one pointer field.

Merged revert-of-revert for 4 CLs.

original  revert
681937    695016
693415    694996
693615    695015
694195    694995

Fixes #74092
Update #74888
Update #74908
Update #74935
(updated issues are bugs in the last attempt at this)

Change-Id: I32246d49b8bac3bb080972dc06ab432a5480d560
Reviewed-on: https://go-review.googlesource.com/c/go/+/714421
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
13 files changed:
src/cmd/compile/internal/ssa/_gen/dec.rules
src/cmd/compile/internal/ssa/_gen/generic.rules
src/cmd/compile/internal/ssa/expand_calls.go
src/cmd/compile/internal/ssa/rewrite.go
src/cmd/compile/internal/ssa/rewritedec.go
src/cmd/compile/internal/ssa/rewritegeneric.go
src/cmd/compile/internal/types/type.go
src/internal/abi/type.go
src/reflect/type.go
src/reflect/value.go
test/fixedbugs/issue74888.go [new file with mode: 0644]
test/fixedbugs/issue74908.go [new file with mode: 0644]
test/fixedbugs/issue74935.go [new file with mode: 0644]

index 9f6dc369759f4efd9edb62952037431b025e3265..fce00262114c21beba242648d5573a555d95442f 100644 (file)
 // Helpers for expand calls
 // Some of these are copied from generic.rules
 
-(IMake _typ (StructMake val)) => (IMake _typ val)
-(StructSelect [0] (IData x)) => (IData x)
+(IMake _typ (StructMake ___)) => imakeOfStructMake(v)
+(StructSelect (IData x)) && v.Type.Size() > 0 => (IData x)
+(StructSelect (IData x)) && v.Type.Size() == 0 && v.Type.IsStruct() => (StructMake)
+(StructSelect (IData x)) && v.Type.Size() == 0 && v.Type.IsArray() => (ArrayMake0)
 
 (StructSelect [i] x:(StructMake ___)) => x.Args[i]
 
 // More annoying case: (ArraySelect[0] (StructSelect[0] isAPtr))
 // There, result of the StructSelect is an Array (not a pointer) and
 // the pre-rewrite input to the ArraySelect is a struct, not a pointer.
-(StructSelect [0] x) && x.Type.IsPtrShaped()  => x
+(StructSelect x) && x.Type.IsPtrShaped()  => x
 (ArraySelect [0] x) && x.Type.IsPtrShaped()  => x
 
 // These, too.  Bits is bits.
 
 (Store _ (StructMake ___) _) => rewriteStructStore(v)
 
+(IMake _typ (ArrayMake1 val)) => (IMake _typ val)
 (ArraySelect (ArrayMake1 x)) => x
 (ArraySelect [0] (IData x)) => (IData x)
 
index e09cd31c31189b0630b9b3951246842e6de69922..372e89863d51a972e6bacf1f52f817969ad898d4 100644 (file)
   @x.Block (Load <v.Type> (OffPtr <v.Type.PtrTo()> [t.FieldOff(int(i))] ptr) mem)
 
 // Putting struct{*byte} and similar into direct interfaces.
-(IMake _typ (StructMake val)) => (IMake _typ val)
-(StructSelect [0] (IData x)) => (IData x)
+(IMake _typ (StructMake ___)) => imakeOfStructMake(v)
+(StructSelect (IData x)) && v.Type.Size() > 0 => (IData x)
+(StructSelect (IData x)) && v.Type.Size() == 0 && v.Type.IsStruct() => (StructMake)
+(StructSelect (IData x)) && v.Type.Size() == 0 && v.Type.IsArray() => (ArrayMake0)
 
 // un-SSAable values use mem->mem copies
 (Store {t} dst (Load src mem) mem) && !CanSSA(t) =>
index 8a5b364c2fd99f07ead880540e65dee671214a63..6afce759b2d30741731b53601574afe8ceb1984f 100644 (file)
@@ -423,7 +423,14 @@ func (x *expandState) decomposeAsNecessary(pos src.XPos, b *Block, a, m0 *Value,
                if a.Op == OpIMake {
                        data := a.Args[1]
                        for data.Op == OpStructMake || data.Op == OpArrayMake1 {
-                               data = data.Args[0]
+                               // A struct make might have a few zero-sized fields.
+                               // Use the pointer-y one we know is there.
+                               for _, a := range data.Args {
+                                       if a.Type.Size() > 0 {
+                                               data = a
+                                               break
+                                       }
+                               }
                        }
                        return x.decomposeAsNecessary(pos, b, data, mem, rc.next(data.Type))
                }
index 07308973b15ec10ad34ac3f85f86df34ad487943..af2568ae8926200448f4d04cce697dc26c2162f9 100644 (file)
@@ -2772,3 +2772,17 @@ func panicBoundsCCToAux(p PanicBoundsCC) Aux {
 func isDictArgSym(sym Sym) bool {
        return sym.(*ir.Name).Sym().Name == typecheck.LocalDictName
 }
+
+// When v is (IMake typ (StructMake ...)), convert to
+// (IMake typ arg) where arg is the pointer-y argument to
+// the StructMake (there must be exactly one).
+func imakeOfStructMake(v *Value) *Value {
+       var arg *Value
+       for _, a := range v.Args[1].Args {
+               if a.Type.Size() > 0 {
+                       arg = a
+                       break
+               }
+       }
+       return v.Block.NewValue2(v.Pos, OpIMake, v.Type, v.Args[0], arg)
+}
index 16d02692105b55a8620b495d869a3bc56e42fa60..c45034ead02a4a2788b475259c9b4690722484ab 100644 (file)
@@ -279,11 +279,20 @@ func rewriteValuedec_OpIData(v *Value) bool {
 func rewriteValuedec_OpIMake(v *Value) bool {
        v_1 := v.Args[1]
        v_0 := v.Args[0]
-       // match: (IMake _typ (StructMake val))
+       // match: (IMake _typ (StructMake ___))
+       // result: imakeOfStructMake(v)
+       for {
+               if v_1.Op != OpStructMake {
+                       break
+               }
+               v.copyOf(imakeOfStructMake(v))
+               return true
+       }
+       // match: (IMake _typ (ArrayMake1 val))
        // result: (IMake _typ val)
        for {
                _typ := v_0
-               if v_1.Op != OpStructMake || len(v_1.Args) != 1 {
+               if v_1.Op != OpArrayMake1 {
                        break
                }
                val := v_1.Args[0]
@@ -839,17 +848,47 @@ func rewriteValuedec_OpStructMake(v *Value) bool {
 func rewriteValuedec_OpStructSelect(v *Value) bool {
        v_0 := v.Args[0]
        b := v.Block
-       // match: (StructSelect [0] (IData x))
+       // match: (StructSelect (IData x))
+       // cond: v.Type.Size() > 0
        // result: (IData x)
        for {
-               if auxIntToInt64(v.AuxInt) != 0 || v_0.Op != OpIData {
+               if v_0.Op != OpIData {
                        break
                }
                x := v_0.Args[0]
+               if !(v.Type.Size() > 0) {
+                       break
+               }
                v.reset(OpIData)
                v.AddArg(x)
                return true
        }
+       // match: (StructSelect (IData x))
+       // cond: v.Type.Size() == 0 && v.Type.IsStruct()
+       // result: (StructMake)
+       for {
+               if v_0.Op != OpIData {
+                       break
+               }
+               if !(v.Type.Size() == 0 && v.Type.IsStruct()) {
+                       break
+               }
+               v.reset(OpStructMake)
+               return true
+       }
+       // match: (StructSelect (IData x))
+       // cond: v.Type.Size() == 0 && v.Type.IsArray()
+       // result: (ArrayMake0)
+       for {
+               if v_0.Op != OpIData {
+                       break
+               }
+               if !(v.Type.Size() == 0 && v.Type.IsArray()) {
+                       break
+               }
+               v.reset(OpArrayMake0)
+               return true
+       }
        // match: (StructSelect [i] x:(StructMake ___))
        // result: x.Args[i]
        for {
@@ -861,13 +900,10 @@ func rewriteValuedec_OpStructSelect(v *Value) bool {
                v.copyOf(x.Args[i])
                return true
        }
-       // match: (StructSelect [0] x)
+       // match: (StructSelect x)
        // cond: x.Type.IsPtrShaped()
        // result: x
        for {
-               if auxIntToInt64(v.AuxInt) != 0 {
-                       break
-               }
                x := v_0
                if !(x.Type.IsPtrShaped()) {
                        break
index 1621153b439d569db95bf92d8945d8ff5fc18c77..f0560671ace3dae8dbb7555ed81171c6619dc3c7 100644 (file)
@@ -8985,16 +8985,13 @@ func rewriteValuegeneric_OpFloor(v *Value) bool {
 func rewriteValuegeneric_OpIMake(v *Value) bool {
        v_1 := v.Args[1]
        v_0 := v.Args[0]
-       // match: (IMake _typ (StructMake val))
-       // result: (IMake _typ val)
+       // match: (IMake _typ (StructMake ___))
+       // result: imakeOfStructMake(v)
        for {
-               _typ := v_0
-               if v_1.Op != OpStructMake || len(v_1.Args) != 1 {
+               if v_1.Op != OpStructMake {
                        break
                }
-               val := v_1.Args[0]
-               v.reset(OpIMake)
-               v.AddArg2(_typ, val)
+               v.copyOf(imakeOfStructMake(v))
                return true
        }
        // match: (IMake _typ (ArrayMake1 val))
@@ -32109,17 +32106,47 @@ func rewriteValuegeneric_OpStructSelect(v *Value) bool {
                v0.AddArg2(v1, mem)
                return true
        }
-       // match: (StructSelect [0] (IData x))
+       // match: (StructSelect (IData x))
+       // cond: v.Type.Size() > 0
        // result: (IData x)
        for {
-               if auxIntToInt64(v.AuxInt) != 0 || v_0.Op != OpIData {
+               if v_0.Op != OpIData {
                        break
                }
                x := v_0.Args[0]
+               if !(v.Type.Size() > 0) {
+                       break
+               }
                v.reset(OpIData)
                v.AddArg(x)
                return true
        }
+       // match: (StructSelect (IData x))
+       // cond: v.Type.Size() == 0 && v.Type.IsStruct()
+       // result: (StructMake)
+       for {
+               if v_0.Op != OpIData {
+                       break
+               }
+               if !(v.Type.Size() == 0 && v.Type.IsStruct()) {
+                       break
+               }
+               v.reset(OpStructMake)
+               return true
+       }
+       // match: (StructSelect (IData x))
+       // cond: v.Type.Size() == 0 && v.Type.IsArray()
+       // result: (ArrayMake0)
+       for {
+               if v_0.Op != OpIData {
+                       break
+               }
+               if !(v.Type.Size() == 0 && v.Type.IsArray()) {
+                       break
+               }
+               v.reset(OpArrayMake0)
+               return true
+       }
        return false
 }
 func rewriteValuegeneric_OpSub16(v *Value) bool {
index 8de589bae3533378a862e3713ba87476f9620819..2f5de288c23402cc0ddd85630933e4a23e0d0787 100644 (file)
@@ -1822,26 +1822,7 @@ func IsReflexive(t *Type) bool {
 // Can this type be stored directly in an interface word?
 // Yes, if the representation is a single pointer.
 func IsDirectIface(t *Type) bool {
-       switch t.Kind() {
-       case TPTR:
-               // Pointers to notinheap types must be stored indirectly. See issue 42076.
-               return !t.Elem().NotInHeap()
-       case TCHAN,
-               TMAP,
-               TFUNC,
-               TUNSAFEPTR:
-               return true
-
-       case TARRAY:
-               // Array of 1 direct iface type can be direct.
-               return t.NumElem() == 1 && IsDirectIface(t.Elem())
-
-       case TSTRUCT:
-               // Struct with 1 field of direct iface type can be direct.
-               return t.NumFields() == 1 && IsDirectIface(t.Field(0).Type)
-       }
-
-       return false
+       return t.Size() == int64(PtrSize) && PtrDataSize(t) == int64(PtrSize)
 }
 
 // IsInterfaceMethod reports whether (field) m is
index 7f44a9de568f8238ff047708bea8922b10f979a9..243b787cfcee9497bc38c7d9ef0a3ee1e0862ef7 100644 (file)
@@ -121,8 +121,8 @@ const (
        TFlagGCMaskOnDemand TFlag = 1 << 4
 
        // TFlagDirectIface means that a value of this type is stored directly
-       // in the data field of an interface, instead of indirectly. Normally
-       // this means the type is pointer-ish.
+       // in the data field of an interface, instead of indirectly.
+       // This flag is just a cached computation of Size_ == PtrBytes == goarch.PtrSize.
        TFlagDirectIface TFlag = 1 << 5
 
        // Leaving this breadcrumb behind for dlv. It should not be used, and no
index 9b8726824e6d9cf0f2dc5ac7454b651e1142aecc..914b5443f3500eb9419b9eb72a4fe669187324b7 100644 (file)
@@ -2528,8 +2528,7 @@ func StructOf(fields []StructField) Type {
        }
 
        switch {
-       case len(fs) == 1 && fs[0].Typ.IsDirectIface():
-               // structs of 1 direct iface type can be direct
+       case typ.Size_ == goarch.PtrSize && typ.PtrBytes == goarch.PtrSize:
                typ.TFlag |= abi.TFlagDirectIface
        default:
                typ.TFlag &^= abi.TFlagDirectIface
@@ -2698,8 +2697,7 @@ func ArrayOf(length int, elem Type) Type {
        }
 
        switch {
-       case length == 1 && typ.IsDirectIface():
-               // array of 1 direct iface type can be direct
+       case array.Size_ == goarch.PtrSize && array.PtrBytes == goarch.PtrSize:
                array.TFlag |= abi.TFlagDirectIface
        default:
                array.TFlag &^= abi.TFlagDirectIface
index b5d5aa8bf2e8a38bdc7705581296ad851ef99da9..a82d976c4787352e22c942a82a92a5d0e21e0e63 100644 (file)
@@ -1279,6 +1279,17 @@ func (v Value) Field(i int) Value {
                        fl |= flagStickyRO
                }
        }
+       if fl&flagIndir == 0 && typ.Size() == 0 {
+               // Special case for picking a field out of a direct struct.
+               // A direct struct must have a pointer field and possibly a
+               // bunch of zero-sized fields. We must return the zero-sized
+               // fields indirectly, as only ptr-shaped things can be direct.
+               // See issue 74935.
+               // We use nil instead of v.ptr as it doesn't matter and
+               // we can avoid pinning a possibly now-unused object.
+               return Value{typ, nil, fl | flagIndir}
+       }
+
        // Either flagIndir is set and v.ptr points at struct,
        // or flagIndir is not set and v.ptr is the actual struct data.
        // In the former case, we want v.ptr + offset.
diff --git a/test/fixedbugs/issue74888.go b/test/fixedbugs/issue74888.go
new file mode 100644 (file)
index 0000000..a0083ad
--- /dev/null
@@ -0,0 +1,20 @@
+// compile
+
+// 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 P struct {
+       q struct{}
+       p *int
+}
+
+func f(x any) {
+       h(x.(P))
+}
+
+//go:noinline
+func h(P) {
+}
diff --git a/test/fixedbugs/issue74908.go b/test/fixedbugs/issue74908.go
new file mode 100644 (file)
index 0000000..cb2e951
--- /dev/null
@@ -0,0 +1,24 @@
+// compile
+
+// 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 Type struct {
+       any
+}
+
+type typeObject struct {
+       e struct{}
+       b *byte
+}
+
+func f(b *byte) Type {
+       return Type{
+               typeObject{
+                       b: b,
+               },
+       }
+}
diff --git a/test/fixedbugs/issue74935.go b/test/fixedbugs/issue74935.go
new file mode 100644 (file)
index 0000000..1f6f718
--- /dev/null
@@ -0,0 +1,19 @@
+// 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
+
+import "reflect"
+
+type W struct {
+       E struct{}
+       X *byte
+}
+
+func main() {
+       w := reflect.ValueOf(W{})
+       _ = w.Field(0).Interface()
+}