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

[Evgenii Stepanov via Phabricator via llvm-commits]

eugenis added inline comments.


================
Comment at: lib/Transforms/Instrumentation/MemorySanitizer.cpp:593
+  // Only do this once.
+  static bool CallbacksInitialized = false;
+  if (CallbacksInitialized)
----------------
glider wrote:
> 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
> IIRC we don't do that in other tools either, do we?
Do what?

> We only create getAndInsertFunction() from initializeCallbacks().
> Wonder if it's possible to just call it from doInitialization()

Yes, but only if at least on function in the module has sanitize_memory attribute.
We also mess with other functions attributes in visitCallSite(), and that can not be done ahead of time because some other function pass may revert the change.








https://reviews.llvm.org/D49292