cmd/gc: fix handling of struct padding in hash/eq.

The test case of issue 4585 was not passing due to
miscalculation of memequal args, and the previous fix
does not handle padding at the end of a struct.

Handling of padding at end of structs also fixes the case
of [n]T where T is such a padded struct.

Fixes #4585.
(again)

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

diff --git a/src/cmd/gc/subr.c b/src/cmd/gc/subr.c
index c032ffae051483578c5f4413562afc7f667c048d..afbdd0ccadbbd1b297c26799f325e186932524c0 100644 (file)
--- a/src/cmd/gc/subr.c
+++ b/src/cmd/gc/subr.c
@@ -504,12 +504,17 @@ nod(int op, Node *nleft, Node *nright)
        return n;
 }
 
+// ispaddedfield returns whether the given field
+// is followed by padding. For the case where t is
+// the last field, total gives the size of the enclosing struct.
 static int
-ispaddedfield(Type *t)
+ispaddedfield(Type *t, vlong total)
 {
        if(t->etype != TFIELD)
                fatal("ispaddedfield called non-field %T", t);
-       return t->down != T && t->width + t->type->width != t->down->width;
+       if(t->down == T)
+               return t->width + t->type->width != total;
+       return t->width + t->type->width != t->down->width;
 }
 
 int
@@ -591,7 +596,7 @@ algtype1(Type *t, Type **bad)
                for(t1=t->type; t1!=T; t1=t1->down) {
                        // Blank fields and padding must be ignored,
                        // so need special compare.
-                       if(isblanksym(t1->sym) || ispaddedfield(t1)) {
+                       if(isblanksym(t1->sym) || ispaddedfield(t1, t->width)) {
                                ret = -1;
                                continue;
                        }
@@ -2619,7 +2624,7 @@ genhash(Sym *sym, Type *t)
        Node *hashel;
        Type *first, *t1;
        int old_safemode;
-       int64 size, mul;
+       int64 size, mul, offend;
 
        if(debug['r'])
                print("genhash %S %T\n", sym, t);
@@ -2705,24 +2710,21 @@ genhash(Sym *sym, Type *t)
                // Walk the struct using memhash for runs of AMEM
                // and calling specific hash functions for the others.
                first = T;
+               offend = 0;
                for(t1=t->type;; t1=t1->down) {
                        if(t1 != T && algtype1(t1->type, nil) == AMEM && !isblanksym(t1->sym)) {
+                               offend = t1->width + t1->type->width;
                                if(first == T)
                                        first = t1;
                                // If it's a memory field but it's padded, stop here.
-                               if(ispaddedfield(t1))
+                               if(ispaddedfield(t1, t->width))
                                        t1 = t1->down;
                                else
                                        continue;
                        }
                        // Run memhash for fields up to this one.
                        if(first != T) {
-                               if(first->down == t1)
-                                       size = first->type->width;
-                               else if(t1 == T)
-                                       size = t->width - first->width;  // first->width is offset
-                               else
-                                       size = t1->width - first->width;  // both are offsets
+                               size = offend - first->width; // first->width is offset
                                hashel = hashmem(first->type);
                                // hashel(h, size, &p.first)
                                call = nod(OCALL, hashel, N);
@@ -2856,6 +2858,7 @@ geneq(Sym *sym, Type *t)
        Type *t1, *first;
        int old_safemode;
        int64 size;
+       int64 offend;
 
        if(debug['r'])
                print("geneq %S %T\n", sym, t);
@@ -2929,12 +2932,14 @@ geneq(Sym *sym, Type *t)
                // and calling specific equality tests for the others.
                // Skip blank-named fields.
                first = T;
+               offend = 0;
                for(t1=t->type;; t1=t1->down) {
                        if(t1 != T && algtype1(t1->type, nil) == AMEM && !isblanksym(t1->sym)) {
+                               offend = t1->width + t1->type->width;
                                if(first == T)
                                        first = t1;
                                // If it's a memory field but it's padded, stop here.
-                               if(ispaddedfield(t1))
+                               if(ispaddedfield(t1, t->width))
                                        t1 = t1->down;
                                else
                                        continue;
@@ -2952,10 +2957,7 @@ geneq(Sym *sym, Type *t)
                                                fn->nbody = list(fn->nbody, eqfield(np, nq, newname(first->sym), neq));
                                } else {
                                        // More than two fields: use memequal.
-                                       if(t1 == T)
-                                               size = t->width - first->width;  // first->width is offset
-                                       else
-                                               size = t1->width - first->width;  // both are offsets
+                                       size = offend - first->width; // first->width is offset
                                        fn->nbody = list(fn->nbody, eqmem(np, nq, newname(first->sym), size, neq));
                                }
                                first = T;

diff --git a/test/fixedbugs/issue4585.go b/test/fixedbugs/issue4585.go
index 558bd1e1008124ff5e2d1938aa5e1cc484fea984..ad1242d1e5f25627ecb779647e2fd973bce6f29a 100644 (file)
--- a/test/fixedbugs/issue4585.go
+++ b/test/fixedbugs/issue4585.go
@@ -32,6 +32,20 @@ type USmall struct {
        A, _, B int32
 }
 
+// V has padding but not on the first field.
+type V struct {
+       A1, A2, A3 int32
+       B          int16
+       C          int32
+}
+
+// W has padding at the end.
+type W struct {
+       A1, A2, A3 int32
+       B          int32
+       C          int8
+}
+
 func test1() {
        var a, b U
        m := make(map[U]int)
@@ -84,8 +98,54 @@ func test3() {
        }
 }
 
+func test4() {
+       var a, b V
+       m := make(map[V]int)
+
+       copy((*[20]byte)(unsafe.Pointer(&a))[:], "Hello World, Gopher!")
+       a.A1, a.A2, a.A3, a.B, a.C = 1, 2, 3, 4, 5
+       b.A1, b.A2, b.A3, b.B, b.C = 1, 2, 3, 4, 5
+
+       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 test5() {
+       var a, b W
+       m := make(map[W]int)
+
+       copy((*[20]byte)(unsafe.Pointer(&a))[:], "Hello World, Gopher!")
+       a.A1, a.A2, a.A3, a.B, a.C = 1, 2, 3, 4, 5
+       b.A1, b.A2, b.A3, b.B, b.C = 1, 2, 3, 4, 5
+
+       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 main() {
        test1()
        test2()
        test3()
+       test4()
+       test5()
 }