[clang] c07339c - Move *San module passes later in the NPM pipeline

Arthur Eubanks via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 8 12:09:11 PDT 2020


Author: Arthur Eubanks
Date: 2020-06-08T12:08:49-07:00
New Revision: c07339c6754e1faf4078d3c592c0ae28455503cd

URL: https://github.com/llvm/llvm-project/commit/c07339c6754e1faf4078d3c592c0ae28455503cd
DIFF: https://github.com/llvm/llvm-project/commit/c07339c6754e1faf4078d3c592c0ae28455503cd.diff

LOG: Move *San module passes later in the NPM pipeline

Summary:
This fixes pr33372.cpp under the new pass manager.

ASan adds padding to globals. For example, it will change a {i32, i32, i32} to a {{i32, i32, i32}, [52 x i8]}. However, when loading from the {i32, i32, i32}, InstCombine may (after various optimizations) end up loading 16 bytes instead of 12, likely because it thinks the [52 x i8] padding is ok to load from. But ASan checks that padding should not be loaded from.

Ultimately this is an issue of *San passes wanting to be run after all optimizations. This change moves the module passes right next to the corresponding function passes.

Also remove comment that's no longer relevant, this is the last ASan/MSan/TSan failure under the NPM (hopefully...).

As mentioned in https://reviews.llvm.org/rG1285e8bcac2c54ddd924ffb813b2b187467ac2a6, NPM doesn't support LTO + sanitizers, so modified some tests that test for that.

Reviewers: leonardchan, vitalybuka

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D81323

Added: 
    

