]> Cypherpunks repositories - gostls13.git/commitdiff
[release-branch.go1.9] cmd/compile: fix mapassign_fast* routines for pointer keys
authorKeith Randall <keithr@alum.mit.edu>
Tue, 21 Nov 2017 15:14:11 +0000 (07:14 -0800)
committerAndrew Bonventre <andybons@golang.org>
Mon, 22 Jan 2018 20:25:13 +0000 (20:25 +0000)
The signature of the mapassign_fast* routines need to distinguish
the pointerness of their key argument.  If the affected routines
suspend part way through, the object pointed to by the key might
get garbage collected because the key is typed as a uint{32,64}.

This is not a problem for mapaccess or mapdelete because the key
in those situations do not live beyond the call involved.  If the
object referenced by the key is garbage collected prematurely, the
code still works fine.  Even if that object is subsequently reallocated,
it can't be written to the map in time to affect the lookup/delete.

Fixes #22781

Change-Id: I0bbbc5e9883d5ce702faf4e655348be1191ee439
Reviewed-on: https://go-review.googlesource.com/79018
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Martin Möhrmann <moehrmann@google.com>
Reviewed-on: https://go-review.googlesource.com/88635
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
src/cmd/compile/internal/gc/builtin.go
src/cmd/compile/internal/gc/builtin/runtime.go
src/cmd/compile/internal/gc/walk.go
src/runtime/hashmap_fast.go
test/fixedbugs/issue22781.go [new file with mode: 0644]

