Page MenuHomePhabricator

Clear LD_PRELOAD when forking external symbolizer.
ClosedPublic

Authored by earthdok on Apr 22 2013, 9:16 AM.

Details

Diff Detail

Event Timeline

Why not simply unsetenv("LD_PRELOAD")?

glider added inline comments.Apr 23 2013, 1:28 AM
lib/sanitizer_common/sanitizer_symbolizer_linux.cc
103

If LD_PRELOAD contains more than one library, you only need to remove yours, not unset LD_PRELOAD completely.

Why not simply unsetenv("LD_PRELOAD")?

I thought unsetenv acquired a lock, which in fact it doesn't.

lib/sanitizer_common/sanitizer_symbolizer_linux.cc
103

How do we know which one is ours?

glider added inline comments.Apr 23 2013, 3:06 AM
lib/sanitizer_common/sanitizer_symbolizer_linux.cc
103

You can obtain the library name from dladdr().

earthdok updated this revision to Unknown Object (????).Apr 23 2013, 3:50 AM
  • removed custom unsetenv implementation
  • added a fixme
lib/sanitizer_common/sanitizer_symbolizer_linux.cc
103

Adding a FIXME for now, as per our offline discussion.

glider accepted this revision.Apr 23 2013, 3:56 AM

LGTM

kcc added inline comments.Apr 23 2013, 5:42 AM
lib/sanitizer_common/sanitizer_symbolizer_linux.cc
107

Will this work with bash?
(bash has its own setenv/getenv. Maybe unsetenv too. I'd prefer to have a custom implementation if possible)

Can we do a "custom" implementation by dlsym("unsetenv") lookup (similar to
current SetEnv implementation)?

earthdok updated this revision to Unknown Object (????).Apr 23 2013, 6:10 AM
  • reverted to a custom implementation of unsetenv

Also, modified the implementation to allow multiple entries for the same
variable.

I couldn't find anything related to setenv in bash sources, but I reverted to a custom implementation anyway.

Can we do a "custom" implementation by dlsym("unsetenv") lookup (similar to current SetEnv implementation)?

No. It's not safe to call dlsym after a fork.

earthdok closed this revision.Dec 5 2014, 9:43 AM