[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)
----------------
glider wrote:
> 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()
> We only create getAndInsertFunction() from initializeCallbacks().
s/create/call


https://reviews.llvm.org/D49292