index f21a4da4913ce9a42cbf5bb4bb4dabf31eb2d7d1..fafed7cae8e458815302efd92df902ef5376c015 100644 (file)
@@ -86,7 +86,9 @@ var runtimeDecls = [...]struct {
        {"mapaccess2_fat", funcTag, 67},
        {"mapassign", funcTag, 62},
        {"mapassign_fast32", funcTag, 63},
+       {"mapassign_fast32ptr", funcTag, 63},
        {"mapassign_fast64", funcTag, 63},
+       {"mapassign_fast64ptr", funcTag, 63},
        {"mapassign_faststr", funcTag, 63},
        {"mapiterinit", funcTag, 68},
        {"mapdelete", funcTag, 68},
index 7f4846db9dbbdaffff08855b6046abd583908a7e..c3994c5fa2d7bc36c55754180989aa75ac64ed2f 100644 (file)
@@ -106,7 +106,9 @@ func mapaccess2_faststr(mapType *byte, hmap map[any]any, key any) (val *any, pre
 func mapaccess2_fat(mapType *byte, hmap map[any]any, key *any, zero *byte) (val *any, pres bool)
 func mapassign(mapType *byte, hmap map[any]any, key *any) (val *any)
 func mapassign_fast32(mapType *byte, hmap map[any]any, key any) (val *any)
+func mapassign_fast32ptr(mapType *byte, hmap map[any]any, key any) (val *any)
 func mapassign_fast64(mapType *byte, hmap map[any]any, key any) (val *any)
+func mapassign_fast64ptr(mapType *byte, hmap map[any]any, key any) (val *any)
 func mapassign_faststr(mapType *byte, hmap map[any]any, key any) (val *any)
 func mapiterinit(mapType *byte, hmap map[any]any, hiter *any)
 func mapdelete(mapType *byte, hmap map[any]any, key *any)
index 76031d160abb9ada73b7e84399a16374169a0d7b..ab2e38208dd58536d31d1d065e7ef8bc11e5826e 100644 (file)
@@ -2785,21 +2785,23 @@ func mapfndel(name string, t *types.Type) *Node {
 const (
        mapslow = iota
        mapfast32
+       mapfast32ptr
        mapfast64
+       mapfast64ptr
        mapfaststr
        nmapfast
 )
 
 type mapnames [nmapfast]string
 
-func mkmapnames(base string) mapnames {
-       return mapnames{base, base + "_fast32", base + "_fast64", base + "_faststr"}
+func mkmapnames(base string, ptr string) mapnames {
+       return mapnames{base, base + "_fast32", base + "_fast32" + ptr, base + "_fast64", base + "_fast64" + ptr, base + "_faststr"}
 }
 
-var mapaccess1 mapnames = mkmapnames("mapaccess1")
-var mapaccess2 mapnames = mkmapnames("mapaccess2")
-var mapassign mapnames = mkmapnames("mapassign")
-var mapdelete mapnames = mkmapnames("mapdelete")
+var mapaccess1 mapnames = mkmapnames("mapaccess1", "")
+var mapaccess2 mapnames = mkmapnames("mapaccess2", "")
+var mapassign mapnames = mkmapnames("mapassign", "ptr")
+var mapdelete mapnames = mkmapnames("mapdelete", "")
 
 func mapfast(t *types.Type) int {
        // Check ../../runtime/hashmap.go:maxValueSize before changing.
@@ -2808,9 +2810,22 @@ func mapfast(t *types.Type) int {
        }
        switch algtype(t.Key()) {
        case AMEM32:
-               return mapfast32
+               if !t.Key().HasPointer() {
+                       return mapfast32
+               }
+               if Widthptr == 4 {
+                       return mapfast32ptr
+               }
+               Fatalf("small pointer %v", t.Key())
        case AMEM64:
-               return mapfast64
+               if !t.Key().HasPointer() {
+                       return mapfast64
+               }
+               if Widthptr == 8 {
+                       return mapfast64ptr
+               }
+               // Two-word object, at least one of which is a pointer.
+               // Use the slow path.
        case ASTRING:
                return mapfaststr
        }
index 67b978790989b05a3c262afc04bba7263d5e786f..c07992b0f3132ad1f8abcad5acfcbce6ec61fb63 100644 (file)
@@ -507,6 +507,94 @@ done:
        return val
 }
 
+func mapassign_fast32ptr(t *maptype, h *hmap, key unsafe.Pointer) unsafe.Pointer {
+       if h == nil {
+               panic(plainError("assignment to entry in nil map"))
+       }
+       if raceenabled {
+               callerpc := getcallerpc(unsafe.Pointer(&t))
+               racewritepc(unsafe.Pointer(h), callerpc, funcPC(mapassign_fast32))
+       }
+       if h.flags&hashWriting != 0 {
+               throw("concurrent map writes")
+       }
+       hash := t.key.alg.hash(noescape(unsafe.Pointer(&key)), uintptr(h.hash0))
+
+       // Set hashWriting after calling alg.hash for consistency with mapassign.
+       h.flags |= hashWriting
+
+       if h.buckets == nil {
+               h.buckets = newarray(t.bucket, 1)
+       }
+
+again:
+       bucket := hash & (uintptr(1)<<h.B - 1)
+       if h.growing() {
+               growWork(t, h, bucket)
+       }
+       b := (*bmap)(unsafe.Pointer(uintptr(h.buckets) + bucket*uintptr(t.bucketsize)))
+       top := uint8(hash >> (sys.PtrSize*8 - 8))
+       if top < minTopHash {
+               top += minTopHash
+       }
+
+       var inserti *uint8
+       var insertk unsafe.Pointer
+       var val unsafe.Pointer
+       for {
+               for i := uintptr(0); i < bucketCnt; i++ {
+                       if b.tophash[i] != top {
+                               if b.tophash[i] == empty && inserti == nil {
+                                       inserti = &b.tophash[i]
+                                       insertk = add(unsafe.Pointer(b), dataOffset+i*4)
+                                       val = add(unsafe.Pointer(b), dataOffset+bucketCnt*4+i*uintptr(t.valuesize))
+                               }
+                               continue
+                       }
+                       k := *((*unsafe.Pointer)(add(unsafe.Pointer(b), dataOffset+i*4)))
+                       if k != key {
+                               continue
+                       }
+                       val = add(unsafe.Pointer(b), dataOffset+bucketCnt*4+i*uintptr(t.valuesize))
+                       goto done
+               }
+               ovf := b.overflow(t)
+               if ovf == nil {
+                       break
+               }
+               b = ovf
+       }
+
+       // Did not find mapping for key. Allocate new cell & add entry.
+
+       // If we hit the max load factor or we have too many overflow buckets,
+       // and we're not already in the middle of growing, start growing.
+       if !h.growing() && (overLoadFactor(int64(h.count), h.B) || tooManyOverflowBuckets(h.noverflow, h.B)) {
+               hashGrow(t, h)
+               goto again // Growing the table invalidates everything, so try again
+       }
+
+       if inserti == nil {
+               // all current buckets are full, allocate a new one.
+               newb := h.newoverflow(t, b)
+               inserti = &newb.tophash[0]
+               insertk = add(unsafe.Pointer(newb), dataOffset)
+               val = add(insertk, bucketCnt*4)
+       }
+
+       // store new key/value at insert position
+       typedmemmove(t.key, insertk, unsafe.Pointer(&key))
+       *inserti = top
+       h.count++
+
+done:
+       if h.flags&hashWriting == 0 {
+               throw("concurrent map writes")
+       }
+       h.flags &^= hashWriting
+       return val
+}
+
 func mapassign_fast64(t *maptype, h *hmap, key uint64) unsafe.Pointer {
        if h == nil {
                panic(plainError("assignment to entry in nil map"))
@@ -595,6 +683,94 @@ done:
        return val
 }
 
+func mapassign_fast64ptr(t *maptype, h *hmap, key unsafe.Pointer) unsafe.Pointer {
+       if h == nil {
+               panic(plainError("assignment to entry in nil map"))
+       }
+       if raceenabled {
+               callerpc := getcallerpc(unsafe.Pointer(&t))
+               racewritepc(unsafe.Pointer(h), callerpc, funcPC(mapassign_fast64))
+       }
+       if h.flags&hashWriting != 0 {
+               throw("concurrent map writes")
+       }
+       hash := t.key.alg.hash(noescape(unsafe.Pointer(&key)), uintptr(h.hash0))
+
+       // Set hashWriting after calling alg.hash for consistency with mapassign.
+       h.flags |= hashWriting
+
+       if h.buckets == nil {
+               h.buckets = newarray(t.bucket, 1)
+       }
+
+again:
+       bucket := hash & (uintptr(1)<<h.B - 1)
+       if h.growing() {
+               growWork(t, h, bucket)
+       }
+       b := (*bmap)(unsafe.Pointer(uintptr(h.buckets) + bucket*uintptr(t.bucketsize)))
+       top := uint8(hash >> (sys.PtrSize*8 - 8))
+       if top < minTopHash {
+               top += minTopHash
+       }
+
+       var inserti *uint8
+       var insertk unsafe.Pointer
+       var val unsafe.Pointer
+       for {
+               for i := uintptr(0); i < bucketCnt; i++ {
+                       if b.tophash[i] != top {
+                               if b.tophash[i] == empty && inserti == nil {
+                                       inserti = &b.tophash[i]
+                                       insertk = add(unsafe.Pointer(b), dataOffset+i*8)
+                                       val = add(unsafe.Pointer(b), dataOffset+bucketCnt*8+i*uintptr(t.valuesize))
+                               }
+                               continue
+                       }
+                       k := *((*unsafe.Pointer)(add(unsafe.Pointer(b), dataOffset+i*8)))
+                       if k != key {
+                               continue
+                       }
+                       val = add(unsafe.Pointer(b), dataOffset+bucketCnt*8+i*uintptr(t.valuesize))
+                       goto done
+               }
+               ovf := b.overflow(t)
+               if ovf == nil {
+                       break
+               }
+               b = ovf
+       }
+
+       // Did not find mapping for key. Allocate new cell & add entry.
+
+       // If we hit the max load factor or we have too many overflow buckets,
+       // and we're not already in the middle of growing, start growing.
+       if !h.growing() && (overLoadFactor(int64(h.count), h.B) || tooManyOverflowBuckets(h.noverflow, h.B)) {
+               hashGrow(t, h)
+               goto again // Growing the table invalidates everything, so try again
+       }
+
+       if inserti == nil {
+               // all current buckets are full, allocate a new one.
+               newb := h.newoverflow(t, b)
+               inserti = &newb.tophash[0]
+               insertk = add(unsafe.Pointer(newb), dataOffset)
+               val = add(insertk, bucketCnt*8)
+       }
+
+       // store new key/value at insert position
+       typedmemmove(t.key, insertk, unsafe.Pointer(&key))
+       *inserti = top
+       h.count++
+
+done:
+       if h.flags&hashWriting == 0 {
+               throw("concurrent map writes")
+       }
+       h.flags &^= hashWriting
+       return val
+}
+
 func mapassign_faststr(t *maptype, h *hmap, ky string) unsafe.Pointer {
        if h == nil {
                panic(plainError("assignment to entry in nil map"))
diff --git a/test/fixedbugs/issue22781.go b/test/fixedbugs/issue22781.go
new file mode 100644 (file)
index 0000000..5ad8239
--- /dev/null
@@ -0,0 +1,29 @@
+// run
+
+// Copyright 2017 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.
+
+package main
+
+import "runtime/debug"
+
+type T struct {
+       // >= 16 bytes to avoid tiny alloc.
+       a, b int
+}
+
+func main() {
+       debug.SetGCPercent(1)
+       for i := 0; i < 100000; i++ {
+               m := make(map[*T]struct{}, 0)
+               for j := 0; j < 20; j++ {
+                       // During the call to mapassign_fast64, the key argument
+                       // was incorrectly treated as a uint64. If the stack was
+                       // scanned during that call, the only pointer to k was
+                       // missed, leading to *k being collected prematurely.
+                       k := new(T)
+                       m[k] = struct{}{}
+               }
+       }
+}