libgo patch committed: Avoid write barriers with traceback info

Message ID CAOyqgcXZfQwp9aM+AALVAK232aU=40LfbGQOi4u4TV8X24abGw@mail.gmail.com
State New
Headers show
Series
  • libgo patch committed: Avoid write barriers with traceback info
Related show

Commit Message

Ian Lance Taylor Sept. 13, 2018, 5:30 p.m.
This patch to libgo's runtime package avoids write barriers with traceback info.

Unlike the gc runtime, libgo stores traceback information in location
structs, which contain strings.  Therefore, copying location structs
around appears to require write barriers, although in fact write
barriers are never important because the strings are never allocated
in Go memory.  They come from libbacktrace.

Some of the generated write barriers come at times when write barriers
are not permitted.  For example, exitsyscall, marked
nowritebarrierrec, calls exitsyscallfast which calls traceGoSysExit
which calls traceEvent which calls traceStackID which calls
trace.stackTab.put which copies location values into memory allocated
by tab.newStack.  This write barrier can be invoked when there is no
p, causing a crash.

This change fixes the problem by ensuring that location values are
copied around in the tracing code with no write barriers.

This was found by fixing the compiler to fully implement
//go:nowritebarrierrec; patch to follow.

Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
to mainline.

Ian

Patch

Index: gcc/go/gofrontend/MERGE
===================================================================
--- gcc/go/gofrontend/MERGE	(revision 264276)
+++ gcc/go/gofrontend/MERGE	(working copy)
@@ -1,4 +1,4 @@ 
-70bd9801911f8ed27df410d36a928166c37a68fd
+baf07c40960dc4f8df9da97281870d80d4245b18
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/go/runtime/proc.go
===================================================================
--- libgo/go/runtime/proc.go	(revision 264276)
+++ libgo/go/runtime/proc.go	(working copy)
@@ -1140,7 +1140,7 @@  func startTheWorldWithSema(emitTraceEven
 func kickoff() {
 	gp := getg()
 
-	if gp.traceback != nil {
+	if gp.traceback != 0 {
 		gtraceback(gp)
 	}
 
@@ -3097,7 +3097,7 @@  func newproc(fn uintptr, arg unsafe.Poin
 	} else {
 		resetNewG(newg, &sp, &spsize)
 	}
-	newg.traceback = nil
+	newg.traceback = 0
 
 	if readgstatus(newg) != _Gdead {
 		throw("newproc1: new g is not Gdead")
Index: libgo/go/runtime/runtime2.go
===================================================================
--- libgo/go/runtime/runtime2.go	(revision 264245)
+++ libgo/go/runtime/runtime2.go	(working copy)
@@ -431,7 +431,7 @@  type g struct {
 
 	isSystemGoroutine bool // whether goroutine is a "system" goroutine
 
-	traceback *tracebackg // stack traceback buffer
+	traceback uintptr // stack traceback buffer
 
 	context      g_ucontext_t // saved context for setcontext
 	stackcontext [10]uintptr  // split-stack context
Index: libgo/go/runtime/trace.go
===================================================================
--- libgo/go/runtime/trace.go	(revision 264245)
+++ libgo/go/runtime/trace.go	(working copy)
@@ -135,6 +135,7 @@  var trace struct {
 }
 
 // traceBufHeader is per-P tracing buffer.
+//go:notinheap
 type traceBufHeader struct {
 	link      traceBufPtr              // in trace.empty/full
 	lastTicks uint64                   // when we wrote the last event
@@ -747,7 +748,8 @@  func (tab *traceStackTable) put(pcs []lo
 	stk.n = len(pcs)
 	stkpc := stk.stack()
 	for i, pc := range pcs {
-		stkpc[i] = pc
+		// Use memmove to avoid write barrier.
+		memmove(unsafe.Pointer(&stkpc[i]), unsafe.Pointer(&pc), unsafe.Sizeof(pc))
 	}
 	part := int(hash % uintptr(len(tab.tab)))
 	stk.link = tab.tab[part]
Index: libgo/go/runtime/traceback_gccgo.go
===================================================================
--- libgo/go/runtime/traceback_gccgo.go	(revision 264245)
+++ libgo/go/runtime/traceback_gccgo.go	(working copy)
@@ -186,7 +186,7 @@  func tracebackothers(me *g) {
 	if gp != nil && gp != me {
 		print("\n")
 		goroutineheader(gp)
-		gp.traceback = (*tracebackg)(noescape(unsafe.Pointer(&tb)))
+		gp.traceback = (uintptr)(noescape(unsafe.Pointer(&tb)))
 		getTraceback(me, gp)
 		printtrace(tb.locbuf[:tb.c], nil)
 		printcreatedby(gp)
@@ -220,7 +220,7 @@  func tracebackothers(me *g) {
 			print("\tgoroutine in C code; stack unavailable\n")
 			printcreatedby(gp)
 		} else {
-			gp.traceback = (*tracebackg)(noescape(unsafe.Pointer(&tb)))
+			gp.traceback = (uintptr)(noescape(unsafe.Pointer(&tb)))
 			getTraceback(me, gp)
 			printtrace(tb.locbuf[:tb.c], nil)
 			printcreatedby(gp)
Index: libgo/runtime/proc.c
===================================================================
--- libgo/runtime/proc.c	(revision 264245)
+++ libgo/runtime/proc.c	(working copy)
@@ -338,7 +338,7 @@  runtime_mcall(FuncVal *fv)
 		gp = runtime_g();
 		mp = gp->m;
 
-		if(gp->traceback != nil)
+		if(gp->traceback != 0)
 			gtraceback(gp);
 	}
 	if (gp == nil || !gp->fromgogo) {
@@ -443,7 +443,7 @@  void getTraceback(G* me, G* gp)
 #endif
 	getcontext(ucontext_arg(&me->context[0]));
 
-	if (gp->traceback != nil) {
+	if (gp->traceback != 0) {
 		runtime_gogo(gp);
 	}
 }
@@ -457,8 +457,8 @@  gtraceback(G* gp)
 	Traceback* traceback;
 	M* holdm;
 
-	traceback = gp->traceback;
-	gp->traceback = nil;
+	traceback = (Traceback*)gp->traceback;
+	gp->traceback = 0;
 	holdm = gp->m;
 	if(holdm != nil && holdm != g->m)
 		runtime_throw("gtraceback: m is not nil");
@@ -510,7 +510,7 @@  runtime_mstart(void *arg)
 	// multiple times via the setcontext call in mcall.
 	getcontext(ucontext_arg(&gp->context[0]));
 
-	if(gp->traceback != nil) {
+	if(gp->traceback != 0) {
 		// Got here from getTraceback.
 		// I'm not sure this ever actually happens--getTraceback
 		// may always go to the getcontext call in mcall.