Commit dcd3b2c1 authored by Michael Anthony Knyszek's avatar Michael Anthony Knyszek Committed by Michael Knyszek

runtime: check whether scavAddr is in inUse on scavengeOne fast path

This change makes it so that we check whether scavAddr is actually
mapped before trying to look at the summary for the fast path, since we
may segfault if that that part of the summary is not mapped in.
Previously this wasn't a problem because we would conservatively map
all memory for the summaries between the lowest mapped heap address and
the highest one.

This change also adds a test for this case.

Change-Id: I2b1d89b5e044dce81745964dfaba829f4becdc57
Reviewed-on: https://go-review.googlesource.com/c/go/+/212637
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
parent 4f757179
......@@ -413,7 +413,10 @@ 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.
if s.summary[len(s.summary)-1][ci].max() >= uint(minPages) {
//
// Only check this if s.scavAddr is covered by any address range
// in s.inUse, so that we know our check of the summary is safe.
if s.inUse.contains(s.scavAddr) && 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
......
......@@ -282,12 +282,13 @@ func TestPageAllocScavenge(t *testing.T) {
if minPages < 1 {
minPages = 1
}
tests := map[string]struct {
type setup struct {
beforeAlloc map[ChunkIdx][]BitRange
beforeScav map[ChunkIdx][]BitRange
expect []test
afterScav map[ChunkIdx][]BitRange
}{
}
tests := map[string]setup{
"AllFreeUnscavExhaust": {
beforeAlloc: map[ChunkIdx][]BitRange{
BaseChunkIdx: {},
......@@ -396,6 +397,26 @@ func TestPageAllocScavenge(t *testing.T) {
},
},
}
if PageAlloc64Bit != 0 {
tests["ScavAllVeryDiscontiguous"] = setup{
beforeAlloc: map[ChunkIdx][]BitRange{
BaseChunkIdx: {},
BaseChunkIdx + 0x1000: {},
},
beforeScav: map[ChunkIdx][]BitRange{
BaseChunkIdx: {},
BaseChunkIdx + 0x1000: {},
},
expect: []test{
{^uintptr(0), 2 * PallocChunkPages * PageSize},
{^uintptr(0), 0},
},
afterScav: map[ChunkIdx][]BitRange{
BaseChunkIdx: {{0, PallocChunkPages}},
BaseChunkIdx + 0x1000: {{0, PallocChunkPages}},
},
}
}
for name, v := range tests {
v := v
runTest := func(t *testing.T, locked bool) {
......
......@@ -29,6 +29,11 @@ func (a addrRange) size() uintptr {
return a.limit - a.base
}
// contains returns whether or not the range contains a given address.
func (a addrRange) contains(addr uintptr) bool {
return addr >= a.base && addr < a.limit
}
// subtract takes the addrRange toPrune and cuts out any overlap with
// from, then returns the new range. subtract assumes that a and b
// either don't overlap at all, only overlap on one side, or are equal.
......@@ -87,6 +92,15 @@ func (a *addrRanges) findSucc(base uintptr) int {
return len(a.ranges)
}
// contains returns true if a covers the address addr.
func (a *addrRanges) contains(addr uintptr) bool {
i := a.findSucc(addr)
if i == 0 {
return false
}
return a.ranges[i-1].contains(addr)
}
// add inserts a new address range to a.
//
// r must not overlap with any address range in a.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment