Skip to content

Commit a886959

Browse files
randall77gopherbot
authored andcommitted
[release-branch.go1.23] 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 #69156 Change-Id: I824c93d1db690999f25a3c43b2816fc28ace7509 Reviewed-on: https://go-review.googlesource.com/c/go/+/610377 Reviewed-by: Dmitri Shuralyov <[email protected]> Reviewed-by: Cuong Manh Le <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Dmitri Shuralyov <[email protected]> Reviewed-by: Keith Randall <[email protected]>
1 parent 80ff7cd commit a886959

File tree

2 files changed

+72
-1
lines changed

2 files changed

+72
-1
lines changed

src/runtime/map.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1209,6 +1209,11 @@ func (h *hmap) sameSizeGrow() bool {
12091209
return h.flags&sameSizeGrow != 0
12101210
}
12111211

1212+
//go:linkname sameSizeGrowForIssue69110Test
1213+
func sameSizeGrowForIssue69110Test(h *hmap) bool {
1214+
return h.sameSizeGrow()
1215+
}
1216+
12121217
// noldbuckets calculates the number of buckets prior to the current map growth.
12131218
func (h *hmap) noldbuckets() uintptr {
12141219
oldB := h.B
@@ -1668,7 +1673,16 @@ func moveToBmap(t *maptype, h *hmap, dst *bmap, pos int, src *bmap) (*bmap, int)
16681673
}
16691674

16701675
func mapclone2(t *maptype, src *hmap) *hmap {
1671-
dst := makemap(t, src.count, nil)
1676+
hint := src.count
1677+
if overLoadFactor(hint, src.B) {
1678+
// Note: in rare cases (e.g. during a same-sized grow) the map
1679+
// can be overloaded. Make sure we don't allocate a destination
1680+
// bucket array larger than the source bucket array.
1681+
// This will cause the cloned map to be overloaded also,
1682+
// but that's better than crashing. See issue 69110.
1683+
hint = int(loadFactorNum * (bucketShift(src.B) / loadFactorDen))
1684+
}
1685+
dst := makemap(t, hint, nil)
16721686
dst.hash0 = src.hash0
16731687
dst.nevacuate = 0
16741688
// flags do not need to be copied here, just like a new map has no flags.

test/fixedbugs/issue69110.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
// run
2+
3+
// Copyright 2024 The Go Authors. All rights reserved.
4+
// Use of this source code is governed by a BSD-style
5+
// license that can be found in the LICENSE file.
6+
7+
package main
8+
9+
import (
10+
"maps"
11+
_ "unsafe"
12+
)
13+
14+
func main() {
15+
for i := 0; i < 100; i++ {
16+
f()
17+
}
18+
}
19+
20+
const NB = 4
21+
22+
func f() {
23+
// Make a map with NB buckets, at max capacity.
24+
// 6.5 entries/bucket.
25+
ne := NB * 13 / 2
26+
m := map[int]int{}
27+
for i := 0; i < ne; i++ {
28+
m[i] = i
29+
}
30+
31+
// delete/insert a lot, to hopefully get lots of overflow buckets
32+
// and trigger a same-size grow.
33+
ssg := false
34+
for i := ne; i < ne+1000; i++ {
35+
delete(m, i-ne)
36+
m[i] = i
37+
if sameSizeGrow(m) {
38+
ssg = true
39+
break
40+
}
41+
}
42+
if !ssg {
43+
return
44+
}
45+
46+
// Insert 1 more entry, which would ordinarily trigger a growth.
47+
// We can't grow while growing, so we instead go over our
48+
// target capacity.
49+
m[-1] = -1
50+
51+
// Cloning in this state will make a map with a destination bucket
52+
// array twice the size of the source.
53+
_ = maps.Clone(m)
54+
}
55+
56+
//go:linkname sameSizeGrow runtime.sameSizeGrowForIssue69110Test
57+
func sameSizeGrow(m map[int]int) bool

0 commit comments

Comments
 (0)