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

Fixes issue 4585.

R=rsc, golang-dev
CC=golang-dev
https://codereview.appspot.com/7142052

4 files changed, 139 insertions, 16 deletions

diff --git a/src/cmd/gc/subr.c b/src/cmd/gc/subr.c
index 674c49bc0..c032ffae0 100644
--- 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 3a2152b09..3bcbb9cd7 100644
--- 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 ee618b148..ad4d6ebdc 100644
--- 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
index 000000000..558bd1e10
--- /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()
+}