diff mbox

libgo patch committed: Let MakeFunc functions call recover

Message ID mcr1u1jj6mt.fsf@iant-glaptop.roam.corp.google.com
State New
Headers show

Commit Message

Ian Lance Taylor Dec. 11, 2013, 11:43 p.m. UTC 
The only Go functions that may call recover are those that are
immediately deferred.  This permits the correct handling of a panic in a
function called by a deferred function.  In gccgo this is implemented by
recording the return address of a function that may call recover, and
checking that return address in the recover call.  That works for normal
functions, but fails for functions created by reflect.MakeFunc.  For a
MakeFunc function the caller will be some function from libffi.  This
patch lets that work correctly by hooking into the stub used to start
functions created by reflect.MakeFunc.  That stub now does the checking
for whether the function may call recover, and the function itself
checks whether it was called directly from libffi.  Bootstrapped and ran
Go testsuite on x86_64-unknown-linux-gnu.  Committed to mainline and 4.8
branch.

Ian
diff mbox

Patch

diff -r 4863a5e8f8a3 libgo/go/reflect/makefunc_386.S
--- a/libgo/go/reflect/makefunc_386.S	Wed Dec 11 14:34:28 2013 -0800
+++ b/libgo/go/reflect/makefunc_386.S	Wed Dec 11 15:35:56 2013 -0800
@@ -48,6 +48,15 @@ 
 	leal	8(%ebp), %eax	/* Set esp field in struct.  */
 	movl	%eax, -24(%ebp)
 
+	/* For MakeFunc functions that call recover.  */
+	movl	4(%ebp), %eax
+	movl	%eax, (%esp)
+#ifdef __PIC__
+	call	__go_makefunc_can_recover@PLT
+#else
+	call	__go_makefunc_can_recover
+#endif
+
 #ifdef __PIC__
 	call	__go_get_closure@PLT
 #else
@@ -65,6 +74,13 @@ 
 	call	reflect.MakeFuncStubGo
 #endif
 
+	/* MakeFunc functions can no longer call recover.  */
+#ifdef __PIC__
+	call __go_makefunc_returning@PLT
+#else
+	call __go_makefunc_returning
+#endif
+
 	/* Set return registers.  */
 
 	movl	-20(%ebp), %eax
diff -r 4863a5e8f8a3 libgo/go/reflect/makefunc_amd64.S
--- a/libgo/go/reflect/makefunc_amd64.S	Wed Dec 11 14:34:28 2013 -0800
+++ b/libgo/go/reflect/makefunc_amd64.S	Wed Dec 11 15:35:56 2013 -0800
@@ -61,6 +61,14 @@ 
 	movdqa	%xmm6, 0xa0(%rsp)
 	movdqa	%xmm7, 0xb0(%rsp)
 
+	/* For MakeFunc functions that call recover.  */
+	movq	8(%rbp), %rdi
+#ifdef __PIC__
+	call	__go_makefunc_can_recover@PLT
+#else
+	call	__go_makefunc_can_recover
+#endif
+
 	# Get function type.
 #ifdef __PIC__
 	call	__go_get_closure@PLT
@@ -77,6 +85,13 @@ 
 	call	reflect.MakeFuncStubGo
 #endif
 
+	/* MakeFunc functions can no longer call recover.  */
+#ifdef __PIC__
+	call __go_makefunc_returning@PLT
+#else
+	call __go_makefunc_returning
+#endif
+
 	# The structure will be updated with any return values.  Load
 	# all possible return registers before returning to the caller.
 
diff -r 4863a5e8f8a3 libgo/runtime/go-defer.c
--- a/libgo/runtime/go-defer.c	Wed Dec 11 14:34:28 2013 -0800
+++ b/libgo/runtime/go-defer.c	Wed Dec 11 15:35:56 2013 -0800
@@ -27,6 +27,7 @@ 
   n->__pfn = pfn;
   n->__arg = arg;
   n->__retaddr = NULL;
+  n->__makefunc_can_recover = 0;
   g->defer = n;
 }
 
