Skip to content

Commit a22e317

Browse files
committed
cmd/compile: always include underlying type for map types
This is a different fix for #37716. Should help make the fix for #46283 easier, since we will no longer need to keep compiler-generated hash functions and the runtime hash function in sync. Change-Id: I84cb93144e425dcd03afc552b5fbd0f2d2cc6d39 Reviewed-on: https://go-review.googlesource.com/c/go/+/322150 Trust: Keith Randall <[email protected]> Run-TryBot: Keith Randall <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Cherry Mui <[email protected]>
1 parent 4356e7e commit a22e317

File tree

6 files changed

+49
-127
lines changed

6 files changed

+49
-127
lines changed

src/cmd/compile/internal/reflectdata/reflect.go

+9
Original file line numberDiff line numberDiff line change
@@ -1112,6 +1112,15 @@ func writeType(t *types.Type) *obj.LSym {
11121112
}
11131113
ot = objw.Uint32(lsym, ot, flags)
11141114
ot = dextratype(lsym, ot, t, 0)
1115+
if u := t.Underlying(); u != t {
1116+
// If t is a named map type, also keep the underlying map
1117+
// type live in the binary. This is important to make sure that
1118+
// a named map and that same map cast to its underlying type via
1119+
// reflection, use the same hash function. See issue 37716.
1120+
r := obj.Addrel(lsym)
1121+
r.Sym = writeType(u)
1122+
r.Type = objabi.R_KEEP
1123+
}
11151124

11161125
case types.TPTR:
11171126
if t.Elem().Kind() == types.TANY {

src/cmd/internal/objabi/reloctype.go

+3
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,9 @@ const (
101101
// *rtype, and may be set to zero by the linker if it determines the method
102102
// text is unreachable by the linked program.
103103
R_METHODOFF
104+
// R_KEEP tells the linker to keep the referred-to symbol in the final binary
105+
// if the symbol containing the R_KEEP relocation is in the final binary.
106+
R_KEEP
104107
R_POWER_TOC
105108
R_GOTPCREL
106109
// R_JMPMIPS (only used on mips64) resolves to non-PC-relative target address

src/cmd/internal/objabi/reloctype_string.go

+36-35
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/runtime/alg.go

+1-18
Original file line numberDiff line numberDiff line change
@@ -178,28 +178,11 @@ func typehash(t *_type, p unsafe.Pointer, h uintptr) uintptr {
178178
return h
179179
case kindStruct:
180180
s := (*structtype)(unsafe.Pointer(t))
181-
memStart := uintptr(0)
182-
memEnd := uintptr(0)
183181
for _, f := range s.fields {
184-
if memEnd > memStart && (f.name.isBlank() || f.offset() != memEnd || f.typ.tflag&tflagRegularMemory == 0) {
185-
// flush any pending regular memory hashing
186-
h = memhash(add(p, memStart), h, memEnd-memStart)
187-
memStart = memEnd
188-
}
189182
if f.name.isBlank() {
190183
continue
191184
}
192-
if f.typ.tflag&tflagRegularMemory == 0 {
193-
h = typehash(f.typ, add(p, f.offset()), h)
194-
continue
195-
}
196-
if memStart == memEnd {
197-
memStart = f.offset()
198-
}
199-
memEnd = f.offset() + f.typ.size
200-
}
201-
if memEnd > memStart {
202-
h = memhash(add(p, memStart), h, memEnd-memStart)
185+
h = typehash(f.typ, add(p, f.offset()), h)
203186
}
204187
return h
205188
default:

src/runtime/export_test.go

-25
Original file line numberDiff line numberDiff line change
@@ -1148,31 +1148,6 @@ func SemNwait(addr *uint32) uint32 {
11481148
return atomic.Load(&root.nwait)
11491149
}
11501150

1151-
// MapHashCheck computes the hash of the key k for the map m, twice.
1152-
// Method 1 uses the built-in hasher for the map.
1153-
// Method 2 uses the typehash function (the one used by reflect).
1154-
// Returns the two hash values, which should always be equal.
1155-
func MapHashCheck(m interface{}, k interface{}) (uintptr, uintptr) {
1156-
// Unpack m.
1157-
mt := (*maptype)(unsafe.Pointer(efaceOf(&m)._type))
1158-
mh := (*hmap)(efaceOf(&m).data)
1159-
1160-
// Unpack k.
1161-
kt := efaceOf(&k)._type
1162-
var p unsafe.Pointer
1163-
if isDirectIface(kt) {
1164-
q := efaceOf(&k).data
1165-
p = unsafe.Pointer(&q)
1166-
} else {
1167-
p = efaceOf(&k).data
1168-
}
1169-
1170-
// Compute the hash functions.
1171-
x := mt.hasher(noescape(p), uintptr(mh.hash0))
1172-
y := typehash(kt, noescape(p), uintptr(mh.hash0))
1173-
return x, y
1174-
}
1175-
11761151
// mspan wrapper for testing.
11771152
//go:notinheap
11781153
type MSpan mspan

src/runtime/hash_test.go

-49
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"fmt"
99
"math"
1010
"math/rand"
11-
"reflect"
1211
. "runtime"
1312
"strings"
1413
"testing"
@@ -49,54 +48,6 @@ func TestMemHash64Equality(t *testing.T) {
4948
}
5049
}
5150

52-
func TestCompilerVsRuntimeHash(t *testing.T) {
53-
// Test to make sure the compiler's hash function and the runtime's hash function agree.
54-
// See issue 37716.
55-
for _, m := range []interface{}{
56-
map[bool]int{},
57-
map[int8]int{},
58-
map[uint8]int{},
59-
map[int16]int{},
60-
map[uint16]int{},
61-
map[int32]int{},
62-
map[uint32]int{},
63-
map[int64]int{},
64-
map[uint64]int{},
65-
map[int]int{},
66-
map[uint]int{},
67-
map[uintptr]int{},
68-
map[*byte]int{},
69-
map[chan int]int{},
70-
map[unsafe.Pointer]int{},
71-
map[float32]int{},
72-
map[float64]int{},
73-
map[complex64]int{},
74-
map[complex128]int{},
75-
map[string]int{},
76-
//map[interface{}]int{},
77-
//map[interface{F()}]int{},
78-
map[[8]uint64]int{},
79-
map[[8]string]int{},
80-
map[struct{ a, b, c, d int32 }]int{}, // Note: tests AMEM128
81-
map[struct{ a, b, _, d int32 }]int{},
82-
map[struct {
83-
a, b int32
84-
c float32
85-
d, e [8]byte
86-
}]int{},
87-
map[struct {
88-
a int16
89-
b int64
90-
}]int{},
91-
} {
92-
k := reflect.New(reflect.TypeOf(m).Key()).Elem().Interface() // the zero key
93-
x, y := MapHashCheck(m, k)
94-
if x != y {
95-
t.Errorf("hashes did not match (%x vs %x) for map %T", x, y, m)
96-
}
97-
}
98-
}
99-
10051
// Smhasher is a torture test for hash functions.
10152
// https://code.google.com/p/smhasher/
10253
// This code is a port of some of the Smhasher tests to Go.

0 commit comments

Comments
 (0)