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

[leonardchan]

Summary

Enable usage of AddressSanitizer and AddressModuleSanitizer ported from the legacy to the new PassManager.

This patch depends on https://reviews.llvm.org/D52739.

[philip.pfaffe]

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?

[fedor.sergeev]

lib/CodeGen/BackendUtil.cpp

1031–1038

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?

[leonardchan]

In 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.

lib/CodeGen/BackendUtil.cpp

1031–1038

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.

[philip.pfaffe]

You're right, my bad; I missed the fact that it's EP_OptimizerLast, which has no equivalent in the new PM. Your implementation is actually correct here.

Did you run your test in a debug build?

[leonardchan]

In D52814#1262550, @philip.pfaffe wrote:

You're right, my bad; I missed the fact that it's EP_OptimizerLast, which has no equivalent in the new PM. Your implementation is actually correct here.

Did you run your test in a debug build?

Yes, and I tested it on release also in case there were any differences between either builds.

[leonardchan]

@philip.pfaffe @fedor.sergeev Do you have any more comments on this patch?

[philip.pfaffe]

LGTM, thank you!

[fedor.sergeev]

LGTM.

lib/CodeGen/BackendUtil.cpp

 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Bitcode/BitcodeReader.h"
 #include "llvm/Bitcode/BitcodeWriter.h"
 #include "llvm/Bitcode/BitcodeWriterPass.h"
 #include "llvm/CodeGen/RegAllocRegistry.h"
 #include "llvm/CodeGen/SchedulerRegistry.h"
 #include "llvm/CodeGen/TargetSubtargetInfo.h"
+#include "llvm/IR/AddressSanitizerPass.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/IRPrintingPasses.h"
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/ModuleSummaryIndex.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/LTO/LTOBackend.h"
 #include "llvm/MC/MCAsmInfo.h"
case 2:
       return PassBuilder::Oz;
     }
 
   case 3:
     return PassBuilder::O3;
   }
 }
 
+static FunctionPassManager buildAddressSanitizerPipeline(bool DebugLogging) {
+  FunctionPassManager FPM(DebugLogging);
+  FPM.addPass(AddressSanitizerPass());
+  return FPM;
+}
+
 /// A clean version of `EmitAssembly` that uses the new pass manager.
 ///
 /// Not all features are currently supported in this system, but where
 /// necessary it falls back to the legacy pass manager to at least provide
 /// basic functionality.
 ///
 /// This API is planned to have its functionality finished and then to replace
 /// `EmitAssembly` at some point in the future when the default switches.
if (CodeGenOpts.OptimizationLevel == 0) {
         MPM = PB.buildLTOPreLinkDefaultPipeline(Level,
                                                 CodeGenOpts.DebugPassManager);
         MPM.addPass(NameAnonGlobalPass());
       } else {
         MPM = PB.buildPerModuleDefaultPipeline(Level,
                                                CodeGenOpts.DebugPassManager);
       }
     }
+
+    if (LangOpts.Sanitize.has(SanitizerKind::Address)) {
+      MPM.addPass(AddressSanitizerModulePass());
+
+      // Add Function Pass
+      CGSCCPassManager MainCGPipeline(CodeGenOpts.DebugPassManager);
+      MainCGPipeline.addPass(createCGSCCToFunctionPassAdaptor(
+          buildAddressSanitizerPipeline(CodeGenOpts.DebugPassManager)));
+      MPM.addPass(
+          createModuleToPostOrderCGSCCPassAdaptor(std::move(MainCGPipeline)));
+    }
   }
 
   // FIXME: We still use the legacy pass manager to do code generation. We
   // create that pass manager here and use it as needed below.
   legacy::PassManager CodeGenPasses;
   bool NeedCodeGen = false;
   std::unique_ptr<llvm::ToolOutputFile> ThinLinkOS, DwoOS;
 

test/CodeGen/asan-new-pm.ll

+; RUN: %clang_cc1 -S -emit-llvm -o - -fexperimental-new-pass-manager -fsanitize=address %s | FileCheck %s
+
+; CHECK: @llvm.global_ctors = {{.*}}@asan.module_ctor
+
+define i32 @test_load(i32* %a) sanitize_address {
+entry:
+; CHECK:  %0 = ptrtoint i32* %a to i64
+; CHECK:  %1 = lshr i64 %0, 3
+; CHECK:  %2 = add i64 %1, 2147450880
+; CHECK:  %3 = inttoptr i64 %2 to i8*
+; CHECK:  %4 = load i8, i8* %3
+; CHECK:  %5 = icmp ne i8 %4, 0
+; CHECK:  br i1 %5, label %6, label %12, !prof !0
+
+; CHECK:; <label>:6:                                      ; preds = %entry
+; CHECK:  %7 = and i64 %0, 7
+; CHECK:  %8 = add i64 %7, 3
+; CHECK:  %9 = trunc i64 %8 to i8
+; CHECK:  %10 = icmp sge i8 %9, %4
+; CHECK:  br i1 %10, label %11, label %12
+
+; CHECK:; <label>:11:                                     ; preds = %6
+; CHECK:  call void @__asan_report_load4(i64 %0)
+; CHECK:  call void asm sideeffect "", ""()
+; CHECK:  unreachable
+
+; CHECK:; <label>:12:                                     ; preds = %6, %entry
+
+  %tmp1 = load i32, i32* %a, align 4
+  ret i32 %tmp1
+}