diff -r 4863a5e8f8a3 libgo/runtime/go-defer.h
--- a/libgo/runtime/go-defer.h	Wed Dec 11 14:34:28 2013 -0800
+++ b/libgo/runtime/go-defer.h	Wed Dec 11 15:35:56 2013 -0800
@@ -34,4 +34,10 @@ 
      set by __go_set_defer_retaddr which is called by the thunks
      created by defer statements.  */
   const void *__retaddr;
+
+  /* Set to true if a function created by reflect.MakeFunc is
+     permitted to recover.  The return address of such a function
+     function will be somewhere in libffi, so __retaddr is not
+     useful.  */
+  _Bool __makefunc_can_recover;
 };
diff -r 4863a5e8f8a3 libgo/runtime/go-recover.c
--- a/libgo/runtime/go-recover.c	Wed Dec 11 14:34:28 2013 -0800
+++ b/libgo/runtime/go-recover.c	Wed Dec 11 15:35:56 2013 -0800
@@ -16,12 +16,14 @@ 
    __go_can_recover--this is, the thunk.  */
 
 _Bool
-__go_can_recover (const void* retaddr)
+__go_can_recover (const void *retaddr)
 {
   G *g;
   struct __go_defer_stack *d;
   const char* ret;
   const char* dret;
+  Location loc;
+  const byte *name;
 
   g = runtime_g ();
 
@@ -52,7 +54,73 @@ 
 #endif
 
   dret = (const char *) d->__retaddr;
-  return ret <= dret && ret + 16 >= dret;
+  if (ret <= dret && ret + 16 >= dret)
+    return 1;
+
+  /* If the function calling recover was created by reflect.MakeFunc,
+     then RETADDR will be somewhere in libffi.  Our caller is
+     permitted to recover if it was called from libffi.  */
+  if (!d->__makefunc_can_recover)
+    return 0;
+
+  if (runtime_callers (2, &loc, 1) < 1)
+    return 0;
+
+  /* If we have no function name, then we weren't called by Go code.
+     Guess that we were called by libffi.  */
+  if (loc.function.len == 0)
+    return 1;
+
+  if (loc.function.len < 4)
+    return 0;
+  name = loc.function.str;
+  if (*name == '_')
+    {
+      if (loc.function.len < 5)
+	return 0;
+      ++name;
+    }
+
+  if (name[0] == 'f' && name[1] == 'f' && name[2] == 'i' && name[3] == '_')
+    return 1;
+
+  return 0;
+}
+
+/* This function is called when code is about to enter a function
+   created by reflect.MakeFunc.  It is called by the function stub
+   used by MakeFunc.  If the stub is permitted to call recover, then a
+   real MakeFunc function is permitted to call recover.  */
+
+void
+__go_makefunc_can_recover (const void *retaddr)
+{
+  struct __go_defer_stack *d;
+
+  d = runtime_g ()->defer;
+  if (d != NULL
+      && !d->__makefunc_can_recover
+      && __go_can_recover (retaddr))
+    d->__makefunc_can_recover = 1;
+}
+
+/* This function is called when code is about to exit a function
+   created by reflect.MakeFunc.  It is called by the function stub
+   used by MakeFunc.  It clears the __makefunc_can_recover field.
+   It's OK to always clear this field, because __go_can_recover will
+   only be called by a stub created for a function that calls recover.
+   That stub will not call a function created by reflect.MakeFunc, so
+   by the time we get here any caller higher up on the call stack no
+   longer needs the information.  */
+
+void
+__go_makefunc_returning (void)
+{
+  struct __go_defer_stack *d;
+
+  d = runtime_g ()->defer;
+  if (d != NULL)
+    d->__makefunc_can_recover = 0;
 }
 
 /* This is only called when it is valid for the caller to recover the
diff -r 4863a5e8f8a3 libgo/runtime/proc.c
--- a/libgo/runtime/proc.c	Wed Dec 11 14:34:28 2013 -0800
+++ b/libgo/runtime/proc.c	Wed Dec 11 15:35:56 2013 -0800
@@ -539,6 +539,7 @@ 
 	d.__arg = (void*)-1;
 	d.__panic = g->panic;
 	d.__retaddr = nil;
+	d.__makefunc_can_recover = 0;
 	d.__frame = &frame;
 	g->defer = &d;