[dev.garbage] cmd/gc, runtime: add locks around print statements

Now each C printf, Go print, or Go println is guaranteed
not to be interleaved with other calls of those functions.
This should help when debugging concurrent failures.

LGTM=rlh
R=rlh
CC=golang-codereviews
https://codereview.appspot.com/169120043

6 files changed, 50 insertions, 3 deletions

diff --git a/src/cmd/gc/builtin.c b/src/cmd/gc/builtin.c
index bd3fca167..aeeadedca 100644
--- a/src/cmd/gc/builtin.c
+++ b/src/cmd/gc/builtin.c
@@ -24,6 +24,8 @@ char *runtimeimport =
 	"func @\"\".printslice (? any)\n"
 	"func @\"\".printnl ()\n"
 	"func @\"\".printsp ()\n"
+	"func @\"\".printlock ()\n"
+	"func @\"\".printunlock ()\n"
 	"func @\"\".concatstring2 (? string, ? string) (? string)\n"
 	"func @\"\".concatstring3 (? string, ? string, ? string) (? string)\n"
 	"func @\"\".concatstring4 (? string, ? string, ? string, ? string) (? string)\n"
diff --git a/src/cmd/gc/go.h b/src/cmd/gc/go.h
index 965a0550d..cc590416b 100644
--- a/src/cmd/gc/go.h
+++ b/src/cmd/gc/go.h
@@ -1464,6 +1464,7 @@ void	walk(Node *fn);
 void	walkexpr(Node **np, NodeList **init);
 void	walkexprlist(NodeList *l, NodeList **init);
 void	walkexprlistsafe(NodeList *l, NodeList **init);
+void	walkexprlistcheap(NodeList *l, NodeList **init);
 void	walkstmt(Node **np);
 void	walkstmtlist(NodeList *l);
 Node*	conv(Node*, Type*);
diff --git a/src/cmd/gc/runtime.go b/src/cmd/gc/runtime.go
index 38bf6abb6..c6007714c 100644
--- a/src/cmd/gc/runtime.go
+++ b/src/cmd/gc/runtime.go
@@ -36,6 +36,8 @@ func printeface(any)
 func printslice(any)
 func printnl()
 func printsp()
+func printlock()
+func printunlock()
 
 func concatstring2(string, string) string
 func concatstring3(string, string, string) string
diff --git a/src/cmd/gc/walk.c b/src/cmd/gc/walk.c
index 7b502eb60..38bed1e22 100644
--- a/src/cmd/gc/walk.c
+++ b/src/cmd/gc/walk.c
@@ -364,6 +364,15 @@ walkexprlistsafe(NodeList *l, NodeList **init)
 }
 
 void
+walkexprlistcheap(NodeList *l, NodeList **init)
+{
+	for(; l; l=l->next) {
+		l->n = cheapexpr(l->n, init);
+		walkexpr(&l->n, init);
+	}
+}
+
+void
 walkexpr(Node **np, NodeList **init)
 {
 	Node *r, *l, *var, *a;
@@ -1773,6 +1782,11 @@ walkprint(Node *nn, NodeList **init)
 	calls = nil;
 	notfirst = 0;
 
+	// Hoist all the argument evaluation up before the lock.
+	walkexprlistcheap(all, init);
+
+	calls = list(calls, mkcall("printlock", T, init));
+
 	for(l=all; l; l=l->next) {
 		if(notfirst) {
 			calls = list(calls, mkcall("printsp", T, init));
@@ -1853,6 +1867,9 @@ walkprint(Node *nn, NodeList **init)
 
 	if(op == OPRINTN)
 		calls = list(calls, mkcall("printnl", T, nil));
+
+	calls = list(calls, mkcall("printunlock", T, init));
+
 	typechecklist(calls, Etop);
 	walkexprlist(calls, init);
 
diff --git a/src/runtime/print1.go b/src/runtime/print1.go
index 8f8268873..3d812bd04 100644
--- a/src/runtime/print1.go
+++ b/src/runtime/print1.go
@@ -41,7 +41,31 @@ func snprintf(dst *byte, n int32, s *byte) {
 	gp.writebuf = nil
 }
 
-//var debuglock mutex
+var debuglock mutex
+
+// The compiler emits calls to printlock and printunlock around
+// the multiple calls that implement a single Go print or println
+// statement. Some of the print helpers (printsp, for example)
+// call print recursively. There is also the problem of a crash
+// happening during the print routines and needing to acquire
+// the print lock to print information about the crash.
+// For both these reasons, let a thread acquire the printlock 'recursively'.
+
+func printlock() {
+	mp := getg().m
+	mp.printlock++
+	if mp.printlock == 1 {
+		lock(&debuglock)
+	}
+}
+
+func printunlock() {
+	mp := getg().m
+	mp.printlock--
+	if mp.printlock == 0 {
+		unlock(&debuglock)
+	}
+}
 
 // write to goroutine-local buffer if diverting output,
 // or else standard error.
@@ -80,7 +104,7 @@ func printnl() {
 // Very simple printf.  Only for debugging prints.
 // Do not add to this without checking with Rob.
 func vprintf(str string, arg unsafe.Pointer) {
-	//lock(&debuglock);
+	printlock()
 
 	s := bytes(str)
 	start := 0
@@ -160,7 +184,7 @@ func vprintf(str string, arg unsafe.Pointer) {
 		gwrite(s[start:i])
 	}
 
-	//unlock(&debuglock);
+	printunlock()
 }
 
 func printpc(p unsafe.Pointer) {
diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h
index 6a02ef1d3..ee86f2d17 100644
--- a/src/runtime/runtime.h
+++ b/src/runtime/runtime.h
@@ -345,6 +345,7 @@ struct	M
 	int32	helpgc;
 	bool	spinning;	// M is out of work and is actively looking for work
 	bool	blocked;	// M is blocked on a Note
+	int8	printlock;
 	uint32	fastrand;
 	uint64	ncgocall;	// number of cgo calls in total
 	int32	ncgo;		// number of cgo calls currently in progress