mirror of
https://github.com/golang/go
synced 2024-11-24 22:47:58 -07:00
f6bff1d587
From the garbage collector's perspective, time can move backwards in
cgocall. However, in the midst of this time warp, the pointer
arguments to cgocall can go from dead back to live. If a stack growth
happens while they're dead and then a GC happens when they become live
again, GC can crash with a bad heap pointer.
Specifically, the sequence that leads to a panic is:
1. cgocall calls entersyscall, which saves the PC and SP of its call
site in cgocall. Call this PC/SP "X". At "X" both pointer arguments
are live.
2. cgocall calls asmcgocall. Call the PC/SP of this call "Y". At "Y"
neither pointer argument is live.
3. asmcgocall calls the C code, which eventually calls back into the
Go code.
4. cgocallbackg remembers the saved PC/SP "X" in some local variables,
calls exitsyscall, and then calls cgocallbackg1.
5. The Go code causes a stack growth. This stack unwind sees PC/SP "Y"
in the cgocall frame. Since the arguments are dead at "Y", they are
not adjusted.
6. The Go code returns to cgocallbackg1, which calls reentersyscall
with the recorded saved PC/SP "X", so "X" gets stashed back into
gp.syscallpc/sp.
7. GC scans the stack. It sees there's a saved syscall PC/SP, so it
starts the traceback at PC/SP "X". At "X" the arguments are considered
live, so it scans them, but since they weren't adjusted, the pointers
are bad, so it panics.
This issue started as of commit ca4089ad
, when the compiler stopped
marking arguments as live for the whole function.
Since this is a variable liveness issue, fix it by adding KeepAlive
calls that keep the arguments live across this whole time warp.
The existing issue7978 test has all of the infrastructure for testing
this except that it's currently up to chance whether a stack growth
happens in the callback (it currently only happens on the
linux-amd64-noopt builder, for example). Update this test to force a
stack growth, which causes it to fail reliably without this fix.
Fixes #17785.
Change-Id: If706963819ee7814e6705693247bcb97a6f7adb8
Reviewed-on: https://go-review.googlesource.com/33710
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
139 lines
3.6 KiB
Go
139 lines
3.6 KiB
Go
// Copyright 2014 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.
|
|
|
|
// Issue 7978. Stack tracing didn't work during cgo code after calling a Go
|
|
// callback. Make sure GC works and the stack trace is correct.
|
|
|
|
package cgotest
|
|
|
|
/*
|
|
#include <stdint.h>
|
|
|
|
void issue7978cb(void);
|
|
|
|
#if defined(__APPLE__) && defined(__arm__)
|
|
// on Darwin/ARM, libSystem doesn't provide implementation of the __sync_fetch_and_add
|
|
// primitive, and although gcc supports it, it doesn't inline its definition.
|
|
// Clang could inline its definition, so we require clang on Darwin/ARM.
|
|
#if defined(__clang__)
|
|
#define HAS_SYNC_FETCH_AND_ADD 1
|
|
#else
|
|
#define HAS_SYNC_FETCH_AND_ADD 0
|
|
#endif
|
|
#else
|
|
#define HAS_SYNC_FETCH_AND_ADD 1
|
|
#endif
|
|
|
|
// use ugly atomic variable sync since that doesn't require calling back into
|
|
// Go code or OS dependencies
|
|
static void issue7978c(uint32_t *sync) {
|
|
#if HAS_SYNC_FETCH_AND_ADD
|
|
while(__sync_fetch_and_add(sync, 0) != 0)
|
|
;
|
|
__sync_fetch_and_add(sync, 1);
|
|
while(__sync_fetch_and_add(sync, 0) != 2)
|
|
;
|
|
issue7978cb();
|
|
__sync_fetch_and_add(sync, 1);
|
|
while(__sync_fetch_and_add(sync, 0) != 6)
|
|
;
|
|
#endif
|
|
}
|
|
*/
|
|
import "C"
|
|
|
|
import (
|
|
"os"
|
|
"runtime"
|
|
"strings"
|
|
"sync/atomic"
|
|
"testing"
|
|
)
|
|
|
|
var issue7978sync uint32
|
|
|
|
func issue7978check(t *testing.T, wantFunc string, badFunc string, depth int) {
|
|
runtime.GC()
|
|
buf := make([]byte, 65536)
|
|
trace := string(buf[:runtime.Stack(buf, true)])
|
|
for _, goroutine := range strings.Split(trace, "\n\n") {
|
|
if strings.Contains(goroutine, "test.issue7978go") {
|
|
trace := strings.Split(goroutine, "\n")
|
|
// look for the expected function in the stack
|
|
for i := 0; i < depth; i++ {
|
|
if badFunc != "" && strings.Contains(trace[1+2*i], badFunc) {
|
|
t.Errorf("bad stack: found %s in the stack:\n%s", badFunc, goroutine)
|
|
return
|
|
}
|
|
if strings.Contains(trace[1+2*i], wantFunc) {
|
|
return
|
|
}
|
|
}
|
|
t.Errorf("bad stack: didn't find %s in the stack:\n%s", wantFunc, goroutine)
|
|
return
|
|
}
|
|
}
|
|
t.Errorf("bad stack: goroutine not found. Full stack dump:\n%s", trace)
|
|
}
|
|
|
|
func issue7978wait(store uint32, wait uint32) {
|
|
if store != 0 {
|
|
atomic.StoreUint32(&issue7978sync, store)
|
|
}
|
|
for atomic.LoadUint32(&issue7978sync) != wait {
|
|
runtime.Gosched()
|
|
}
|
|
}
|
|
|
|
//export issue7978cb
|
|
func issue7978cb() {
|
|
// Force a stack growth from the callback to put extra
|
|
// pressure on the runtime. See issue #17785.
|
|
growStack(64)
|
|
issue7978wait(3, 4)
|
|
}
|
|
|
|
func growStack(n int) int {
|
|
var buf [128]int
|
|
if n == 0 {
|
|
return 0
|
|
}
|
|
return buf[growStack(n-1)]
|
|
}
|
|
|
|
func issue7978go() {
|
|
C.issue7978c((*C.uint32_t)(&issue7978sync))
|
|
issue7978wait(7, 8)
|
|
}
|
|
|
|
func test7978(t *testing.T) {
|
|
if runtime.Compiler == "gccgo" {
|
|
t.Skip("gccgo can not do stack traces of C code")
|
|
}
|
|
if C.HAS_SYNC_FETCH_AND_ADD == 0 {
|
|
t.Skip("clang required for __sync_fetch_and_add support on darwin/arm")
|
|
}
|
|
if runtime.GOOS == "android" || runtime.GOOS == "darwin" && (runtime.GOARCH == "arm" || runtime.GOARCH == "arm64") {
|
|
t.Skip("GOTRACEBACK is not passed on to the exec wrapper")
|
|
}
|
|
if os.Getenv("GOTRACEBACK") != "2" {
|
|
t.Fatalf("GOTRACEBACK must be 2")
|
|
}
|
|
issue7978sync = 0
|
|
go issue7978go()
|
|
// test in c code, before callback
|
|
issue7978wait(0, 1)
|
|
issue7978check(t, "_Cfunc_issue7978c(", "", 1)
|
|
// test in go code, during callback
|
|
issue7978wait(2, 3)
|
|
issue7978check(t, "test.issue7978cb(", "test.issue7978go", 3)
|
|
// test in c code, after callback
|
|
issue7978wait(4, 5)
|
|
issue7978check(t, "_Cfunc_issue7978c(", "_cgoexpwrap", 1)
|
|
// test in go code, after return from cgo
|
|
issue7978wait(6, 7)
|
|
issue7978check(t, "test.issue7978go(", "", 3)
|
|
atomic.StoreUint32(&issue7978sync, 8)
|
|
}
|