]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: fix aeshash of empty string
authorKeith Randall <khr@golang.org>
Tue, 1 Sep 2015 19:53:15 +0000 (12:53 -0700)
committerKeith Randall <khr@golang.org>
Tue, 15 Sep 2015 17:51:23 +0000 (17:51 +0000)
Aeshash currently computes the hash of the empty string as
hash("", seed) = seed.  This is bad because the hash of a compound
object with empty strings in it doesn't include information about
where those empty strings were.  For instance [2]string{"", "foo"}
and [2]string{"foo", ""} might get the same hash.

Fix this by returning a scrambled seed instead of the seed itself.
With this fix, we can remove the scrambling done by the generated
array hash routines.

The test also rejects hash("", seed) = 0, if we ever thought
it would be a good idea to try that.

The fallback hash is already OK in this regard.

Change-Id: Iaedbaa5be8d6a246dc7e9383d795000e0f562037
Reviewed-on: https://go-review.googlesource.com/14129
Reviewed-by: jcd . <jcd@golang.org>
src/cmd/compile/internal/gc/subr.go
src/runtime/asm_386.s
src/runtime/asm_amd64.s
src/runtime/hash_test.go

index 0a4a1112cb20a985e885982dbe1d6b90cef85378..be93d2783c399412cc9c9466c80ea0652571ad1a 100644 (file)
@@ -2611,21 +2611,6 @@ func genhash(sym *Sym, t *Type) {
                colasdefn(n.List, n)
                ni = n.List.N
 
-               // TODO: with aeshash we don't need these shift/mul parts
-
-               // h = h<<3 | h>>61
-               n.Nbody = list(n.Nbody, Nod(OAS, nh, Nod(OOR, Nod(OLSH, nh, Nodintconst(3)), Nod(ORSH, nh, Nodintconst(int64(Widthptr)*8-3)))))
-
-               // h *= mul
-               // Same multipliers as in runtime.memhash.
-               var mul int64
-               if Widthptr == 4 {
-                       mul = 3267000013
-               } else {
-                       mul = 23344194077549503
-               }
-               n.Nbody = list(n.Nbody, Nod(OAS, nh, Nod(OMUL, nh, Nodintconst(mul))))
-
                // h = hashel(&p[i], h)
                call := Nod(OCALL, hashel, nil)
 
index 2bc5d8b658e9b6d33706398cfe10ec48e41219ce..fbce0153db12262837a893cae0ca651db3dd80fe 100644 (file)
@@ -1012,9 +1012,10 @@ endofpage:
        RET
 
 aes0:
-       // return input seed
-       MOVL    h+4(FP), AX
-       MOVL    AX, (DX)
+       // Return scrambled input seed
+       AESENC  X7, X6
+       AESENC  X7, X6
+       MOVL    X6, (DX)
        RET
 
 aes16:
index dc975bebc2b8a130a8daa7846f88d9f43fb42c3a..4020bdfbfc6977841c05188d53055b6d7899e0d3 100644 (file)
@@ -1003,9 +1003,10 @@ endofpage:
        RET
 
 aes0:
-       // return input seed
-       MOVQ    h+8(FP), AX
-       MOVQ    AX, (DX)
+       // Return scrambled input seed
+       AESENC  X7, X6
+       AESENC  X7, X6
+       MOVQ    X6, (DX)
        RET
 
 aes16:
index 579b0e3e67a5c349369819dde1d4161262d05160..165148556d1f7d95355b3f66b09fedd4650a799f 100644 (file)
@@ -561,3 +561,100 @@ func BenchmarkHash16(b *testing.B)    { benchmarkHash(b, 16) }
 func BenchmarkHash64(b *testing.B)    { benchmarkHash(b, 64) }
 func BenchmarkHash1024(b *testing.B)  { benchmarkHash(b, 1024) }
 func BenchmarkHash65536(b *testing.B) { benchmarkHash(b, 65536) }
+
+func TestArrayHash(t *testing.T) {
+       // Make sure that "" in arrays hash correctly.  The hash
+       // should at least scramble the input seed so that, e.g.,
+       // {"","foo"} and {"foo",""} have different hashes.
+
+       // If the hash is bad, then all (8 choose 4) = 70 keys
+       // have the same hash. If so, we allocate 70/8 = 8
+       // overflow buckets.  If the hash is good we don't
+       // normally allocate any overflow buckets, and the
+       // probability of even one or two overflows goes down rapidly.
+       // (There is always 1 allocation of the bucket array.  The map
+       // header is allocated on the stack.)
+       f := func() {
+               // Make the key type at most 128 bytes.  Otherwise,
+               // we get an allocation per key.
+               type key [8]string
+               m := make(map[key]bool, 70)
+
+               // fill m with keys that have 4 "foo"s and 4 ""s.
+               for i := 0; i < 256; i++ {
+                       var k key
+                       cnt := 0
+                       for j := uint(0); j < 8; j++ {
+                               if i>>j&1 != 0 {
+                                       k[j] = "foo"
+                                       cnt++
+                               }
+                       }
+                       if cnt == 4 {
+                               m[k] = true
+                       }
+               }
+               if len(m) != 70 {
+                       t.Errorf("bad test: (8 choose 4) should be 70, not %d", len(m))
+               }
+       }
+       if n := testing.AllocsPerRun(10, f); n > 6 {
+               t.Errorf("too many allocs %f - hash not balanced", n)
+       }
+}
+func TestStructHash(t *testing.T) {
+       // See the comment in TestArrayHash.
+       f := func() {
+               type key struct {
+                       a, b, c, d, e, f, g, h string
+               }
+               m := make(map[key]bool, 70)
+
+               // fill m with keys that have 4 "foo"s and 4 ""s.
+               for i := 0; i < 256; i++ {
+                       var k key
+                       cnt := 0
+                       if i&1 != 0 {
+                               k.a = "foo"
+                               cnt++
+                       }
+                       if i&2 != 0 {
+                               k.b = "foo"
+                               cnt++
+                       }
+                       if i&4 != 0 {
+                               k.c = "foo"
+                               cnt++
+                       }
+                       if i&8 != 0 {
+                               k.d = "foo"
+                               cnt++
+                       }
+                       if i&16 != 0 {
+                               k.e = "foo"
+                               cnt++
+                       }
+                       if i&32 != 0 {
+                               k.f = "foo"
+                               cnt++
+                       }
+                       if i&64 != 0 {
+                               k.g = "foo"
+                               cnt++
+                       }
+                       if i&128 != 0 {
+                               k.h = "foo"
+                               cnt++
+                       }
+                       if cnt == 4 {
+                               m[k] = true
+                       }
+               }
+               if len(m) != 70 {
+                       t.Errorf("bad test: (8 choose 4) should be 70, not %d", len(m))
+               }
+       }
+       if n := testing.AllocsPerRun(10, f); n > 6 {
+               t.Errorf("too many allocs %f - hash not balanced", n)
+       }
+}