runtime: check summary before scavenging in fast path

In scavengeOne's fast path, we currently don't check the summary for the
chunk that scavAddr points to, which means that we might accidentally
scavenge unused address space if the previous scavenge moves the
scavAddr into that space. The result of this today is a crash.

This change makes it so that scavengeOne's fast path only happens after
the check, following the comment in mpagealloc.go. It also adds a test
for this case.

Fixes #35465.
Updates #35112.

Change-Id: I861d44ee75e42a0e1f5aaec243bc449228273903
Reviewed-on: https://go-review.googlesource.com/c/go/+/206978
Run-TryBot: Michael Knyszek <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Cherry Zhang <[email protected]>

Author: mknyszek

src/runtime/mgcscavenge.go

@@ -408,13 +408,20 @@ func (s *pageAlloc) scavengeOne(max uintptr, locked bool) uintptr {
 	// Check the chunk containing the scav addr, starting at the addr
 	// and see if there are any free and unscavenged pages.
 	ci := chunkIndex(s.scavAddr)
-	base, npages := s.chunks[ci].findScavengeCandidate(chunkPageIndex(s.scavAddr), minPages, maxPages)
-
-	// If we found something, scavenge it and return!
-	if npages != 0 {
-		s.scavengeRangeLocked(ci, base, npages)
-		unlockHeap()
-		return uintptr(npages) * pageSize
+	if s.summary[len(s.summary)-1][ci].max() >= uint(minPages) {
+		// We only bother looking for a candidate if there at least
+		// minPages free pages at all. It's important that we only
+		// continue if the summary says we can because that's how
+		// we can tell if parts of the address space are unused.
+		// See the comment on s.chunks in mpagealloc.go.
+		base, npages := s.chunks[ci].findScavengeCandidate(chunkPageIndex(s.scavAddr), minPages, maxPages)
+
+		// If we found something, scavenge it and return!
+		if npages != 0 {
+			s.scavengeRangeLocked(ci, base, npages)
+			unlockHeap()
+			return uintptr(npages) * pageSize
+		}
 	}
 	unlockHeap()
 

src/runtime/mgcscavenge_test.go

@@ -373,6 +373,25 @@ func TestPageAllocScavenge(t *testing.T) {
 				BaseChunkIdx + 1: {{0, PallocChunkPages}},
 			},
 		},
+		"ScavDiscontiguous": {
+			beforeAlloc: map[ChunkIdx][]BitRange{
+				BaseChunkIdx:       {},
+				BaseChunkIdx + 0xe: {},
+			},
+			beforeScav: map[ChunkIdx][]BitRange{
+				BaseChunkIdx:       {{uint(minPages), PallocChunkPages - uint(2*minPages)}},
+				BaseChunkIdx + 0xe: {{uint(2 * minPages), PallocChunkPages - uint(2*minPages)}},
+			},
+			expect: []test{
+				{2 * minPages * PageSize, 2 * minPages * PageSize},
+				{^uintptr(0), 2 * minPages * PageSize},
+				{^uintptr(0), 0},
+			},
+			afterScav: map[ChunkIdx][]BitRange{
+				BaseChunkIdx:       {{0, PallocChunkPages}},
+				BaseChunkIdx + 0xe: {{0, PallocChunkPages}},
+			},
+		},
 	}
 	for name, v := range tests {
 		v := v