[PATCH] D49292: [MSan] factor userspace-specific declarations into createUserspaceApi(). NFC

[Alexander Potapenko via Phabricator via llvm-commits]

glider added inline comments.


================
Comment at: lib/Transforms/Instrumentation/MemorySanitizer.cpp:593
+  // Only do this once.
+  static bool CallbacksInitialized = false;
+  if (CallbacksInitialized)
----------------
eugenis wrote:
> glider wrote:
> > eugenis wrote:
> > > This should probably be a property of a module, not a compiler invocation.
> > > 
> > We used to check for WarningFn value here, and are just using a special bool instead now.
> > It might be cleaner to make CallbacksInitialized a property of `class MemorySanitizer`, but how can it be a property of a module?
> Just consider that this bool can become true only once in the compiler process. No matter how many modules are created.
> 
> Moving it to class MemorySanitizer sounds good.
> 
> In fact, a FunctionPass is not supposed to create new functions (which AFAIK includes function declarations) outside of doInitialization(). MSan breaks this assumption in a bunch of places...
> 
> Just consider that this bool can become true only once in the compiler process. No matter how many modules are created.
IIRC we don't do that in other tools either, do we?

> Moving it to class MemorySanitizer sounds good.
Done

> In fact, a FunctionPass is not supposed to create new functions (which AFAIK includes function declarations) outside of doInitialization(). MSan breaks this assumption in a bunch of places...
We only create getAndInsertFunction() from initializeCallbacks().
Wonder if it's possible to just call it from doInitialization()


https://reviews.llvm.org/D49292