[PATCH] D52814: [PassManager/Sanitizer] Enable usage of ported AddressSanitizer passes with -fsanitize=address

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

In https://reviews.llvm.org/D52814#1254901, @philip.pfaffe wrote:

> Is this the right place in the pipeline to put the passes? The legacy PM inserts the asan passes at EP_OptimizerLast, why not do the same thing?


I noticed this also and thought adding this using `registerScalarOptimizerLateEPCallback` was the initial correct way to add this function pass, but it seems that the vector of callbacks this gets added to (`ScalarOptimizerLateEPCallbacks` in `buildFunctionSimplificationPipeline`) doesn't get called unless some sort of optimization is requested (that is I run with `-fexperimental-new-pass-manager -fsanitize=address -O1`) when the sanitizer should still be applied even without optimization.

We could force this pipeline to be run if we run `PB.buildPerModuleDefaultPipeline()`regardless of the optimization level, but I don't know why it's only called if optimization is zero.



================
Comment at: lib/CodeGen/BackendUtil.cpp:1031-1038
+      MPM.addPass(AddressSanitizerModulePass());
+
+      // Add Function Pass
+      CGSCCPassManager MainCGPipeline(CodeGenOpts.DebugPassManager);
+      MainCGPipeline.addPass(createCGSCCToFunctionPassAdaptor(
+          buildAddressSanitizerPipeline(CodeGenOpts.DebugPassManager)));
+      MPM.addPass(
----------------
fedor.sergeev wrote:
> I dont believe CGSCC is appropriate here.
> 
> I would expect to see a simple ModuleToFunction adapter, something like this:
> 
> MPM.addPass(AddressSanitizerModulePass());
> MPM.addPass(createModuleToFunctionPassAdaptor(AddressSanitizerFunctionPass())
> 
> Also, it seems that legacy runs Function sanitizer first and then Module sanitizer after it.
> This sequence does it backwards. Is it intentional?
My bad. I didn't see there was a `createModuleToFunctionPassAdaptor` earlier and used CGSCC since that seemed to be how other function passes were added to the module.

I also wasn't sure if the order mattered since it seems both passes can be run independently of each other and provide different instrumentation. I think @vitalybuka might be able to answer that better, Regardless, I switched them still so Module runs after Function.


Repository:
  rC Clang

https://reviews.llvm.org/D52814