From e6ef7709393d2d16834adea8451b9c164d8ee4ed Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Wed, 23 Feb 2022 15:37:02 -0500 Subject: [PATCH] go/types/typeutil: don't recurse into constraints when hashing tparams When hashing signature type parameters, we recursed into their constraints in an attempt to improve the accuracy of the hash (signatures are considered identical modulo type parameter renaming). This introduces a mechanism for infinite recursion via recursive constraints, as reported in golang/go#51314. To fix this, we can hash just the type parameter index and rely on types.Identical for de-duplicating map entries. Fixes golang/go#51314 Change-Id: Ifd7596d978b98293391bbe64f72f1da2b2a5d51c Reviewed-on: https://go-review.googlesource.com/c/tools/+/387754 Trust: Robert Findley Run-TryBot: Robert Findley Reviewed-by: Tim King gopls-CI: kokoro TryBot-Result: Gopher Robot --- go/types/typeutil/map.go | 10 ++++++---- go/types/typeutil/map_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/go/types/typeutil/map.go b/go/types/typeutil/map.go index 490ee904a6..c9f8f25a0d 100644 --- a/go/types/typeutil/map.go +++ b/go/types/typeutil/map.go @@ -379,7 +379,7 @@ func (h Hasher) hashFor(t types.Type) uint32 { func (h Hasher) hashTuple(tuple *types.Tuple) uint32 { // See go/types.identicalTypes for rationale. n := tuple.Len() - var hash uint32 = 9137 + 2*uint32(n) + hash := 9137 + 2*uint32(n) for i := 0; i < n; i++ { hash += 3 * h.Hash(tuple.At(i).Type()) } @@ -398,7 +398,7 @@ func (h Hasher) hashUnion(t *typeparams.Union) uint32 { } func (h Hasher) hashTermSet(terms []*typeparams.Term) uint32 { - var hash uint32 = 9157 + 2*uint32(len(terms)) + hash := 9157 + 2*uint32(len(terms)) for _, term := range terms { // term order is not significant. termHash := h.Hash(term.Type()) @@ -416,14 +416,16 @@ func (h Hasher) hashTermSet(terms []*typeparams.Term) uint32 { // If h.sigTParams is set and contains t, then we are in the process of hashing // a signature, and the hash value of t must depend only on t's index and // constraint: signatures are considered identical modulo type parameter -// renaming. +// renaming. To avoid infinite recursion, we only hash the type parameter +// index, and rely on types.Identical to handle signatures where constraints +// are not identical. // // Otherwise the hash of t depends only on t's pointer identity. func (h Hasher) hashTypeParam(t *typeparams.TypeParam) uint32 { if h.sigTParams != nil { i := t.Index() if i >= 0 && i < h.sigTParams.Len() && t == h.sigTParams.At(i) { - return 9173 + 2*h.Hash(t.Constraint()) + 3*uint32(i) + return 9173 + 3*uint32(i) } } return h.hashPtr(t.Obj()) diff --git a/go/types/typeutil/map_test.go b/go/types/typeutil/map_test.go index 17f87edfd5..8cd643e5b4 100644 --- a/go/types/typeutil/map_test.go +++ b/go/types/typeutil/map_test.go @@ -233,6 +233,17 @@ var ME2 = G2[int].M // ME1Type should have identical type as ME1. var ME1Type func(G1[int], G1[int], G2[int]) + +// Examples from issue #51314 +type Constraint[T any] interface{} +func Foo[T Constraint[T]]() {} +func Fn[T1 ~*T2, T2 ~*T1](t1 T1, t2 T2) {} + +// Bar and Baz are identical to Foo. +func Bar[P Constraint[P]]() {} +func Baz[Q any]() {} // The underlying type of Constraint[P] is any. +// But Quux is not. +func Quux[Q interface{ quux() }]() {} ` fset := token.NewFileSet() @@ -284,6 +295,13 @@ var ME1Type func(G1[int], G1[int], G2[int]) ME1 = scope.Lookup("ME1").Type() ME1Type = scope.Lookup("ME1Type").Type() ME2 = scope.Lookup("ME2").Type() + + Constraint = scope.Lookup("Constraint").Type() + Foo = scope.Lookup("Foo").Type() + Fn = scope.Lookup("Fn").Type() + Bar = scope.Lookup("Foo").Type() + Baz = scope.Lookup("Foo").Type() + Quux = scope.Lookup("Quux").Type() ) tmap := new(typeutil.Map) @@ -345,6 +363,14 @@ var ME1Type func(G1[int], G1[int], G2[int]) {ME1, "ME1", true}, {ME1Type, "ME1Type", false}, {ME2, "ME2", true}, + + // See golang/go#51314: avoid infinite recursion on cyclic type constraints. + {Constraint, "Constraint", true}, + {Foo, "Foo", true}, + {Fn, "Fn", true}, + {Bar, "Bar", false}, + {Baz, "Baz", false}, + {Quux, "Quux", true}, } for _, step := range steps {