cmd/gc: don't hash nor compare struct padding or blank fields.

Fixes #4585.

R=rsc, golang-dev
CC=golang-dev
https://golang.org/cl/7142052

diff --git a/src/cmd/gc/subr.c b/src/cmd/gc/subr.c
index 674c49bc03af1378de849371d474b134eff32161..c032ffae051483578c5f4413562afc7f667c048d 100644 (file)
--- a/src/cmd/gc/subr.c
+++ b/src/cmd/gc/subr.c
@@ -504,6 +504,14 @@ nod(int op, Node *nleft, Node *nright)
        return n;
 }
 
+static int
+ispaddedfield(Type *t)
+{
+       if(t->etype != TFIELD)
+               fatal("ispaddedfield called non-field %T", t);
+       return t->down != T && t->width + t->type->width != t->down->width;
+}
+
 int
 algtype1(Type *t, Type **bad)
 {
@@ -581,8 +589,12 @@ algtype1(Type *t, Type **bad)
                }
                ret = AMEM;
                for(t1=t->type; t1!=T; t1=t1->down) {
-                       if(isblanksym(t1->sym))
+                       // Blank fields and padding must be ignored,
+                       // so need special compare.
+                       if(isblanksym(t1->sym) || ispaddedfield(t1)) {
+                               ret = -1;
                                continue;
+                       }
                        a = algtype1(t1->type, bad);
                        if(a == ANOEQ)
                                return ANOEQ;  // not comparable
@@ -2694,14 +2706,16 @@ genhash(Sym *sym, Type *t)
                // and calling specific hash functions for the others.
                first = T;
                for(t1=t->type;; t1=t1->down) {
-                       if(t1 != T && (isblanksym(t1->sym) || algtype1(t1->type, nil) == AMEM)) {
-                               if(first == T && !isblanksym(t1->sym))
+                       if(t1 != T && algtype1(t1->type, nil) == AMEM && !isblanksym(t1->sym)) {
+                               if(first == T)
                                        first = t1;
-                               continue;
+                               // If it's a memory field but it's padded, stop here.
+                               if(ispaddedfield(t1))
+                                       t1 = t1->down;
+                               else
+                                       continue;
                        }
                        // Run memhash for fields up to this one.
-                       while(first != T && isblanksym(first->sym))
-                               first = first->down;
                        if(first != T) {
                                if(first->down == t1)
                                        size = first->type->width;
@@ -2724,6 +2738,8 @@ genhash(Sym *sym, Type *t)
                        }
                        if(t1 == T)
                                break;
+                       if(isblanksym(t1->sym))
+                               continue;
 
                        // Run hash for this field.
                        hashel = hashfor(t1->type);
@@ -2737,6 +2753,8 @@ genhash(Sym *sym, Type *t)
                        call->list = list(call->list, na);
                        fn->nbody = list(fn->nbody, call);
                }
+               // make sure body is not empty.
+               fn->nbody = list(fn->nbody, nod(ORETURN, N, N));
                break;
        }
 
@@ -2909,18 +2927,21 @@ geneq(Sym *sym, Type *t)
        case TSTRUCT:
                // Walk the struct using memequal for runs of AMEM
                // and calling specific equality tests for the others.
+               // Skip blank-named fields.
                first = T;
                for(t1=t->type;; t1=t1->down) {
-                       if(t1 != T && (isblanksym(t1->sym) || algtype1(t1->type, nil) == AMEM)) {
-                               if(first == T && !isblanksym(t1->sym))
+                       if(t1 != T && algtype1(t1->type, nil) == AMEM && !isblanksym(t1->sym)) {
+                               if(first == T)
                                        first = t1;
-                               continue;
+                               // If it's a memory field but it's padded, stop here.
+                               if(ispaddedfield(t1))
+                                       t1 = t1->down;
+                               else
+                                       continue;
                        }
                        // Run memequal for fields up to this one.
                        // TODO(rsc): All the calls to newname are wrong for
                        // cross-package unexported fields.
-                       while(first != T && isblanksym(first->sym))
-                               first = first->down;
                        if(first != T) {
                                if(first->down == t1) {
                                        fn->nbody = list(fn->nbody, eqfield(np, nq, newname(first->sym), neq));
@@ -2941,6 +2962,8 @@ geneq(Sym *sym, Type *t)
                        }
                        if(t1 == T)
                                break;
+                       if(isblanksym(t1->sym))
+                               continue;
 
                        // Check this field, which is not just memory.
                        fn->nbody = list(fn->nbody, eqfield(np, nq, newname(t1->sym), neq));

diff --git a/src/cmd/gc/walk.c b/src/cmd/gc/walk.c
index 3a2152b0924a9a18cb78ed466a0177397a542035..3bcbb9cd74cac4e0cd7aa906a4bbcb0999b48184 100644 (file)
--- a/src/cmd/gc/walk.c
+++ b/src/cmd/gc/walk.c
@@ -2746,6 +2746,8 @@ walkcompare(Node **np, NodeList **init)
                // Struct of four or fewer fields.
                // Inline comparisons.
                for(t1=t->type; t1; t1=t1->down) {
+                       if(isblanksym(t1->sym))
+                               continue;
                        li = nod(OXDOT, l, newname(t1->sym));
                        ri = nod(OXDOT, r, newname(t1->sym));
                        a = nod(n->op, li, ri);

diff --git a/test/blank.go b/test/blank.go
index ee618b148585675d8c52ae1652c2af63f73d767c..ad4d6ebdc6fb33527d4370922a20fa24286a017b 100644 (file)
--- a/test/blank.go
+++ b/test/blank.go
@@ -8,6 +8,8 @@
 
 package main
 
+import "unsafe"
+
 import _ "fmt"
 
 var call string
@@ -102,8 +104,15 @@ func main() {
                panic(sum)
        }
 
+       type T1 struct{ x, y, z int }
+       t1 := *(*T)(unsafe.Pointer(&T1{1, 2, 3}))
+       t2 := *(*T)(unsafe.Pointer(&T1{4, 5, 6}))
+       if t1 != t2 {
+               panic("T{} != T{}")
+       }
+
        h(a, b)
-       
+
        m()
 }
 
@@ -133,14 +142,13 @@ func fp1(x, y int) {
        }
 }
 
-
 func m() {
        var i I
-       
+
        i = TI{}
        i.M(1, 1)
        i.M(2, 2)
-       
+
        fp(1, 1)
        fp(2, 2)
 }
@@ -162,4 +170,3 @@ func _() {
 func ff() {
        var _ int = 1
 }
-

diff --git a/test/fixedbugs/issue4585.go b/test/fixedbugs/issue4585.go
new file mode 100644 (file)
index 0000000..558bd1e
--- /dev/null
+++ b/test/fixedbugs/issue4585.go
@@ -0,0 +1,91 @@
+// run
+
+// Copyright 2013 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.
+
+// Issue 4585: comparisons and hashes process blank
+// fields and padding in structs.
+
+package main
+
+import "unsafe"
+
+// T is a structure with padding.
+type T struct {
+       A     int16
+       B     int64
+       C     int16
+       D     int64
+       Dummy [64]byte
+}
+
+// U is a structure with a blank field
+type U struct {
+       A, _, B int
+       Dummy   [64]byte
+}
+
+// USmall is like U but the frontend will inline comparison
+// instead of calling the generated eq function.
+type USmall struct {
+       A, _, B int32
+}
+
+func test1() {
+       var a, b U
+       m := make(map[U]int)
+       copy((*[16]byte)(unsafe.Pointer(&a))[:], "hello world!")
+       a.A, a.B = 1, 2
+       b.A, b.B = 1, 2
+       if a != b {
+               panic("broken equality: a != b")
+       }
+
+       m[a] = 1
+       m[b] = 2
+       if len(m) == 2 {
+               panic("broken hash: len(m) == 2")
+       }
+       if m[a] != 2 {
+               panic("m[a] != 2")
+       }
+}
+
+func test2() {
+       var a, b T
+       m := make(map[T]int)
+
+       copy((*[16]byte)(unsafe.Pointer(&a))[:], "hello world!")
+       a.A, a.B, a.C, a.D = 1, 2, 3, 4
+       b.A, b.B, b.C, b.D = 1, 2, 3, 4
+
+       if a != b {
+               panic("broken equality: a != b")
+       }
+
+       m[a] = 1
+       m[b] = 2
+       if len(m) == 2 {
+               panic("broken hash: len(m) == 2")
+       }
+       if m[a] != 2 {
+               panic("m[a] != 2")
+       }
+}
+
+func test3() {
+       var a, b USmall
+       copy((*[12]byte)(unsafe.Pointer(&a))[:], "hello world!")
+       a.A, a.B = 1, 2
+       b.A, b.B = 1, 2
+       if a != b {
+               panic("broken equality: a != b")
+       }
+}
+
+func main() {
+       test1()
+       test2()
+       test3()
+}