[PATCH] D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager

[Leonard Chan via Phabricator via llvm-commits]

leonardchan added a comment.

In https://reviews.llvm.org/D52739#1258087, @philip.pfaffe wrote:

> Some comments inline. Does it make sense to merge the module and function passes into one? Is it really necessary to distinguish the passes?


This I'm not sure of since it seems that they both do different things that work independently of each other, though I can't find an example/test where they are used on their own. @vitalybuka may be able to offer more insight into this. Otherwise if it's necessary to combine the passes, I can create a separate patch that does this and changes any relevant tests.



================
Comment at: include/llvm/IR/AddressSanitizerPass.h:23
+
+class AddressSanitizerWrapper;
+class AddressSanitizerModuleWrapper;
----------------
leonardchan wrote:
> fedor.sergeev wrote:
> > I dont believe you need any extra wrappers in addition to AddressSanitizer*Pass ones.
> > 
> > Just use class AddressSanitizer/AddresSanitizerModule here.
> Done, but this would require moving `AddressSanitizer`, `AddressSanitizerModule`, and `FunctionStackPoisoner` outside of their anonymous namespace.
Moved them back into their anonymous namespace.


================
Comment at: lib/Transforms/Instrumentation/AddressSanitizer.cpp:2538
+                                  const TargetLibraryInfo *TLI) {
+  if (!Inited) {
+    // Initialize the private fields. No one has accessed them before.
----------------
philip.pfaffe wrote:
> Instead of doing this kind of lazy initialization, just do it in the constructor.
This initialization requires access to the Module that this pass runs over but I don't think I have access to that in the constructor.


Repository:
  rL LLVM

https://reviews.llvm.org/D52739