Skip to content

Commit

Permalink
runtime: don't scan go'd function args past length of ptr bitmap
Browse files Browse the repository at this point in the history
Use the length of the bitmap to decide how much to pass to the
write barrier, not the total length of the arguments.

The test needs enough arguments so that two distinct bitmaps
get interpreted as a single longer bitmap.

Update #29362

Change-Id: I78f3f7f9ec89c2ad4678f0c52d3d3def9cac8e72
Reviewed-on: https://go-review.googlesource.com/c/156123
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
  • Loading branch information
randall77 committed Jan 3, 2019
1 parent 5073bf3 commit 6886677
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/runtime/proc.go
Original file line number Diff line number Diff line change
Expand Up @@ -3306,7 +3306,7 @@ func newproc1(fn *funcval, argp *uint8, narg int32, callergp *g, callerpc uintpt
if stkmap.nbit > 0 {
// We're in the prologue, so it's always stack map index 0.
bv := stackmapdata(stkmap, 0)
bulkBarrierBitmap(spArg, spArg, uintptr(narg), 0, bv.bytedata)
bulkBarrierBitmap(spArg, spArg, uintptr(bv.n)*sys.PtrSize, 0, bv.bytedata)
}
}
}
Expand Down
53 changes: 53 additions & 0 deletions test/fixedbugs/issue29362b.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// run

// Copyright 2018 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.

// Verify that we don't consider a Go'd function's
// arguments as pointers when they aren't.

package main

import (
"unsafe"
)

var badPtr uintptr

var sink []byte

func init() {
// Allocate large enough to use largeAlloc.
b := make([]byte, 1<<16-1)
sink = b // force heap allocation
// Any space between the object and the end of page is invalid to point to.
badPtr = uintptr(unsafe.Pointer(&b[len(b)-1])) + 1
}

var throttle = make(chan struct{}, 10)

// There are 2 arg bitmaps for this function, each with 2 bits.
// In the first, p and q are both live, so that bitmap is 11.
// In the second, only p is live, so that bitmap is 10.
// Bitmaps are byte aligned, so if the first bitmap is interpreted as
// extending across the entire argument area, we incorrectly concatenate
// the bitmaps and end up using 110000001. That bad bitmap causes a6
// to be considered a pointer.
func noPointerArgs(p, q *byte, a0, a1, a2, a3, a4, a5, a6 uintptr) {
sink = make([]byte, 4096)
sinkptr = q
<-throttle
sinkptr = p
}

var sinkptr *byte

func main() {
const N = 1000
for i := 0; i < N; i++ {
throttle <- struct{}{}
go noPointerArgs(nil, nil, badPtr, badPtr, badPtr, badPtr, badPtr, badPtr, badPtr)
sink = make([]byte, 4096)
}
}

0 comments on commit 6886677

Please sign in to comment.