libgo patch committed: In syscall save all regs for GC

[Ian Lance Taylor <iant at google dot com>]

This patch to libgo fixes a rare but serious bug.  The Go garbage
collector only examines Go stacks.  When Go code calls a function that
is not written in Go, it first calls syscall.Entersyscall.  Entersyscall
records the position of the Go stack pointer and saves a copy of all the
registers.  If the garbage collector runs while the thread is executing
the non-Go code, the garbage collector fetches the stack pointer and
registers from the saved location.

Entersyscall saves the registers using the getcontext function.
Unfortunately I didn't consider the possibility that Entersyscall might
itself change a register before calling getcontext.  This only matters
for callee-saved registers, as caller-saved registers would be visible
on the saved stack.  And it only matters if Entersyscall is compiled to
save and modify a callee-saved register before it calls getcontext.  And
it only matters if a garbage collection occurs while the non-Go code is
executing.  And it only matters if the only copy of a valid Go pointer
happens to be in the callee-saved register when Entersyscall is called.
When all those conditions are true, the Go pointer might get collected
incorrectly, leading to memory corruption.

This patch tries to avoid the problem by splitting Entersyscall into two
functions.  The first is a simple function that just calls getcontext
and then calls the rest of Entersyscall.  This should fix the problem,
provided the simple Entersyscall function does not itself modify any
callee-saved registers before calling getcontext.  That seems to be true
on the systems I checked.  But since the argument to getcontext is an
offset from a TLS variable, it won't be true on a system which needs to
save callee-saved registers in order to get the address of a TLS
variable.  I don't know why any system would work that way, but I don't
know how to rule it out.  I think that on any such system this will have
to be implemented in assembler.  I can't put the ucontext_t structure on
the stack, because this function can not split stacks, and the
ucontext_t structure is large enough that it could cause a stack
overflow.

For this patch I bootstrapped and ran the Go testsuite on
x86_64-unknown-linux-gnu.  Committed to mainline and 4.8 branch.

Ian

diff -r de64b19b480e libgo/runtime/proc.c
--- a/libgo/runtime/proc.c	Thu Mar 06 16:00:18 2014 -0800
+++ b/libgo/runtime/proc.c	Thu Mar 06 20:55:49 2014 -0800
@@ -1857,10 +1857,30 @@
 // entersyscall is going to return immediately after.
 
 void runtime_entersyscall(void) __attribute__ ((no_split_stack));
+static void doentersyscall(void) __attribute__ ((no_split_stack, noinline));
 
 void
 runtime_entersyscall()
 {
+	// Save the registers in the g structure so that any pointers
+	// held in registers will be seen by the garbage collector.
+	getcontext(&g->gcregs);
+
+	// Do the work in a separate function, so that this function
+	// doesn't save any registers on its own stack.  If this
+	// function does save any registers, we might store the wrong
+	// value in the call to getcontext.
+	//
+	// FIXME: This assumes that we do not need to save any
+	// callee-saved registers to access the TLS variable g.  We
+	// don't want to put the ucontext_t on the stack because it is
+	// large and we can not split the stack here.
+	doentersyscall();
+}
+
+static void
+doentersyscall()
+{
 	// Disable preemption because during this function g is in Gsyscall status,
 	// but can have inconsistent g->sched, do not let GC observe it.
 	m->locks++;
@@ -1878,10 +1898,6 @@
 	}
 #endif
 
-	// Save the registers in the g structure so that any pointers
-	// held in registers will be seen by the garbage collector.
-	getcontext(&g->gcregs);
-
 	g->status = Gsyscall;
 
 	if(runtime_atomicload(&runtime_sched.sysmonwait)) {  // TODO: fast atomic