code review 5966067: runtime: give proper messages when user defined DYLD_IN...

runtime: give proper messages when user defined DYLD_INSERT_LIBRARIES on Darwin
       also gives clear error when bsdthread_register fails on Darwin
       Fixes issue 2992.

[minux1, 2012-04-04 17:12:00]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go/

[minux1, 2012-04-04 17:13:51]

If you want to trigger the error message.
$ cat lib.c
#include <stdio.h>
int f() __attribute__((constructor));
int f() { fprintf(stderr, "hello from lib.so!\n"); }
$ gcc -o lib.so -dynamiclib -fno-common lib.c
$ DYLD_INSERT_LIBRARIES=lib.so DYLD_FORCE_FLAT_NAMESPACE=1 ./6.out

[rsc, 2012-04-04 17:24:55]

Can we just check for the environment variable instead?
I try to avoid error messages that guess at what's wrong.
We can check the environment variable and refuse to even
call bsdthread_register.

[minux1, 2012-04-04 18:14:31]

On 2012/04/04 17:24:55, rsc wrote:
> Can we just check for the environment variable instead?
At the time of runtime.osinit(), syscall.envs haven't been initialized.

You can try Patch Set 4, but the getenv test won't ever be true.

[rsc, 2012-04-05 15:20:19]

http://codereview.appspot.com/5966067/diff/3006/src/pkg/runtime/thread_darwin.c
File src/pkg/runtime/thread_darwin.c (right):

http://codereview.appspot.com/5966067/diff/3006/src/pkg/runtime/thread_darwin...
src/pkg/runtime/thread_darwin.c:53: // Register our thread-creation callback
(see sys_darwin_{amd64,386}.s)
It's pretty ugly, but I still want to be able to give a precise error message.
Maybe move this code (including the register call) to the bottom of goenvs
instead?

Here, we can leave just: 

// bsdthread_register delayed until end of goenvs so that we
// can look at the environment first.

You can't call goenvs here because it needs malloc.

[minux1, 2012-04-05 20:45:16]

PTAL.

I've tested this on Mac OS X 10.6.8 amd64 and 386, could you please help test
this
on Mac OS X 10.7?

[mkrautz, 2012-04-09 18:36:43]

On 2012/04/05 20:45:16, minux wrote:
> PTAL.
> 
> I've tested this on Mac OS X 10.6.8 amd64 and 386, could you please help test
> this
> on Mac OS X 10.7?

Works for me on 10.7.

[minux1, 2012-04-19 19:45:29]

ping.

[rsc, 2012-04-25 03:08:51]

http://codereview.appspot.com/5966067/diff/15001/src/pkg/runtime/thread_darwin.c
File src/pkg/runtime/thread_darwin.c (right):

http://codereview.appspot.com/5966067/diff/15001/src/pkg/runtime/thread_darwi...
src/pkg/runtime/thread_darwin.c:80: if(runtime·getenv("DYLD_INSERT_LIBRARIES"))
if(runtime·bsdthread_register() != 0) {
    if(runtime·getenv("DYLD_INSERT_LIBRARIES") != nil)
        runtime·throw("runtime: bsdthread_register error (unset
DYLD_INSERT_LIBRARIES)");
    runtime·throw("runtime: bsdthread_register error")
}

[minux1, 2012-04-25 05:51:57]

Hello rsc@golang.org, krautz@gmail.com (cc: golang-dev@googlegroups.com),

Please take another look.

[rsc, 2012-04-30 19:55:30]

*** Submitted as http://code.google.com/p/go/source/detail?r=09fe7345e11d ***

runtime: give proper messages when user defined DYLD_INSERT_LIBRARIES on Darwin
       also gives clear error when bsdthread_register fails on Darwin
       Fixes issue 2992.

R=rsc, krautz
CC=golang-dev
http://codereview.appspot.com/5966067

Committer: Russ Cox <rsc@golang.org>

[rsc, 2012-04-30 19:55:39]

LGTM