[PATCH] D54469: Introduce new `disable_init` ASan option that is only supported on platforms where `SANITIZER_SUPPORTS_DISABLED_INIT` is true. Currently this is only supported on Darwin.

[Dan Liew via Phabricator via llvm-commits]

delcypher added inline comments.


================
Comment at: lib/sanitizer_common/sanitizer_platform.h:348-352
+#if SANITIZER_MAC
+#define SANITIZER_SUPPORTS_INIT_FOR_DLOPEN 1
+#else
+#define SANITIZER_SUPPORTS_INIT_FOR_DLOPEN 0
+#endif
----------------
kubamracek wrote:
> delcypher wrote:
> > kubamracek wrote:
> > > Why do we need this? I'd remove it.
> > Two reasons:
> > 
> > 1. I wanted the feature to be zero-cost for platforms that don't use it. You can see it's used in the `if` condition in `AsanInitInternal()`. It's a compile time constant so for platforms that don't support this feature the branch should just be folded away and so it won't even be checked at runtime.
> > 2. We need to something to let us switch between the definitions of  `InitIsViaDlopen()` and ` HandleDlopenInit()`. This macro is used for this.
> > 
> > If you have a clean solution to handle the second reason without `SANITIZER_SUPPORTS_INIT_FOR_DLOPEN` then I can implement it.
> We're trying to minimize compile-time constants, as that adds complexity. Why not just have this behavior (reading an env var to skip initialization) on all platforms?
I don't think having this behaviour an all platforms is a good idea. Our implementation is Darwin specific and I don't think other platforms should have to care about this at all. I think it should be a platform maintainer's choice to allow/disallow disabling of the ASan init process. Given that I'm adding a new feature that is only relevant to Darwin right now I think the feature should be completely non-existent on other platforms. This is why I chose to introduce the ` SANITIZER_SUPPORTS_INIT_FOR_DLOPEN` compile time constant.

Also if we were to make this feature work on all platforms then it would make more sense to put the option in `ASAN_OPTIONS` (where it was originally in this patch) which you asked me **not to do**.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D54469