From 88e0ec2979bb39bd8811ec50a69fcb5007a24623 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Tue, 31 May 2016 16:04:00 -0700 Subject: [PATCH] runtime/cgo: avoid races on cgo_context_function Change-Id: Ie9e6fda675e560234e90b9022526fd689d770818 Reviewed-on: https://go-review.googlesource.com/23610 Reviewed-by: Dmitry Vyukov TryBot-Result: Gobot Gobot --- src/runtime/cgo/gcc_context.c | 16 ++++------- src/runtime/cgo/gcc_libinit.c | 39 +++++++++++++++++++++++++-- src/runtime/cgo/gcc_libinit_openbsd.c | 23 ++++++++++++++-- src/runtime/cgo/gcc_libinit_windows.c | 27 +++++++++++++++++-- src/runtime/cgo/libcgo.h | 2 +- 5 files changed, 89 insertions(+), 18 deletions(-) diff --git a/src/runtime/cgo/gcc_context.c b/src/runtime/cgo/gcc_context.c index 81556cd464..1e6cf7ee12 100644 --- a/src/runtime/cgo/gcc_context.c +++ b/src/runtime/cgo/gcc_context.c @@ -7,21 +7,15 @@ #include "libcgo.h" -// The context function, used when tracing back C calls into Go. -void (*x_cgo_context_function)(struct context_arg*); - -// Sets the context function to call to record the traceback context -// when calling a Go function from C code. Called from runtime.SetCgoTraceback. -void x_cgo_set_context_function(void (*context)(struct context_arg*)) { - x_cgo_context_function = context; -} - // Releases the cgo traceback context. void _cgo_release_context(uintptr_t ctxt) { - if (ctxt != 0 && x_cgo_context_function != nil) { + void (*pfn)(struct context_arg*); + + pfn = _cgo_get_context_function(); + if (ctxt != 0 && pfn != nil) { struct context_arg arg; arg.Context = ctxt; - (*x_cgo_context_function)(&arg); + (*pfn)(&arg); } } diff --git a/src/runtime/cgo/gcc_libinit.c b/src/runtime/cgo/gcc_libinit.c index c5b9476380..0bdf40a4ca 100644 --- a/src/runtime/cgo/gcc_libinit.c +++ b/src/runtime/cgo/gcc_libinit.c @@ -15,6 +15,9 @@ static pthread_cond_t runtime_init_cond = PTHREAD_COND_INITIALIZER; static pthread_mutex_t runtime_init_mu = PTHREAD_MUTEX_INITIALIZER; static int runtime_init_done; +// The context function, used when tracing back C calls into Go. +static void (*cgo_context_function)(struct context_arg*); + void x_cgo_sys_thread_create(void* (*func)(void*), void* arg) { pthread_t p; @@ -27,16 +30,30 @@ x_cgo_sys_thread_create(void* (*func)(void*), void* arg) { uintptr_t _cgo_wait_runtime_init_done() { + void (*pfn)(struct context_arg*); + pthread_mutex_lock(&runtime_init_mu); while (runtime_init_done == 0) { pthread_cond_wait(&runtime_init_cond, &runtime_init_mu); } + + // TODO(iant): For the case of a new C thread calling into Go, such + // as when using -buildmode=c-archive, we know that Go runtime + // initialization is complete but we do not know that all Go init + // functions have been run. We should not fetch cgo_context_function + // until they have been, because that is where a call to + // SetCgoTraceback is likely to occur. We are going to wait for Go + // initialization to be complete anyhow, later, by waiting for + // main_init_done to be closed in cgocallbackg1. We should wait here + // instead. See also issue #15943. + pfn = cgo_context_function; + pthread_mutex_unlock(&runtime_init_mu); - if (x_cgo_context_function != nil) { + if (pfn != nil) { struct context_arg arg; arg.Context = 0; - (*x_cgo_context_function)(&arg); + (*pfn)(&arg); return arg.Context; } return 0; @@ -49,3 +66,21 @@ x_cgo_notify_runtime_init_done(void* dummy) { pthread_cond_broadcast(&runtime_init_cond); pthread_mutex_unlock(&runtime_init_mu); } + +// Sets the context function to call to record the traceback context +// when calling a Go function from C code. Called from runtime.SetCgoTraceback. +void x_cgo_set_context_function(void (*context)(struct context_arg*)) { + pthread_mutex_lock(&runtime_init_mu); + cgo_context_function = context; + pthread_mutex_unlock(&runtime_init_mu); +} + +// Gets the context function. +void (*(_cgo_get_context_function(void)))(struct context_arg*) { + void (*ret)(struct context_arg*); + + pthread_mutex_lock(&runtime_init_mu); + ret = cgo_context_function; + pthread_mutex_unlock(&runtime_init_mu); + return ret; +} diff --git a/src/runtime/cgo/gcc_libinit_openbsd.c b/src/runtime/cgo/gcc_libinit_openbsd.c index 07dfcaf660..626bf8adca 100644 --- a/src/runtime/cgo/gcc_libinit_openbsd.c +++ b/src/runtime/cgo/gcc_libinit_openbsd.c @@ -6,6 +6,9 @@ #include #include "libcgo.h" +// The context function, used when tracing back C calls into Go. +static void (*cgo_context_function)(struct context_arg*); + void x_cgo_sys_thread_create(void* (*func)(void*), void* arg) { fprintf(stderr, "x_cgo_sys_thread_create not implemented"); @@ -14,12 +17,16 @@ x_cgo_sys_thread_create(void* (*func)(void*), void* arg) { uintptr_t _cgo_wait_runtime_init_done() { + void (*pfn)(struct context_arg*); + // TODO(spetrovic): implement this method. - if (x_cgo_context_function != nil) { + + pfn = _cgo_get_context_function(); + if (pfn != nil) { struct context_arg arg; arg.Context = 0; - (*x_cgo_context_function)(&arg); + (*pfn)(&arg); return arg.Context; } return 0; @@ -29,3 +36,15 @@ void x_cgo_notify_runtime_init_done(void* dummy) { // TODO(spetrovic): implement this method. } + +// Sets the context function to call to record the traceback context +// when calling a Go function from C code. Called from runtime.SetCgoTraceback. +void x_cgo_set_context_function(void (*context)(struct context_arg*)) { + // TODO(iant): Needs synchronization. + cgo_context_function = context; +} + +// Gets the context function. +void (*(_cgo_get_context_function(void)))(struct context_arg*) { + return cgo_context_function; +} diff --git a/src/runtime/cgo/gcc_libinit_windows.c b/src/runtime/cgo/gcc_libinit_windows.c index f5c306d49a..0824e20ad8 100644 --- a/src/runtime/cgo/gcc_libinit_windows.c +++ b/src/runtime/cgo/gcc_libinit_windows.c @@ -70,15 +70,18 @@ _cgo_is_runtime_initialized() { uintptr_t _cgo_wait_runtime_init_done() { + void (*pfn)(struct context_arg*); + _cgo_maybe_run_preinit(); while (!_cgo_is_runtime_initialized()) { WaitForSingleObject(runtime_init_wait, INFINITE); } - if (x_cgo_context_function != nil) { + pfn = _cgo_get_context_function(); + if (pfn != nil) { struct context_arg arg; arg.Context = 0; - (*x_cgo_context_function)(&arg); + (*pfn)(&arg); return arg.Context; } return 0; @@ -98,3 +101,23 @@ x_cgo_notify_runtime_init_done(void* dummy) { } } +// The context function, used when tracing back C calls into Go. +static void (*cgo_context_function)(struct context_arg*); + +// Sets the context function to call to record the traceback context +// when calling a Go function from C code. Called from runtime.SetCgoTraceback. +void x_cgo_set_context_function(void (*context)(struct context_arg*)) { + EnterCriticalSection(&runtime_init_cs); + cgo_context_function = context; + LeaveCriticalSection(&runtime_init_cs); +} + +// Gets the context function. +void (*(_cgo_get_context_function(void)))(struct context_arg*) { + void (*ret)(struct context_arg*); + + EnterCriticalSection(&runtime_init_cs); + ret = cgo_context_function; + LeaveCriticalSection(&runtime_init_cs); + return ret; +} diff --git a/src/runtime/cgo/libcgo.h b/src/runtime/cgo/libcgo.h index 249d052edc..01f9e72174 100644 --- a/src/runtime/cgo/libcgo.h +++ b/src/runtime/cgo/libcgo.h @@ -93,7 +93,7 @@ void darwin_arm_init_mach_exception_handler(void); struct context_arg { uintptr_t Context; }; -extern void (*x_cgo_context_function)(struct context_arg*); +extern void (*(_cgo_get_context_function(void)))(struct context_arg*); /* * TSAN support. This is only useful when building with -- 2.48.1