From 19d4baed5721eb7c95e56c2bec68ae7db6308f9e Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Wed, 8 Oct 2014 14:03:13 +0000 Subject: [PATCH] re PR go/60406 (recover.go: test13reflect2 test failure) PR go/60406 runtime: Check callers in can_recover if return address doesn't match. Also use __builtin_extract_return_address and tighten up the checks in FFI code. Fixes PR 60406. From-SVN: r216003 --- libgo/go/reflect/makefunc_ffi_c.c | 27 ++-- libgo/runtime/go-defer.c | 2 +- libgo/runtime/go-panic.h | 7 +- libgo/runtime/go-recover.c | 233 +++++++++++++++++++++++------- libgo/runtime/panic.c | 6 +- 5 files changed, 197 insertions(+), 78 deletions(-) diff --git a/libgo/go/reflect/makefunc_ffi_c.c b/libgo/go/reflect/makefunc_ffi_c.c index fba269dcb58..a3dfd4acd2c 100644 --- a/libgo/go/reflect/makefunc_ffi_c.c +++ b/libgo/go/reflect/makefunc_ffi_c.c @@ -36,21 +36,23 @@ void ffiFree(void *data) Go callback function (passed in user_data) with the pointer to the arguments and the results area. */ +static void ffi_callback (ffi_cif *, void *, void **, void *) + __asm__ ("reflect.ffi_callback"); + static void ffi_callback (ffi_cif* cif __attribute__ ((unused)), void *results, void **args, void *user_data) { - Location locs[6]; + Location locs[8]; int n; int i; - const void *pc; FuncVal *fv; void (*f) (void *, void *); /* This function is called from some series of FFI closure functions - called by a Go function. We want to pass the PC of the Go - function to makefunc_can_recover. Look up the stack for a - function that is definitely not an FFI function. */ + called by a Go function. We want to see whether the caller of + the closure functions can recover. Look up the stack and skip + the FFI functions. */ n = runtime_callers (1, &locs[0], sizeof locs / sizeof locs[0], true); for (i = 0; i < n; i++) { @@ -61,28 +63,19 @@ ffi_callback (ffi_cif* cif __attribute__ ((unused)), void *results, if (locs[i].function.len < 4) break; name = locs[i].function.str; - if (*name == '_') - { - if (locs[i].function.len < 5) - break; - ++name; - } if (name[0] != 'f' || name[1] != 'f' || name[2] != 'i' || name[3] != '_') break; } if (i < n) - pc = (const void *) locs[i].pc; - else - pc = __builtin_return_address (0); - - __go_makefunc_can_recover (pc); + __go_makefunc_ffi_can_recover (locs + i, n - i); fv = (FuncVal *) user_data; __go_set_closure (fv); f = (void *) fv->fn; f (args, results); - __go_makefunc_returning (); + if (i < n) + __go_makefunc_returning (); } /* Allocate an FFI closure and arrange to call ffi_callback. */ diff --git a/libgo/runtime/go-defer.c b/libgo/runtime/go-defer.c index 5dd8c3105f7..3a48fe1130e 100644 --- a/libgo/runtime/go-defer.c +++ b/libgo/runtime/go-defer.c @@ -80,6 +80,6 @@ __go_set_defer_retaddr (void *retaddr) g = runtime_g (); if (g->defer != NULL) - g->defer->__retaddr = retaddr; + g->defer->__retaddr = __builtin_extract_return_addr (retaddr); return 0; } diff --git a/libgo/runtime/go-panic.h b/libgo/runtime/go-panic.h index bcaa7e1ee38..d29fe88b57a 100644 --- a/libgo/runtime/go-panic.h +++ b/libgo/runtime/go-panic.h @@ -38,9 +38,12 @@ extern void __go_print_string (struct String); extern struct __go_empty_interface __go_recover (void); -extern _Bool __go_can_recover (const void *); +extern _Bool __go_can_recover (void *); -extern void __go_makefunc_can_recover (const void *retaddr); +extern void __go_makefunc_can_recover (void *retaddr); + +struct Location; +extern void __go_makefunc_ffi_can_recover (struct Location *, int); extern void __go_makefunc_returning (void); diff --git a/libgo/runtime/go-recover.c b/libgo/runtime/go-recover.c index 2d3db55cbc9..fc66f61cab3 100644 --- a/libgo/runtime/go-recover.c +++ b/libgo/runtime/go-recover.c @@ -9,6 +9,36 @@ #include "go-panic.h" #include "go-defer.h" +/* If the top of the defer stack can be recovered, then return it. + Otherwise return NULL. */ + +static struct __go_defer_stack * +current_defer () +{ + G *g; + struct __go_defer_stack *d; + + g = runtime_g (); + + d = g->defer; + if (d == NULL) + return NULL; + + /* The panic which would be recovered is the one on the top of the + panic stack. We do not want to recover it if that panic was on + the top of the panic stack when this function was deferred. */ + if (d->__panic == g->panic) + return NULL; + + /* The deferred thunk will call _go_set_defer_retaddr. If this has + not happened, then we have not been called via defer, and we can + not recover. */ + if (d->__retaddr == NULL) + return NULL; + + return d; +} + /* This is called by a thunk to see if the real function should be permitted to recover a panic value. Recovering a value is permitted if the thunk was called directly by defer. RETADDR is @@ -16,79 +46,126 @@ __go_can_recover--this is, the thunk. */ _Bool -__go_can_recover (const void *retaddr) +__go_can_recover (void *retaddr) { - G *g; struct __go_defer_stack *d; const char* ret; const char* dret; - Location loc; + Location locs[16]; const byte *name; + intgo len; + int n; + int i; + _Bool found_ffi_callback; - g = runtime_g (); - - d = g->defer; + d = current_defer (); if (d == NULL) return 0; - /* The panic which this function would recover is the one on the top - of the panic stack. We do not want to recover it if that panic - was on the top of the panic stack when this function was - deferred. */ - if (d->__panic == g->panic) - return 0; - - /* D->__RETADDR is the address of a label immediately following the - call to the thunk. We can recover a panic if that is the same as - the return address of the thunk. We permit a bit of slack in - case there is any code between the function return and the label, - such as an instruction to adjust the stack pointer. */ - - ret = (const char *) retaddr; - -#ifdef __sparc__ - /* On SPARC the address we get, from __builtin_return_address, is - the address of the call instruction. Adjust forward, also - skipping the delayed instruction following the call. */ - ret += 8; -#endif + ret = (const char *) __builtin_extract_return_addr (retaddr); dret = (const char *) d->__retaddr; if (ret <= dret && ret + 16 >= dret) return 1; + /* On some systems, in some cases, the return address does not work + reliably. See http://gcc.gnu.org/PR60406. If we are permitted + to call recover, the call stack will look like this: + __go_panic, __go_undefer, etc. + thunk to call deferred function (calls __go_set_defer_retaddr) + function that calls __go_can_recover (passing return address) + __go_can_recover + Calling runtime_callers will skip the thunks. So if our caller's + caller starts with __go, then we are permitted to call + recover. */ + + if (runtime_callers (1, &locs[0], 2, false) < 2) + return 0; + + name = locs[1].function.str; + len = locs[1].function.len; + + /* Although locs[1].function is a Go string, we know it is + NUL-terminated. */ + if (len > 4 + && __builtin_strchr ((const char *) name, '.') == NULL + && __builtin_strncmp ((const char *) name, "__go_", 4) == 0) + return 1; + + /* If we are called from __go_makefunc_can_recover, then we need to + look one level higher. */ + if (locs[0].function.len > 0 + && __builtin_strcmp ((const char *) locs[0].function.str, + "__go_makefunc_can_recover") == 0) + { + if (runtime_callers (3, &locs[0], 1, false) < 1) + return 0; + name = locs[0].function.str; + len = locs[0].function.len; + if (len > 4 + && __builtin_strchr ((const char *) name, '.') == NULL + && __builtin_strncmp ((const char *) name, "__go_", 4) == 0) + 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. */ + then __go_makefunc_can_recover or __go_makefunc_ffi_can_recover + will have set the __makefunc_can_recover field. */ if (!d->__makefunc_can_recover) return 0; - if (runtime_callers (2, &loc, 1, false) < 1) - return 0; + /* We look up the stack, ignoring libffi functions and functions in + the reflect package, until we find reflect.makeFuncStub or + reflect.ffi_callback called by FFI functions. Then we check the + caller of that function. */ - /* 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 == '_') + n = runtime_callers (2, &locs[0], sizeof locs / sizeof locs[0], false); + found_ffi_callback = 0; + for (i = 0; i < n; i++) { - if (loc.function.len < 5) - return 0; - ++name; + const byte *name; + + if (locs[i].function.len == 0) + { + /* No function name means this caller isn't Go code. Assume + that this is libffi. */ + continue; + } + + /* Ignore functions in libffi. */ + name = locs[i].function.str; + if (__builtin_strncmp ((const char *) name, "ffi_", 4) == 0) + continue; + + if (found_ffi_callback) + break; + + if (__builtin_strcmp ((const char *) name, "reflect.ffi_callback") == 0) + { + found_ffi_callback = 1; + continue; + } + + if (__builtin_strcmp ((const char *) name, "reflect.makeFuncStub") == 0) + { + i++; + break; + } + + /* Ignore other functions in the reflect package. */ + if (__builtin_strncmp ((const char *) name, "reflect.", 8) == 0) + continue; + + /* We should now be looking at the real caller. */ + break; } - if (name[0] == 'f' && name[1] == 'f' && name[2] == 'i' && name[3] == '_') - return 1; - - /* We may also be called by reflect.makeFuncImpl.call or - reflect.ffiCall, for a function created by reflect.MakeFunc. */ - if (__builtin_strstr ((const char *) name, "makeFuncImpl") != NULL - || __builtin_strcmp ((const char *) name, "reflect.ffiCall") == 0) - return 1; + if (i < n && locs[i].function.len > 0) + { + name = locs[i].function.str; + if (__builtin_strncmp ((const char *) name, "__go_", 4) == 0) + return 1; + } return 0; } @@ -99,14 +176,58 @@ __go_can_recover (const void *retaddr) real MakeFunc function is permitted to call recover. */ void -__go_makefunc_can_recover (const void *retaddr) +__go_makefunc_can_recover (void *retaddr) { struct __go_defer_stack *d; - d = runtime_g ()->defer; - if (d != NULL - && !d->__makefunc_can_recover - && __go_can_recover (retaddr)) + d = current_defer (); + if (d == NULL) + return; + + /* If we are already in a call stack of MakeFunc functions, there is + nothing we can usefully check here. */ + if (d->__makefunc_can_recover) + return; + + if (__go_can_recover (retaddr)) + d->__makefunc_can_recover = 1; +} + +/* This function is called when code is about to enter a function + created by the libffi version of reflect.MakeFunc. This function + is passed the names of the callers of the libffi code that called + the stub. It uses to decide whether it is permitted to call + recover, and sets d->__makefunc_can_recover so that __go_recover + can make the same decision. */ + +void +__go_makefunc_ffi_can_recover (struct Location *loc, int n) +{ + struct __go_defer_stack *d; + const byte *name; + intgo len; + + d = current_defer (); + if (d == NULL) + return; + + /* If we are already in a call stack of MakeFunc functions, there is + nothing we can usefully check here. */ + if (d->__makefunc_can_recover) + return; + + /* LOC points to the caller of our caller. That will be a thunk. + If its caller was a runtime function, then it was called directly + by defer. */ + + if (n < 2) + return; + + name = (loc + 1)->function.str; + len = (loc + 1)->function.len; + if (len > 4 + && __builtin_strchr ((const char *) name, '.') == NULL + && __builtin_strncmp ((const char *) name, "__go_", 4) == 0) d->__makefunc_can_recover = 1; } diff --git a/libgo/runtime/panic.c b/libgo/runtime/panic.c index 6a5d007f03c..de000db9889 100644 --- a/libgo/runtime/panic.c +++ b/libgo/runtime/panic.c @@ -49,8 +49,10 @@ runtime_freedefer(Defer *d) } // Run all deferred functions for the current goroutine. +// This is noinline for go_can_recover. +static void __go_rundefer (void) __attribute__ ((noinline)); static void -rundefer(void) +__go_rundefer(void) { G *g; Defer *d; @@ -219,7 +221,7 @@ void runtime_Goexit (void) __asm__ (GOSYM_PREFIX "runtime.Goexit"); void runtime_Goexit(void) { - rundefer(); + __go_rundefer(); runtime_goexit(); }