runtime: size maps.Clone destination bucket array safely

In rare situations, like during same-sized grows, the source map for
maps.Clone may be overloaded (has more than 6.5 entries per
bucket). This causes the runtime to allocate a larger bucket array for
the destination map than for the source map. The maps.Clone code
walks off the end of the source array if it is smaller than the
destination array.

This is a pretty simple fix, ensuring that the destination bucket
array is never longer than the source bucket array. Maybe a better fix
is to make the Clone code handle shorter source arrays correctly, but
this fix is deliberately simple to reduce the risk of backporting this
fix.

Fixes #69110

Change-Id: I824c93d1db690999f25a3c43b2816fc28ace7509
Reviewed-on: https://go-review.googlesource.com/c/go/+/609757
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>

diff --git a/src/runtime/map_noswiss.go b/src/runtime/map_noswiss.go
index 3a1c774b9d7296be34f2542b80e1f0481921c7b9..95c4f049b1d6cf39c4e18d1e260217c8f40274b2 100644 (file)
--- a/src/runtime/map_noswiss.go
+++ b/src/runtime/map_noswiss.go
@@ -1213,6 +1213,11 @@ func (h *hmap) sameSizeGrow() bool {
        return h.flags&sameSizeGrow != 0
 }
 
+//go:linkname sameSizeGrowForIssue69110Test
+func sameSizeGrowForIssue69110Test(h *hmap) bool {
+       return h.sameSizeGrow()
+}
+
 // noldbuckets calculates the number of buckets prior to the current map growth.
 func (h *hmap) noldbuckets() uintptr {
        oldB := h.B
@@ -1672,7 +1677,16 @@ func moveToBmap(t *maptype, h *hmap, dst *bmap, pos int, src *bmap) (*bmap, int)
 }
 
 func mapclone2(t *maptype, src *hmap) *hmap {
-       dst := makemap(t, src.count, nil)
+       hint := src.count
+       if overLoadFactor(hint, src.B) {
+               // Note: in rare cases (e.g. during a same-sized grow) the map
+               // can be overloaded. Make sure we don't allocate a destination
+               // bucket array larger than the source bucket array.
+               // This will cause the cloned map to be overloaded also,
+               // but that's better than crashing. See issue 69110.
+               hint = int(loadFactorNum * (bucketShift(src.B) / loadFactorDen))
+       }
+       dst := makemap(t, hint, nil)
        dst.hash0 = src.hash0
        dst.nevacuate = 0
        // flags do not need to be copied here, just like a new map has no flags.

diff --git a/src/runtime/map_swiss.go b/src/runtime/map_swiss.go
index b76878da9a8e36a99b07c6fc54b96e5a95fdc79a..aa2fb618598eea8311f975ae68c56db209465ad5 100644 (file)
--- a/src/runtime/map_swiss.go
+++ b/src/runtime/map_swiss.go
@@ -1119,6 +1119,11 @@ func (h *hmap) sameSizeGrow() bool {
        return h.flags&sameSizeGrow != 0
 }
 
+//go:linkname sameSizeGrowForIssue69110Test
+func sameSizeGrowForIssue69110Test(h *hmap) bool {
+       return h.sameSizeGrow()
+}
+
 // noldbuckets calculates the number of buckets prior to the current map growth.
 func (h *hmap) noldbuckets() uintptr {
        oldB := h.B
@@ -1497,7 +1502,16 @@ func moveToBmap(t *maptype, h *hmap, dst *bmap, pos int, src *bmap) (*bmap, int)
 }
 
 func mapclone2(t *maptype, src *hmap) *hmap {
-       dst := makemap(t, src.count, nil)
+       hint := src.count
+       if overLoadFactor(hint, src.B) {
+               // Note: in rare cases (e.g. during a same-sized grow) the map
+               // can be overloaded. Make sure we don't allocate a destination
+               // bucket array larger than the source bucket array.
+               // This will cause the cloned map to be overloaded also,
+               // but that's better than crashing. See issue 69110.
+               hint = int(loadFactorNum * (bucketShift(src.B) / loadFactorDen))
+       }
+       dst := makemap(t, hint, nil)
        dst.hash0 = src.hash0
        dst.nevacuate = 0
        // flags do not need to be copied here, just like a new map has no flags.

diff --git a/test/fixedbugs/issue69110.go b/test/fixedbugs/issue69110.go
new file mode 100644 (file)
index 0000000..71a4bca
--- /dev/null
+++ b/test/fixedbugs/issue69110.go
@@ -0,0 +1,57 @@
+// run
+
+// Copyright 2024 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 (
+       "maps"
+       _ "unsafe"
+)
+
+func main() {
+       for i := 0; i < 100; i++ {
+               f()
+       }
+}
+
+const NB = 4
+
+func f() {
+       // Make a map with NB buckets, at max capacity.
+       // 6.5 entries/bucket.
+       ne := NB * 13 / 2
+       m := map[int]int{}
+       for i := 0; i < ne; i++ {
+               m[i] = i
+       }
+
+       // delete/insert a lot, to hopefully get lots of overflow buckets
+       // and trigger a same-size grow.
+       ssg := false
+       for i := ne; i < ne+1000; i++ {
+               delete(m, i-ne)
+               m[i] = i
+               if sameSizeGrow(m) {
+                       ssg = true
+                       break
+               }
+       }
+       if !ssg {
+               return
+       }
+
+       // Insert 1 more entry, which would ordinarily trigger a growth.
+       // We can't grow while growing, so we instead go over our
+       // target capacity.
+       m[-1] = -1
+
+       // Cloning in this state will make a map with a destination bucket
+       // array twice the size of the source.
+       _ = maps.Clone(m)
+}
+
+//go:linkname sameSizeGrow runtime.sameSizeGrowForIssue69110Test
+func sameSizeGrow(m map[int]int) bool