[PATCH] D55711: Don't trigger sanitizer initialization from `sysctlbyname` interceptor.

[Dan Liew via Phabricator via llvm-commits]

delcypher added a comment.

In D55711#1331469 <https://reviews.llvm.org/D55711#1331469>, @vitalybuka wrote:

> If we are going to support this use-case we need a test.
>
> I think you can clone/update sysctl.cc, and define all possible __*san_default_options as following:
>
>   __tsan_default_options() {
>     test_sysctl();
>     test_sysctlbyname();
>     test_sysctlnametomib();
>   }
>
>
> This is probably early enough.
>  Please check that the new test fail without your patch.


@vitalybuka

As @kubamracek pointed we already have a failing test. However I tried getting your suggested testing approach and unfortunately it doesn't work.

1. Defining `__tsan_default_options()` as you suggest does trigger a crash, however this was due to `__tsan_default_options()` and the functions it calls being instrumented by TSan. In particular the instrumentation adds calls to `__tsan_func_entry()` which triggers a crash if we're already doing TSan init. Changing the implementation of the `sysctl` interceptors doesn't fix this because we crash even before we get to those.
2. I rewrote the test to build `__tsan_default_options()` (and the functions it calls) without any instrumentation. However now the test succeeds, i.e. we can call the current implementation of `sysctl` from inside `__tsan_default_options` and there's no crash.

Basically triggering a call to `sysctl` inside `__tsan_default_options` is the wrong thing to do. This is because if `__tsan_default_options` is being called it means TSan's init has already been triggered and it's already too late. To reproduce this bug (not in a iOS simulator test) we'd have to trigger the call to `sysctl` much earlier and I don't know how to do this. I think we're better of just relying on the iOS simulator test and just making sure they are run on Darwin by default if the simulator is available.


Repository:
  rCRT Compiler Runtime

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55711/new/

https://reviews.llvm.org/D55711