runtime: fix min/max logic in findScavengeCandidate

Before this CL, if max > min and max was unaligned to min, then the
function could return an unaligned (unaligned to min) region to
scavenge. On most platforms, this leads to some kind of crash.

Fix this by explicitly aligning max to the next multiple of min.

Fixes #35445.
Updates #35112.

Change-Id: I0af42d4a307b48a97e47ed152c619d77b0298291
Reviewed-on: https://go-review.googlesource.com/c/go/+/206277
Reviewed-by: Ian Lance Taylor <iant@golang.org>

diff --git a/src/runtime/mgcscavenge.go b/src/runtime/mgcscavenge.go
index 4c2fb4403c762f8c3d53808959ef9267cb8a28e8..db7c8121dca7f1ed1e3f004afe7e4a5fb51dd616 100644 (file)
--- a/src/runtime/mgcscavenge.go
+++ b/src/runtime/mgcscavenge.go
@@ -602,9 +602,10 @@ func (m *pallocData) hasScavengeCandidate(min uintptr) bool {
 // findScavengeCandidate effectively returns entire free and unscavenged regions.
 // If max < pallocChunkPages, it may truncate the returned region such that size is
 // max. However, findScavengeCandidate may still return a larger region if, for
-// example, it chooses to preserve huge pages. That is, even if max is small,
-// size is not guaranteed to be equal to max. max is allowed to be less than min,
-// in which case it is as if max == min.
+// example, it chooses to preserve huge pages, or if max is not aligned to min (it
+// will round up). That is, even if max is small, the returned size is not guaranteed
+// to be equal to max. max is allowed to be less than min, in which case it is as if
+// max == min.
 func (m *pallocData) findScavengeCandidate(searchIdx uint, min, max uintptr) (uint, uint) {
        if min&(min-1) != 0 || min == 0 {
                print("runtime: min = ", min, "\n")
@@ -613,10 +614,15 @@ func (m *pallocData) findScavengeCandidate(searchIdx uint, min, max uintptr) (ui
                print("runtime: min = ", min, "\n")
                throw("min too large")
        }
-       // max is allowed to be less than min, but we need to ensure
-       // we never truncate further than min.
-       if max < min {
+       // max may not be min-aligned, so we might accidentally truncate to
+       // a max value which causes us to return a non-min-aligned value.
+       // To prevent this, align max up to a multiple of min (which is always
+       // a power of 2). This also prevents max from ever being less than
+       // min, unless it's zero, so handle that explicitly.
+       if max == 0 {
                max = min
+       } else {
+               max = alignUp(max, min)
        }
 
        i := int(searchIdx / 64)

diff --git a/src/runtime/mgcscavenge_test.go b/src/runtime/mgcscavenge_test.go
index ca507450dbd9114b992c86e30eaa5740592ba2e9..b29a4d796ab88db2117b7fbbd2809cbc85db3ca2 100644 (file)
--- a/src/runtime/mgcscavenge_test.go
+++ b/src/runtime/mgcscavenge_test.go
@@ -184,6 +184,12 @@ func TestPallocDataFindScavengeCandidate(t *testing.T) {
                        max:       3 * m,
                        want:      BitRange{128, 3 * uint(m)},
                }
+               tests["Max0"+suffix] = test{
+                       scavenged: []BitRange{{0, PallocChunkPages - uint(m)}},
+                       min:       m,
+                       max:       0,
+                       want:      BitRange{PallocChunkPages - uint(m), uint(m)},
+               }
                if m <= 8 {
                        tests["OneFree"] = test{
                                alloc: []BitRange{{0, 40}, {40 + uint(m), PallocChunkPages - (40 + uint(m))}},
@@ -200,6 +206,12 @@ func TestPallocDataFindScavengeCandidate(t *testing.T) {
                        }
                }
                if m > 1 {
+                       tests["MaxUnaligned"+suffix] = test{
+                               scavenged: []BitRange{{0, PallocChunkPages - uint(m*2-1)}},
+                               min:       m,
+                               max:       m - 2,
+                               want:      BitRange{PallocChunkPages - uint(m), uint(m)},
+                       }
                        tests["SkipSmall"+suffix] = test{
                                alloc: []BitRange{{0, 64 - uint(m)}, {64, 5}, {70, 11}, {82, PallocChunkPages - 82}},
                                min:   m,