From 1274d58dacc92204f5bb233b22a93c30a37f34e5 Mon Sep 17 00:00:00 2001 From: Mark Freeman Date: Wed, 3 Dec 2025 15:14:22 -0500 Subject: [PATCH] go/types, types2: add check for finite size at value observance Each type must be representable by a finite amount of Go source code after replacing all alias type names, value names, and embedded interfaces (per #56103) with the RHS from their respective declarations ("expansion"); otherwise the type is invalid. Furthermore, each type must have a finite size. Checking for finite source after expansion is handled in decl.go. Checking for finite size is handled in validtype.go and is delayed to the end of type checking (unless used in unsafe.Sizeof, in which case it is computed eagerly). We can only construct values of valid types. Thus, while a type is pending (on the object path and thus not yet valid), it cannot construct a value of its own type (directly or indirectly). This change enforces the indirect case by validating each type at value observance (and hence upholding the invariant that values of only valid types are created). Validation is cached on Named types to avoid duplicate work. As an example, consider: type A [unsafe.Sizeof(B{})]int type B A Clearly, since there are no aliases, A and B have finite source. At the site of B{}, B will be checked for finite size, recursing down the values of B, at which point A is seen. Since A is on the object path, there is a cycle preventing B from being proven valid before B{}, violating our invariant. Note that this change also works for generic types. Fixes #75918 Fixes #76478 Change-Id: I76d493b5da9571780fed4b3c76803750ec184818 Reviewed-on: https://go-review.googlesource.com/c/go/+/726580 LUCI-TryBot-Result: Go LUCI Reviewed-by: Robert Griesemer Auto-Submit: Mark Freeman --- src/cmd/compile/internal/types2/cycles.go | 40 +++++++++++++++++++ src/cmd/compile/internal/types2/expr.go | 9 ++--- src/cmd/compile/internal/types2/named.go | 3 +- .../compile/internal/types2/sizeof_test.go | 2 +- src/go/types/cycles.go | 40 +++++++++++++++++++ src/go/types/expr.go | 9 ++--- src/go/types/named.go | 3 +- src/go/types/sizeof_test.go | 2 +- .../types/testdata/fixedbugs/issue52915.go | 4 +- .../types/testdata/fixedbugs/issue75918.go | 13 ++++++ .../types/testdata/fixedbugs/issue76478.go | 25 ++++++++++++ 11 files changed, 131 insertions(+), 19 deletions(-) create mode 100644 src/internal/types/testdata/fixedbugs/issue75918.go create mode 100644 src/internal/types/testdata/fixedbugs/issue76478.go diff --git a/src/cmd/compile/internal/types2/cycles.go b/src/cmd/compile/internal/types2/cycles.go index b916219c97..f6721fa593 100644 --- a/src/cmd/compile/internal/types2/cycles.go +++ b/src/cmd/compile/internal/types2/cycles.go @@ -102,3 +102,43 @@ func (check *Checker) directCycle(tname *TypeName, pathIdx map[*TypeName]int) { } } } + +// TODO(markfreeman): Can the value cached on Named be used in validType / hasVarSize? + +// finiteSize returns whether a type has finite size. +func (check *Checker) finiteSize(t Type) (b bool) { + switch t := Unalias(t).(type) { + case *Named: + if t.finite != nil { + return *t.finite + } + + defer func() { + t.finite = &b + }() + + if i, ok := check.objPathIdx[t.obj]; ok { + cycle := check.objPath[i:] + check.cycleError(cycle, firstInSrc(cycle)) + return false + } + check.push(t.obj) + defer check.pop() + + return check.finiteSize(t.fromRHS) + + case *Array: + // The array length is already computed. If it was a valid length, it + // is finite; else, an error was reported in the computation. + return check.finiteSize(t.elem) + + case *Struct: + for _, f := range t.fields { + if !check.finiteSize(f.typ) { + return false + } + } + } + + return true +} diff --git a/src/cmd/compile/internal/types2/expr.go b/src/cmd/compile/internal/types2/expr.go index 637cbaee5d..e3ef1af1ce 100644 --- a/src/cmd/compile/internal/types2/expr.go +++ b/src/cmd/compile/internal/types2/expr.go @@ -1041,12 +1041,9 @@ func (check *Checker) pendingType(x *operand) { if x.mode == invalid || x.mode == novalue { return } - if n, ok := Unalias(x.typ).(*Named); ok { - if i, ok := check.objPathIdx[n.obj]; ok { - check.cycleError(check.objPath, i) - x.mode = invalid - x.typ = Typ[Invalid] - } + if !check.finiteSize(x.typ) { + x.mode = invalid + x.typ = Typ[Invalid] } } diff --git a/src/cmd/compile/internal/types2/named.go b/src/cmd/compile/internal/types2/named.go index edd6357248..6d5ff976c3 100644 --- a/src/cmd/compile/internal/types2/named.go +++ b/src/cmd/compile/internal/types2/named.go @@ -110,7 +110,8 @@ type Named struct { allowNilRHS bool // same as below, as well as briefly during checking of a type declaration allowNilUnderlying bool // may be true from creation via [NewNamed] until [Named.SetUnderlying] - inst *instance // information for instantiated types; nil otherwise + inst *instance // information for instantiated types; nil otherwise + finite *bool // whether the type has finite size, or nil mu sync.Mutex // guards all fields below state_ uint32 // the current state of this type; must only be accessed atomically or when mu is held diff --git a/src/cmd/compile/internal/types2/sizeof_test.go b/src/cmd/compile/internal/types2/sizeof_test.go index f206c12fc3..a3697b666c 100644 --- a/src/cmd/compile/internal/types2/sizeof_test.go +++ b/src/cmd/compile/internal/types2/sizeof_test.go @@ -31,7 +31,7 @@ func TestSizeof(t *testing.T) { {Interface{}, 40, 80}, {Map{}, 16, 32}, {Chan{}, 12, 24}, - {Named{}, 64, 120}, + {Named{}, 68, 128}, {TypeParam{}, 28, 48}, {term{}, 12, 24}, diff --git a/src/go/types/cycles.go b/src/go/types/cycles.go index bd894258b1..ca4d5f7744 100644 --- a/src/go/types/cycles.go +++ b/src/go/types/cycles.go @@ -105,3 +105,43 @@ func (check *Checker) directCycle(tname *TypeName, pathIdx map[*TypeName]int) { } } } + +// TODO(markfreeman): Can the value cached on Named be used in validType / hasVarSize? + +// finiteSize returns whether a type has finite size. +func (check *Checker) finiteSize(t Type) (b bool) { + switch t := Unalias(t).(type) { + case *Named: + if t.finite != nil { + return *t.finite + } + + defer func() { + t.finite = &b + }() + + if i, ok := check.objPathIdx[t.obj]; ok { + cycle := check.objPath[i:] + check.cycleError(cycle, firstInSrc(cycle)) + return false + } + check.push(t.obj) + defer check.pop() + + return check.finiteSize(t.fromRHS) + + case *Array: + // The array length is already computed. If it was a valid length, it + // is finite; else, an error was reported in the computation. + return check.finiteSize(t.elem) + + case *Struct: + for _, f := range t.fields { + if !check.finiteSize(f.typ) { + return false + } + } + } + + return true +} diff --git a/src/go/types/expr.go b/src/go/types/expr.go index 09f7cdda80..6ff5695390 100644 --- a/src/go/types/expr.go +++ b/src/go/types/expr.go @@ -1033,12 +1033,9 @@ func (check *Checker) pendingType(x *operand) { if x.mode == invalid || x.mode == novalue { return } - if n, ok := Unalias(x.typ).(*Named); ok { - if i, ok := check.objPathIdx[n.obj]; ok { - check.cycleError(check.objPath, i) - x.mode = invalid - x.typ = Typ[Invalid] - } + if !check.finiteSize(x.typ) { + x.mode = invalid + x.typ = Typ[Invalid] } } diff --git a/src/go/types/named.go b/src/go/types/named.go index be6a0f5426..ba0f92015d 100644 --- a/src/go/types/named.go +++ b/src/go/types/named.go @@ -113,7 +113,8 @@ type Named struct { allowNilRHS bool // same as below, as well as briefly during checking of a type declaration allowNilUnderlying bool // may be true from creation via [NewNamed] until [Named.SetUnderlying] - inst *instance // information for instantiated types; nil otherwise + inst *instance // information for instantiated types; nil otherwise + finite *bool // whether the type has finite size, or nil mu sync.Mutex // guards all fields below state_ uint32 // the current state of this type; must only be accessed atomically or when mu is held diff --git a/src/go/types/sizeof_test.go b/src/go/types/sizeof_test.go index 694ab32462..2f18595517 100644 --- a/src/go/types/sizeof_test.go +++ b/src/go/types/sizeof_test.go @@ -30,7 +30,7 @@ func TestSizeof(t *testing.T) { {Interface{}, 40, 80}, {Map{}, 16, 32}, {Chan{}, 12, 24}, - {Named{}, 64, 120}, + {Named{}, 68, 128}, {TypeParam{}, 28, 48}, {term{}, 12, 24}, diff --git a/src/internal/types/testdata/fixedbugs/issue52915.go b/src/internal/types/testdata/fixedbugs/issue52915.go index 70dc664375..e60c1767e9 100644 --- a/src/internal/types/testdata/fixedbugs/issue52915.go +++ b/src/internal/types/testdata/fixedbugs/issue52915.go @@ -18,6 +18,4 @@ func _[P any]() { _ = unsafe.Sizeof(struct{ T[P] }{}) } -// TODO(gri) This is a follow-on error due to T[int] being invalid. -// We should try to avoid it. -const _ = unsafe /* ERROR "not constant" */ .Sizeof(T[int]{}) +const _ = unsafe.Sizeof(T /* ERROR "invalid recursive type" */ [int]{}) diff --git a/src/internal/types/testdata/fixedbugs/issue75918.go b/src/internal/types/testdata/fixedbugs/issue75918.go new file mode 100644 index 0000000000..373db4a21b --- /dev/null +++ b/src/internal/types/testdata/fixedbugs/issue75918.go @@ -0,0 +1,13 @@ +// 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 p + +import "unsafe" + +type A /* ERROR "invalid recursive type" */ [unsafe.Sizeof(S{})]byte + +type S struct { + a A +} diff --git a/src/internal/types/testdata/fixedbugs/issue76478.go b/src/internal/types/testdata/fixedbugs/issue76478.go new file mode 100644 index 0000000000..f16b40e04e --- /dev/null +++ b/src/internal/types/testdata/fixedbugs/issue76478.go @@ -0,0 +1,25 @@ +// 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 p + +import "unsafe" + +type A /* ERROR "invalid recursive type" */ [unsafe.Sizeof(B{})]int +type B A + +type C /* ERROR "invalid recursive type" */ [unsafe.Sizeof(f())]int +func f() D { + return D{} +} +type D C + +type E /* ERROR "invalid recursive type" */ [unsafe.Sizeof(g[F]())]int +func g[P any]() P { + panic(0) +} +type F struct { + f E +} + -- 2.52.0