Modified: 
    clang/lib/CodeGen/BackendUtil.cpp
    clang/test/CodeGen/asan-new-pm.ll
    clang/test/CodeGen/sanitizer-module-constructor.c

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index 9f28fed40ebc..ebf74dbbe1f5 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -1265,7 +1265,6 @@ void EmitAssemblyHelper::EmitAssemblyWithNewPassManager(
 
       // Register callbacks to schedule sanitizer passes at the appropriate part of
       // the pipeline.
-      // FIXME: either handle asan/the remaining sanitizers or error out
       if (LangOpts.Sanitize.has(SanitizerKind::LocalBounds))
         PB.registerScalarOptimizerLateEPCallback(
             [](FunctionPassManager &FPM, PassBuilder::OptimizationLevel Level) {
@@ -1288,48 +1287,38 @@ void EmitAssemblyHelper::EmitAssemblyWithNewPassManager(
       if (LangOpts.Sanitize.has(SanitizerKind::Memory)) {
         int TrackOrigins = CodeGenOpts.SanitizeMemoryTrackOrigins;
         bool Recover = CodeGenOpts.SanitizeRecover.has(SanitizerKind::Memory);
-        PB.registerPipelineStartEPCallback(
-            [TrackOrigins, Recover](ModulePassManager &MPM) {
-              MPM.addPass(MemorySanitizerPass({TrackOrigins, Recover, false}));
-            });
         PB.registerOptimizerLastEPCallback(
             [TrackOrigins, Recover](ModulePassManager &MPM,
                                     PassBuilder::OptimizationLevel Level) {
+              MPM.addPass(MemorySanitizerPass({TrackOrigins, Recover, false}));
               MPM.addPass(createModuleToFunctionPassAdaptor(
                   MemorySanitizerPass({TrackOrigins, Recover, false})));
             });
       }
       if (LangOpts.Sanitize.has(SanitizerKind::Thread)) {
-        PB.registerPipelineStartEPCallback(
-            [](ModulePassManager &MPM) { MPM.addPass(ThreadSanitizerPass()); });
         PB.registerOptimizerLastEPCallback(
             [](ModulePassManager &MPM, PassBuilder::OptimizationLevel Level) {
+              MPM.addPass(ThreadSanitizerPass());
               MPM.addPass(
                   createModuleToFunctionPassAdaptor(ThreadSanitizerPass()));
             });
       }
       if (LangOpts.Sanitize.has(SanitizerKind::Address)) {
-        PB.registerPipelineStartEPCallback([&](ModulePassManager &MPM) {
-          MPM.addPass(
-              RequireAnalysisPass<ASanGlobalsMetadataAnalysis, Module>());
-        });
         bool Recover = CodeGenOpts.SanitizeRecover.has(SanitizerKind::Address);
         bool UseAfterScope = CodeGenOpts.SanitizeAddressUseAfterScope;
-        PB.registerOptimizerLastEPCallback(
-            [Recover, UseAfterScope](ModulePassManager &MPM,
-                                     PassBuilder::OptimizationLevel Level) {
-              MPM.addPass(
-                  createModuleToFunctionPassAdaptor(AddressSanitizerPass(
-                      /*CompileKernel=*/false, Recover, UseAfterScope)));
-            });
         bool ModuleUseAfterScope = asanUseGlobalsGC(TargetTriple, CodeGenOpts);
         bool UseOdrIndicator = CodeGenOpts.SanitizeAddressUseOdrIndicator;
-        PB.registerPipelineStartEPCallback(
-            [Recover, ModuleUseAfterScope,
-             UseOdrIndicator](ModulePassManager &MPM) {
+        PB.registerOptimizerLastEPCallback(
+            [Recover, UseAfterScope, ModuleUseAfterScope, UseOdrIndicator](
+                ModulePassManager &MPM, PassBuilder::OptimizationLevel Level) {
+              MPM.addPass(
+                  RequireAnalysisPass<ASanGlobalsMetadataAnalysis, Module>());
               MPM.addPass(ModuleAddressSanitizerPass(
                   /*CompileKernel=*/false, Recover, ModuleUseAfterScope,
                   UseOdrIndicator));
+              MPM.addPass(
+                  createModuleToFunctionPassAdaptor(AddressSanitizerPass(
+                      /*CompileKernel=*/false, Recover, UseAfterScope)));
             });
       }
       if (Optional<GCOVOptions> Options = getGCOVOptions(CodeGenOpts))

diff  --git a/clang/test/CodeGen/asan-new-pm.ll b/clang/test/CodeGen/asan-new-pm.ll
index e107e992ac6c..fd840c4e7d10 100644
--- a/clang/test/CodeGen/asan-new-pm.ll
+++ b/clang/test/CodeGen/asan-new-pm.ll
@@ -1,12 +1,8 @@
 ; Test that ASan runs with the new pass manager
-; RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - -fexperimental-new-pass-manager -fsanitize=address %s | FileCheck %s --check-prefixes=CHECK,LTO,THINLTO
-; RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - -fexperimental-new-pass-manager -fsanitize=address -flto %s | FileCheck %s --check-prefixes=CHECK,LTO
-; RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - -fexperimental-new-pass-manager -fsanitize=address -flto=thin %s | FileCheck %s --check-prefixes=CHECK,THINLTO
-; RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - -O1 -fexperimental-new-pass-manager -fsanitize=address %s | FileCheck %s --check-prefixes=CHECK,LTO,THINLTO
-; RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - -O1 -fexperimental-new-pass-manager -fsanitize=address -flto %s | FileCheck %s --check-prefixes=CHECK,LTO
-; RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - -O1 -fexperimental-new-pass-manager -fsanitize=address -flto=thin %s | FileCheck %s --check-prefixes=CHECK,THINLTO
+; RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - -fexperimental-new-pass-manager -fsanitize=address %s | FileCheck %s
+; RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - -O1 -fexperimental-new-pass-manager -fsanitize=address %s | FileCheck %s
 
-; DAG-CHECK: @llvm.global_ctors = {{.*}}@asan.module_ctor
+; CHECK-DAG: @llvm.global_ctors = {{.*}}@asan.module_ctor
 
 define i32 @test_load(i32* %a) sanitize_address {
 entry:
@@ -16,13 +12,11 @@ entry:
 
 ; CHECK: __asan_init
 
-; DAG-CHECK: define internal void @asan.module_ctor() {
+; CHECK-DAG: define internal void @asan.module_ctor() {
 ; CHECK:       {{.*}} call void @__asan_init()
 ; CHECK:       {{.*}} call void @__asan_version_mismatch_check_v8()
 ; CHECK:       ret void
 ; CHECK:     }
 
-; DAG-CHECK: __asan_version_mismatch_check_v8
+; CHECK-DAG: __asan_version_mismatch_check_v8
 
-; This is not used in ThinLTO
-; DAG-LTO: __asan_report_load4

diff  --git a/clang/test/CodeGen/sanitizer-module-constructor.c b/clang/test/CodeGen/sanitizer-module-constructor.c
index 70d75e9aa7b4..3e29a923a9f1 100644
--- a/clang/test/CodeGen/sanitizer-module-constructor.c
+++ b/clang/test/CodeGen/sanitizer-module-constructor.c
@@ -19,4 +19,3 @@ void h() { f(e); }
 // CHECK: Running pass: {{.*}}SanitizerPass on {{.*}}sanitizer-module-constructor.c
 // CHECK-NOT: Running pass: LoopSimplifyPass on {{.*}}san.module_ctor
 // CHECK: Running analysis: DominatorTreeAnalysis on {{.*}}san.module_ctor
-// CHECK: Running pass: LoopSimplifyPass on {{.*}}san.module_ctor


        


More information about the cfe-commits mailing list