]> Cypherpunks repositories - gostls13.git/commitdiff
go/types: use Identical to verify type identity in the Context map
authorRobert Findley <rfindley@google.com>
Wed, 10 Nov 2021 02:39:53 +0000 (21:39 -0500)
committerRobert Findley <rfindley@google.com>
Fri, 12 Nov 2021 18:10:48 +0000 (18:10 +0000)
We don't have guarantees that our type hash is perfect, and in fact
fuzzing found cases where identical types hashed to different values. In
case non-identical types hash to the same value, we should ensure that
we de-duplicate using Identical.

Adjust the type map to keep a slice of distinct type identities, so that
we can guarantee that type identity is preserved by de-duplication.

To allow look-up of instances by their identity, before they are
actually instantiated, add a Context.lookup method that accepts origin
type and type arguments. Replace the multi-function typeForHash method
with an update method that requires its argument be non-nil.

Change-Id: I8fe6fb2955f508db608161b7285b02d0a2fa0e46
Reviewed-on: https://go-review.googlesource.com/c/go/+/362798
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
src/go/types/context.go
src/go/types/instantiate.go
src/go/types/named.go
src/go/types/predicates.go
src/go/types/subst.go
src/go/types/typexpr.go

index 7caf631b57ecaadedafd871eaf129a307468ac7b..e89babcd705f25816e49674dc98452f171131b76 100644 (file)
@@ -6,6 +6,7 @@ package types
 
 import (
        "bytes"
+       "fmt"
        "strings"
        "sync"
 )
