From 2ad57b45833cb7db3f5ae501d97b731ef16e8ff6 Mon Sep 17 00:00:00 2001 From: =?utf8?q?R=C3=A9my=20Oudompheng?= Date: Fri, 18 Jan 2013 18:26:43 +0100 Subject: [PATCH] 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 --- src/cmd/gc/subr.c | 45 +++++++++++++----- src/cmd/gc/walk.c | 2 + test/blank.go | 17 +++++-- test/fixedbugs/issue4585.go | 91 +++++++++++++++++++++++++++++++++++++ 4 files changed, 139 insertions(+), 16 deletions(-) create mode 100644 test/fixedbugs/issue4585.go diff --git a/src/cmd/gc/subr.c b/src/cmd/gc/subr.c index 674c49bc03..c032ffae05 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 3a2152b092..3bcbb9cd74 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 ee618b1485..ad4d6ebdc6 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 0000000000..558bd1e100 --- /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() +} -- 2.48.1