]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: don't include instantiating types in type hash
authorKeith Randall <khr@golang.org>
Wed, 2 Mar 2022 23:08:23 +0000 (15:08 -0800)
committerKeith Randall <khr@golang.org>
Thu, 3 Mar 2022 00:23:02 +0000 (00:23 +0000)
This CL is a bit overkill, but it is pretty safe for 1.18. We'll
want to revisit for 1.19 so we can avoid the hash collisions between
types, e.g. G[int] and G[float64], that will cause some slowdowns
(but not incorrect behavior). Thanks Cherry for the simple idea.

Fixes #51250

Change-Id: I68130e09ba68e7cc35687bc623f63547bc552867
Reviewed-on: https://go-review.googlesource.com/c/go/+/389474
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

src/cmd/compile/internal/reflectdata/reflect.go
src/cmd/compile/internal/types/fmt.go
test/typeparam/issue51250a.dir/a.go [new file with mode: 0644]
test/typeparam/issue51250a.dir/b.go [new file with mode: 0644]
test/typeparam/issue51250a.dir/main.go [new file with mode: 0644]
test/typeparam/issue51250a.go [new file with mode: 0644]

index 0402c2d82c8736e11bfffbeb86b5c6d910279748..ec217be4c35b2a0c31599d71c99266754024f349 100644 (file)
@@ -1428,7 +1428,7 @@ func WriteBasicTypes() {
 
 type typeAndStr struct {
        t       *types.Type
-       short   string // "short" here means NameString
+       short   string // "short" here means TypeSymName
        regular string
 }
 
index 93061d724d18b09486a9e5b35044d8606ad0c440..a42d97cd31c71a1ef46e9839010da476ceeeba9c 100644 (file)
@@ -72,6 +72,7 @@ const (
        fmtDebug
        fmtTypeID
        fmtTypeIDName
+       fmtTypeIDHash
 )
 
 // Sym
@@ -144,10 +145,21 @@ func symfmt(b *bytes.Buffer, s *Sym, verb rune, mode fmtMode) {
        if q := pkgqual(s.Pkg, verb, mode); q != "" {
                b.WriteString(q)
                b.WriteByte('.')
-               if mode == fmtTypeIDName {
+               switch mode {
+               case fmtTypeIDName:
                        // If name is a generic instantiation, it might have local package placeholders
                        // in it. Replace those placeholders with the package name. See issue 49547.
                        name = strings.Replace(name, LocalPkg.Prefix, q, -1)
+               case fmtTypeIDHash:
+                       // If name is a generic instantiation, don't hash the instantiating types.
+                       // This isn't great, but it is safe. If we hash the instantiating types, then
+                       // we need to make sure they have just the package name. At this point, they
+                       // either have "", or the whole package path, and it is hard to reconcile
+                       // the two without depending on -p (which we might do someday).
+                       // See issue 51250.
+                       if i := strings.Index(name, "["); i >= 0 {
+                               name = name[:i]
+                       }
                }
        }
        b.WriteString(name)
@@ -176,7 +188,7 @@ func pkgqual(pkg *Pkg, verb rune, mode fmtMode) string {
                case fmtDebug:
                        return pkg.Name
 
-               case fmtTypeIDName:
+               case fmtTypeIDName, fmtTypeIDHash:
                        // dcommontype, typehash
                        return pkg.Name
 
@@ -334,7 +346,7 @@ func tconv2(b *bytes.Buffer, t *Type, verb rune, mode fmtMode, visited map[*Type
        if t == AnyType || t == ByteType || t == RuneType {
                // in %-T mode collapse predeclared aliases with their originals.
                switch mode {
-               case fmtTypeIDName, fmtTypeID:
+               case fmtTypeIDName, fmtTypeIDHash, fmtTypeID:
                        t = Types[t.Kind()]
                default:
                        sconv2(b, t.Sym(), 'S', mode)
@@ -425,7 +437,7 @@ func tconv2(b *bytes.Buffer, t *Type, verb rune, mode fmtMode, visited map[*Type
        case TPTR:
                b.WriteByte('*')
                switch mode {
-               case fmtTypeID, fmtTypeIDName:
+               case fmtTypeID, fmtTypeIDName, fmtTypeIDHash:
                        if verb == 'S' {
                                tconv2(b, t.Elem(), 'S', mode, visited)
                                return
@@ -487,7 +499,7 @@ func tconv2(b *bytes.Buffer, t *Type, verb rune, mode fmtMode, visited map[*Type
                        case IsExported(f.Sym.Name):
                                sconv2(b, f.Sym, 'S', mode)
                        default:
-                               if mode != fmtTypeIDName {
+                               if mode != fmtTypeIDName && mode != fmtTypeIDHash {
                                        mode = fmtTypeID
                                }
                                sconv2(b, f.Sym, 'v', mode)
@@ -557,7 +569,7 @@ func tconv2(b *bytes.Buffer, t *Type, verb rune, mode fmtMode, visited map[*Type
                        b.WriteByte(byte(open))
                        fieldVerb := 'v'
                        switch mode {
-                       case fmtTypeID, fmtTypeIDName, fmtGo:
+                       case fmtTypeID, fmtTypeIDName, fmtTypeIDHash, fmtGo:
                                // no argument names on function signature, and no "noescape"/"nosplit" tags
                                fieldVerb = 'S'
                        }
@@ -691,7 +703,7 @@ func fldconv(b *bytes.Buffer, f *Field, verb rune, mode fmtMode, visited map[*Ty
                                if name == ".F" {
                                        name = "F" // Hack for toolstash -cmp.
                                }
-                               if !IsExported(name) && mode != fmtTypeIDName {
+                               if !IsExported(name) && mode != fmtTypeIDName && mode != fmtTypeIDHash {
                                        name = sconv(s, 0, mode) // qualify non-exported names (used on structs, not on funarg)
                                }
                        } else {
@@ -759,7 +771,7 @@ func FmtConst(v constant.Value, sharp bool) string {
 
 // TypeHash computes a hash value for type t to use in type switch statements.
 func TypeHash(t *Type) uint32 {
-       p := t.NameString()
+       p := tconv(t, 0, fmtTypeIDHash)
 
        // Using MD5 is overkill, but reduces accidental collisions.
        h := md5.Sum([]byte(p))
diff --git a/test/typeparam/issue51250a.dir/a.go b/test/typeparam/issue51250a.dir/a.go
new file mode 100644 (file)
index 0000000..12dd60a
--- /dev/null
@@ -0,0 +1,9 @@
+// Copyright 2022 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 a
+
+type G[T any] struct {
+       x T
+}
diff --git a/test/typeparam/issue51250a.dir/b.go b/test/typeparam/issue51250a.dir/b.go
new file mode 100644 (file)
index 0000000..114c9f8
--- /dev/null
@@ -0,0 +1,24 @@
+// Copyright 2022 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 b
+
+import "./a"
+
+type T struct { a int }
+
+var I interface{} = a.G[T]{}
+
+//go:noinline
+func F(x interface{}) {
+       switch x.(type) {
+       case a.G[T]:
+       case int:
+               panic("bad")
+       case float64:
+               panic("bad")
+       default:
+               panic("bad")
+       }
+}
diff --git a/test/typeparam/issue51250a.dir/main.go b/test/typeparam/issue51250a.dir/main.go
new file mode 100644 (file)
index 0000000..45288be
--- /dev/null
@@ -0,0 +1,24 @@
+// Copyright 2022 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 (
+       "./a"
+       "./b"
+)
+
+func main() {
+       switch b.I.(type) {
+       case a.G[b.T]:
+       case int:
+               panic("bad")
+       case float64:
+               panic("bad")
+       default:
+               panic("bad")
+       }
+
+       b.F(a.G[b.T]{})
+}
diff --git a/test/typeparam/issue51250a.go b/test/typeparam/issue51250a.go
new file mode 100644 (file)
index 0000000..aefbe67
--- /dev/null
@@ -0,0 +1,7 @@
+// rundir
+
+// Copyright 2022 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 ignored