From 8455a848e7e98da060ec3979c9cf32c82bae9a20 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Thu, 3 Jan 2019 12:13:53 -0800 Subject: [PATCH] [release-branch.go1.11] runtime: don't scan go'd function args past length of ptr bitmap 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. Fixes #29565 Change-Id: I78f3f7f9ec89c2ad4678f0c52d3d3def9cac8e72 Reviewed-on: https://go-review.googlesource.com/c/156123 Run-TryBot: Keith Randall TryBot-Result: Gobot Gobot Reviewed-by: Austin Clements Reviewed-on: https://go-review.googlesource.com/c/156359 Reviewed-by: Ian Lance Taylor --- src/runtime/proc.go | 2 +- test/fixedbugs/issue29362b.go | 53 +++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 test/fixedbugs/issue29362b.go diff --git a/src/runtime/proc.go b/src/runtime/proc.go index e309c505b4..301d9213ff 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -3374,7 +3374,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) } } } diff --git a/test/fixedbugs/issue29362b.go b/test/fixedbugs/issue29362b.go new file mode 100644 index 0000000000..d1e3b4733f --- /dev/null +++ b/test/fixedbugs/issue29362b.go @@ -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) + } +} -- 2.50.0