@@ -17,15 +18,15 @@ import (
 // It is safe for concurrent use.
 type Context struct {
        mu      sync.Mutex
-       typeMap map[string]*Named // type hash -> instance
-       nextID  int               // next unique ID
-       seen    map[*Named]int    // assigned unique IDs
+       typeMap map[string][]*Named // type hash -> instances
+       nextID  int                 // next unique ID
+       seen    map[*Named]int      // assigned unique IDs
 }
 
 // NewContext creates a new Context.
 func NewContext() *Context {
        return &Context{
-               typeMap: make(map[string]*Named),
+               typeMap: make(map[string][]*Named),
                seen:    make(map[*Named]int),
        }
 }
@@ -57,17 +58,46 @@ func (ctxt *Context) typeHash(typ Type, targs []Type) string {
        return strings.Replace(buf.String(), " ", "#", -1) // ReplaceAll is not available in Go1.4
 }
 
-// typeForHash returns the recorded type for the type hash h, if it exists.
-// If no type exists for h and n is non-nil, n is recorded for h.
-func (ctxt *Context) typeForHash(h string, n *Named) *Named {
+// lookup returns an existing instantiation of orig with targs, if it exists.
+// Otherwise, it returns nil.
+func (ctxt *Context) lookup(h string, orig *Named, targs []Type) *Named {
        ctxt.mu.Lock()
        defer ctxt.mu.Unlock()
-       if existing := ctxt.typeMap[h]; existing != nil {
-               return existing
+
+       for _, e := range ctxt.typeMap[h] {
+               if identicalInstance(orig, targs, e.orig, e.TypeArgs().list()) {
+                       return e
+               }
+               if debug {
+                       // Panic during development to surface any imperfections in our hash.
+                       panic(fmt.Sprintf("non-identical instances: (orig: %s, targs: %v) and %s", orig, targs, e))
+               }
        }
-       if n != nil {
-               ctxt.typeMap[h] = n
+
+       return nil
+}
+
+// update de-duplicates n against previously seen types with the hash h.  If an
+// identical type is found with the type hash h, the previously seen type is
+// returned. Otherwise, n is returned, and recorded in the Context for the hash
+// h.
+func (ctxt *Context) update(h string, n *Named) *Named {
+       assert(n != nil)
+
+       ctxt.mu.Lock()
+       defer ctxt.mu.Unlock()
+
+       for _, e := range ctxt.typeMap[h] {
+               if n == nil || Identical(n, e) {
+                       return e
+               }
+               if debug {
+                       // Panic during development to surface any imperfections in our hash.
+                       panic(fmt.Sprintf("%s and %s are not identical", n, e))
+               }
        }
+
+       ctxt.typeMap[h] = append(ctxt.typeMap[h], n)
        return n
 }
 
index 8d8d2818429ecf322b4b441f208d049227445ffb..1077ad81604f479e48580aed78d987fc2054db2b 100644 (file)
@@ -60,7 +60,7 @@ func (check *Checker) instance(pos token.Pos, typ Type, targs []Type, ctxt *Cont
                        h = ctxt.typeHash(t, targs)
                        // typ may already have been instantiated with identical type arguments. In
                        // that case, re-use the existing instance.
-                       if named := ctxt.typeForHash(h, nil); named != nil {
+                       if named := ctxt.lookup(h, t, targs); named != nil {
                                return named
                        }
                }
@@ -73,7 +73,7 @@ func (check *Checker) instance(pos token.Pos, typ Type, targs []Type, ctxt *Cont
                if ctxt != nil {
                        // It's possible that we've lost a race to add named to the context.
                        // In this case, use whichever instance is recorded in the context.
-                       named = ctxt.typeForHash(h, named)
+                       named = ctxt.update(h, named)
                }
                return named
 
index 393d40b127cb27a7dcb45e748978105913969876..12d87af084208aa48bd9aed17992c68c80c7b1b6 100644 (file)
@@ -255,7 +255,7 @@ func expandNamed(ctxt *Context, n *Named, instPos token.Pos) (tparams *TypeParam
                ctxt = check.bestContext(ctxt)
                h := ctxt.typeHash(n.orig, n.targs.list())
                // ensure that an instance is recorded for h to avoid infinite recursion.
-               ctxt.typeForHash(h, n)
+               ctxt.update(h, n)
 
                smap := makeSubstMap(n.orig.tparams.list(), n.targs.list())
                underlying = n.check.subst(instPos, n.orig.underlying, smap, ctxt)
index e8689a12cc5c8ebe91b4de9138a0f02a27111a1b..e7f9d3b1db150c9cd8a8e03c14f669437a233c50 100644 (file)
@@ -375,6 +375,23 @@ func identical(x, y Type, cmpTags bool, p *ifacePair) bool {
        return false
 }
 
+// identicalInstance reports if two type instantiations are identical.
+// Instantiations are identical if their origin and type arguments are
+// identical.
+func identicalInstance(xorig Type, xargs []Type, yorig Type, yargs []Type) bool {
+       if len(xargs) != len(yargs) {
+               return false
+       }
+
+       for i, xa := range xargs {
+               if !Identical(xa, yargs[i]) {
+                       return false
+               }
+       }
+
+       return Identical(xorig, yorig)
+}
+
 func identicalTParams(x, y []*TypeParam, cmpTags bool, p *ifacePair) bool {
        if len(x) != len(y) {
                return false
index a05195150f6b7566bb5922ea0b24ebea6b0bcd76..0e3eafdaf18d59e3666d30cd630048f193a83f43 100644 (file)
@@ -209,7 +209,7 @@ func (subst *subster) typ(typ Type) Type {
                // before creating a new named type, check if we have this one already
                h := subst.ctxt.typeHash(t.orig, newTArgs)
                dump(">>> new type hash: %s", h)
-               if named := subst.ctxt.typeForHash(h, nil); named != nil {
+               if named := subst.ctxt.lookup(h, t.orig, newTArgs); named != nil {
                        dump(">>> found %s", named)
                        return named
                }
index 12e0f968c21d26867e05c312c44fb5efb165fdc5..17d07649efe89feea8645489d370dbbc87b5fd9e 100644 (file)
@@ -390,8 +390,8 @@ func (check *Checker) instantiatedType(x ast.Expr, targsx []ast.Expr, def *Named
                return gtyp // error already reported
        }
 
-       origin, _ := gtyp.(*Named)
-       if origin == nil {
+       orig, _ := gtyp.(*Named)
+       if orig == nil {
                panic(fmt.Sprintf("%v: cannot instantiate %v", x.Pos(), gtyp))
        }
 
@@ -409,23 +409,23 @@ func (check *Checker) instantiatedType(x ast.Expr, targsx []ast.Expr, def *Named
        }
 
        // create the instance
-       h := check.conf.Context.typeHash(origin, targs)
+       h := check.conf.Context.typeHash(orig, targs)
        // targs may be incomplete, and require inference. In any case we should de-duplicate.
-       inst := check.conf.Context.typeForHash(h, nil)
+       inst := check.conf.Context.lookup(h, orig, targs)
        // If inst is non-nil, we can't just return here. Inst may have been
        // constructed via recursive substitution, in which case we wouldn't do the
        // validation below. Ensure that the validation (and resulting errors) runs
        // for each instantiated type in the source.
        if inst == nil {
-               tname := NewTypeName(x.Pos(), origin.obj.pkg, origin.obj.name, nil)
-               inst = check.newNamed(tname, origin, nil, nil, nil) // underlying, methods and tparams are set when named is resolved
+               tname := NewTypeName(x.Pos(), orig.obj.pkg, orig.obj.name, nil)
+               inst = check.newNamed(tname, orig, nil, nil, nil) // underlying, methods and tparams are set when named is resolved
                inst.targs = NewTypeList(targs)
-               inst = check.conf.Context.typeForHash(h, inst)
+               inst = check.conf.Context.update(h, inst)
        }
        def.setUnderlying(inst)
 
        inst.resolver = func(ctxt *Context, n *Named) (*TypeParamList, Type, []*Func) {
-               tparams := origin.TypeParams().list()
+               tparams := orig.TypeParams().list()
 
                inferred := targs
                if len(targs) < len(tparams) {