[PATCH] D66695: msan, codegen, instcombine: Keep more lifetime markers used for msan
This revision was automatically updated to reflect the committed changes. Closed by commit rL369979: msan, codegen, instcombine: Keep more lifetime markers used for msan (authored by vitalybuka, committed by ). Herald added a subscriber: delcypher. Changed prior to commit: https://reviews.llvm.org/D66695?vs=216994=217252#toc Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66695/new/ https://reviews.llvm.org/D66695 Files: cfe/trunk/lib/CodeGen/CGExpr.cpp cfe/trunk/lib/CodeGen/CodeGenFunction.cpp cfe/trunk/test/CodeGen/lifetime-sanitizer.c cfe/trunk/test/CodeGenCXX/lifetime-sanitizer.cpp compiler-rt/trunk/test/msan/loop-scope.cpp llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp llvm/trunk/test/Transforms/InstCombine/lifetime-sanitizer.ll Index: llvm/trunk/test/Transforms/InstCombine/lifetime-sanitizer.ll === --- llvm/trunk/test/Transforms/InstCombine/lifetime-sanitizer.ll +++ llvm/trunk/test/Transforms/InstCombine/lifetime-sanitizer.ll @@ -34,6 +34,21 @@ ret void } +define void @msan() sanitize_memory { +entry: + ; CHECK-LABEL: @msan( + %text = alloca i8, align 1 + + call void @llvm.lifetime.start.p0i8(i64 1, i8* %text) + call void @llvm.lifetime.end.p0i8(i64 1, i8* %text) + ; CHECK: call void @llvm.lifetime.start + ; CHECK-NEXT: call void @llvm.lifetime.end + + call void @foo(i8* %text) ; Keep alloca alive + + ret void +} + define void @no_asan() { entry: ; CHECK-LABEL: @no_asan( Index: llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp === --- llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp @@ -3885,6 +3885,7 @@ // Asan needs to poison memory to detect invalid access which is possible // even for empty lifetime range. if (II->getFunction()->hasFnAttribute(Attribute::SanitizeAddress) || +II->getFunction()->hasFnAttribute(Attribute::SanitizeMemory) || II->getFunction()->hasFnAttribute(Attribute::SanitizeHWAddress)) break; Index: cfe/trunk/test/CodeGenCXX/lifetime-sanitizer.cpp === --- cfe/trunk/test/CodeGenCXX/lifetime-sanitizer.cpp +++ cfe/trunk/test/CodeGenCXX/lifetime-sanitizer.cpp @@ -3,6 +3,9 @@ // RUN: %clang -w -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 \ // RUN: -fsanitize=address -fsanitize-address-use-after-scope %s | \ // RUN: FileCheck %s -check-prefixes=CHECK,LIFETIME +// RUN: %clang -w -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 \ +// RUN: -fsanitize=memory %s | \ +// RUN: FileCheck %s -check-prefixes=CHECK,LIFETIME extern int bar(char *A, int n); Index: cfe/trunk/test/CodeGen/lifetime-sanitizer.c === --- cfe/trunk/test/CodeGen/lifetime-sanitizer.c +++ cfe/trunk/test/CodeGen/lifetime-sanitizer.c @@ -2,6 +2,9 @@ // RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \ // RUN: -fsanitize=address -fsanitize-address-use-after-scope %s | \ // RUN: FileCheck %s -check-prefix=LIFETIME +// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \ +// RUN: -fsanitize=memory %s | \ +// RUN: FileCheck %s -check-prefix=LIFETIME extern int bar(char *A, int n); Index: cfe/trunk/lib/CodeGen/CGExpr.cpp === --- cfe/trunk/lib/CodeGen/CGExpr.cpp +++ cfe/trunk/lib/CodeGen/CGExpr.cpp @@ -516,13 +516,12 @@ // Avoid creating a conditional cleanup just to hold an llvm.lifetime.end // marker. Instead, start the lifetime of a conditional temporary earlier - // so that it's unconditional. Don't do this in ASan's use-after-scope - // mode so that it gets the more precise lifetime marks. If the type has - // a non-trivial destructor, we'll have a cleanup block for it anyway, - // so this typically doesn't help; skip it in that case. + // so that it's unconditional. Don't do this with sanitizers which need + // more precise lifetime marks. ConditionalEvaluation *OldConditional = nullptr; CGBuilderTy::InsertPoint OldIP; if (isInConditionalBranch() && !E->getType().isDestructedType() && + !SanOpts.has(SanitizerKind::Memory) && !CGM.getCodeGenOpts().SanitizeAddressUseAfterScope) { OldConditional = OutermostConditional; OutermostConditional = nullptr; Index: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp === --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp @@ -47,13 +47,9 @@ if (CGOpts.DisableLifetimeMarkers) return false; - // Disable lifetime markers in msan builds. - // FIXME: Remove this when
[PATCH] D66695: msan, codegen, instcombine: Keep more lifetime markers used for msan
lebedev.ri added inline comments. Comment at: llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll:37-50 +define void @msan() sanitize_memory { +entry: + ; CHECK-LABEL: @msan( + %text = alloca i8, align 1 + + call void @llvm.lifetime.start.p0i8(i64 1, i8* %text) + call void @llvm.lifetime.end.p0i8(i64 1, i8* %text) eugenis wrote: > lebedev.ri wrote: > > Would be good to have end-to-end test (`-O0`/`-O1`/`-O2`/`-O3`) in > > PhaseOrdering. > Do you mean exact same test, but with -O0/../-O3 instead of -instcombine? Yes, but please put it into PhaseOrdering directory, Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66695/new/ https://reviews.llvm.org/D66695 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D66695: msan, codegen, instcombine: Keep more lifetime markers used for msan
eugenis accepted this revision. eugenis added a comment. This revision is now accepted and ready to land. LGTM Comment at: llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll:37-50 +define void @msan() sanitize_memory { +entry: + ; CHECK-LABEL: @msan( + %text = alloca i8, align 1 + + call void @llvm.lifetime.start.p0i8(i64 1, i8* %text) + call void @llvm.lifetime.end.p0i8(i64 1, i8* %text) lebedev.ri wrote: > Would be good to have end-to-end test (`-O0`/`-O1`/`-O2`/`-O3`) in > PhaseOrdering. Do you mean exact same test, but with -O0/../-O3 instead of -instcombine? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66695/new/ https://reviews.llvm.org/D66695 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D66695: msan, codegen, instcombine: Keep more lifetime markers used for msan
lebedev.ri added inline comments. Comment at: llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll:37-50 +define void @msan() sanitize_memory { +entry: + ; CHECK-LABEL: @msan( + %text = alloca i8, align 1 + + call void @llvm.lifetime.start.p0i8(i64 1, i8* %text) + call void @llvm.lifetime.end.p0i8(i64 1, i8* %text) Would be good to have end-to-end test (`-O0`/`-O1`/`-O2`/`-O3`) in PhaseOrdering. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66695/new/ https://reviews.llvm.org/D66695 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D66695: msan, codegen, instcombine: Keep more lifetime markers used for msan
vitalybuka updated this revision to Diff 216994. vitalybuka added a comment. update comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66695/new/ https://reviews.llvm.org/D66695 Files: clang/lib/CodeGen/CGExpr.cpp clang/lib/CodeGen/CodeGenFunction.cpp clang/test/CodeGen/lifetime-sanitizer.c clang/test/CodeGenCXX/lifetime-sanitizer.cpp compiler-rt/test/msan/loop-scope.cpp llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll Index: llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll === --- llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll +++ llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll @@ -34,6 +34,21 @@ ret void } +define void @msan() sanitize_memory { +entry: + ; CHECK-LABEL: @msan( + %text = alloca i8, align 1 + + call void @llvm.lifetime.start.p0i8(i64 1, i8* %text) + call void @llvm.lifetime.end.p0i8(i64 1, i8* %text) + ; CHECK: call void @llvm.lifetime.start + ; CHECK-NEXT: call void @llvm.lifetime.end + + call void @foo(i8* %text) ; Keep alloca alive + + ret void +} + define void @no_asan() { entry: ; CHECK-LABEL: @no_asan( Index: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp === --- llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp +++ llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp @@ -3885,6 +3885,7 @@ // Asan needs to poison memory to detect invalid access which is possible // even for empty lifetime range. if (II->getFunction()->hasFnAttribute(Attribute::SanitizeAddress) || +II->getFunction()->hasFnAttribute(Attribute::SanitizeMemory) || II->getFunction()->hasFnAttribute(Attribute::SanitizeHWAddress)) break; Index: compiler-rt/test/msan/loop-scope.cpp === --- /dev/null +++ compiler-rt/test/msan/loop-scope.cpp @@ -0,0 +1,18 @@ +// RUN: %clangxx_msan -O2 %s -o %t && \ +// RUN: not %run %t 2>&1 | FileCheck %s + +#include + +int *p; + +int main() { + for (int i = 0; i < 3; i++) { +int x; +if (i == 0) + x = 0; +p = + } + return *p; // BOOM + // CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value + // CHECK: #0 0x{{.*}} in main {{.*}}loop-scope.cpp:[[@LINE-2]] +} Index: clang/test/CodeGenCXX/lifetime-sanitizer.cpp === --- clang/test/CodeGenCXX/lifetime-sanitizer.cpp +++ clang/test/CodeGenCXX/lifetime-sanitizer.cpp @@ -3,6 +3,9 @@ // RUN: %clang -w -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 \ // RUN: -fsanitize=address -fsanitize-address-use-after-scope %s | \ // RUN: FileCheck %s -check-prefixes=CHECK,LIFETIME +// RUN: %clang -w -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 \ +// RUN: -fsanitize=memory %s | \ +// RUN: FileCheck %s -check-prefixes=CHECK,LIFETIME extern int bar(char *A, int n); Index: clang/test/CodeGen/lifetime-sanitizer.c === --- clang/test/CodeGen/lifetime-sanitizer.c +++ clang/test/CodeGen/lifetime-sanitizer.c @@ -2,6 +2,9 @@ // RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \ // RUN: -fsanitize=address -fsanitize-address-use-after-scope %s | \ // RUN: FileCheck %s -check-prefix=LIFETIME +// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \ +// RUN: -fsanitize=memory %s | \ +// RUN: FileCheck %s -check-prefix=LIFETIME extern int bar(char *A, int n); Index: clang/lib/CodeGen/CodeGenFunction.cpp === --- clang/lib/CodeGen/CodeGenFunction.cpp +++ clang/lib/CodeGen/CodeGenFunction.cpp @@ -47,13 +47,9 @@ if (CGOpts.DisableLifetimeMarkers) return false; - // Disable lifetime markers in msan builds. - // FIXME: Remove this when msan works with lifetime markers. - if (LangOpts.Sanitize.has(SanitizerKind::Memory)) -return false; - - // Asan uses markers for use-after-scope checks. - if (CGOpts.SanitizeAddressUseAfterScope) + // Sanitizers may use markers. + if (CGOpts.SanitizeAddressUseAfterScope || + LangOpts.Sanitize.has(SanitizerKind::Memory)) return true; // For now, only in optimized builds. Index: clang/lib/CodeGen/CGExpr.cpp === --- clang/lib/CodeGen/CGExpr.cpp +++ clang/lib/CodeGen/CGExpr.cpp @@ -516,13 +516,12 @@ // Avoid creating a conditional cleanup just to hold an llvm.lifetime.end // marker. Instead, start the lifetime of a conditional temporary earlier - // so that it's unconditional. Don't do this in ASan's use-after-scope - // mode so that it gets the more precise lifetime marks. If the type has - // a
[PATCH] D66695: msan, codegen, instcombine: Keep more lifetime markers used for msan
vitalybuka updated this revision to Diff 216991. vitalybuka added a comment. return hwasan Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66695/new/ https://reviews.llvm.org/D66695 Files: clang/lib/CodeGen/CGExpr.cpp clang/lib/CodeGen/CodeGenFunction.cpp clang/test/CodeGen/lifetime-sanitizer.c clang/test/CodeGenCXX/lifetime-sanitizer.cpp compiler-rt/test/msan/loop-scope.cpp llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll Index: llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll === --- llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll +++ llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll @@ -34,6 +34,21 @@ ret void } +define void @msan() sanitize_memory { +entry: + ; CHECK-LABEL: @msan( + %text = alloca i8, align 1 + + call void @llvm.lifetime.start.p0i8(i64 1, i8* %text) + call void @llvm.lifetime.end.p0i8(i64 1, i8* %text) + ; CHECK: call void @llvm.lifetime.start + ; CHECK-NEXT: call void @llvm.lifetime.end + + call void @foo(i8* %text) ; Keep alloca alive + + ret void +} + define void @no_asan() { entry: ; CHECK-LABEL: @no_asan( Index: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp === --- llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp +++ llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp @@ -3885,6 +3885,7 @@ // Asan needs to poison memory to detect invalid access which is possible // even for empty lifetime range. if (II->getFunction()->hasFnAttribute(Attribute::SanitizeAddress) || +II->getFunction()->hasFnAttribute(Attribute::SanitizeMemory) || II->getFunction()->hasFnAttribute(Attribute::SanitizeHWAddress)) break; Index: compiler-rt/test/msan/loop-scope.cpp === --- /dev/null +++ compiler-rt/test/msan/loop-scope.cpp @@ -0,0 +1,18 @@ +// RUN: %clangxx_msan -O2 %s -o %t && \ +// RUN: not %run %t 2>&1 | FileCheck %s + +#include + +int *p; + +int main() { + for (int i = 0; i < 3; i++) { +int x; +if (i == 0) + x = 0; +p = + } + return *p; // BOOM + // CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value + // CHECK: #0 0x{{.*}} in main {{.*}}loop-scope.cpp:[[@LINE-2]] +} Index: clang/test/CodeGenCXX/lifetime-sanitizer.cpp === --- clang/test/CodeGenCXX/lifetime-sanitizer.cpp +++ clang/test/CodeGenCXX/lifetime-sanitizer.cpp @@ -3,6 +3,9 @@ // RUN: %clang -w -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 \ // RUN: -fsanitize=address -fsanitize-address-use-after-scope %s | \ // RUN: FileCheck %s -check-prefixes=CHECK,LIFETIME +// RUN: %clang -w -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 \ +// RUN: -fsanitize=memory %s | \ +// RUN: FileCheck %s -check-prefixes=CHECK,LIFETIME extern int bar(char *A, int n); Index: clang/test/CodeGen/lifetime-sanitizer.c === --- clang/test/CodeGen/lifetime-sanitizer.c +++ clang/test/CodeGen/lifetime-sanitizer.c @@ -2,6 +2,9 @@ // RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \ // RUN: -fsanitize=address -fsanitize-address-use-after-scope %s | \ // RUN: FileCheck %s -check-prefix=LIFETIME +// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \ +// RUN: -fsanitize=memory %s | \ +// RUN: FileCheck %s -check-prefix=LIFETIME extern int bar(char *A, int n); Index: clang/lib/CodeGen/CodeGenFunction.cpp === --- clang/lib/CodeGen/CodeGenFunction.cpp +++ clang/lib/CodeGen/CodeGenFunction.cpp @@ -47,13 +47,9 @@ if (CGOpts.DisableLifetimeMarkers) return false; - // Disable lifetime markers in msan builds. - // FIXME: Remove this when msan works with lifetime markers. - if (LangOpts.Sanitize.has(SanitizerKind::Memory)) -return false; - - // Asan uses markers for use-after-scope checks. - if (CGOpts.SanitizeAddressUseAfterScope) + // Sanitizers may use markers. + if (CGOpts.SanitizeAddressUseAfterScope || + LangOpts.Sanitize.has(SanitizerKind::Memory) return true; // For now, only in optimized builds. Index: clang/lib/CodeGen/CGExpr.cpp === --- clang/lib/CodeGen/CGExpr.cpp +++ clang/lib/CodeGen/CGExpr.cpp @@ -523,6 +523,7 @@ ConditionalEvaluation *OldConditional = nullptr; CGBuilderTy::InsertPoint OldIP; if (isInConditionalBranch() && !E->getType().isDestructedType() && + !SanOpts.has(SanitizerKind::Memory) && !CGM.getCodeGenOpts().SanitizeAddressUseAfterScope) { OldConditional = OutermostConditional;
[PATCH] D66695: msan, codegen, instcombine: Keep more lifetime markers used for msan
vitalybuka updated this revision to Diff 216992. vitalybuka added a comment. fix compilation error Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66695/new/ https://reviews.llvm.org/D66695 Files: clang/lib/CodeGen/CGExpr.cpp clang/lib/CodeGen/CodeGenFunction.cpp clang/test/CodeGen/lifetime-sanitizer.c clang/test/CodeGenCXX/lifetime-sanitizer.cpp compiler-rt/test/msan/loop-scope.cpp llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll Index: llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll === --- llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll +++ llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll @@ -34,6 +34,21 @@ ret void } +define void @msan() sanitize_memory { +entry: + ; CHECK-LABEL: @msan( + %text = alloca i8, align 1 + + call void @llvm.lifetime.start.p0i8(i64 1, i8* %text) + call void @llvm.lifetime.end.p0i8(i64 1, i8* %text) + ; CHECK: call void @llvm.lifetime.start + ; CHECK-NEXT: call void @llvm.lifetime.end + + call void @foo(i8* %text) ; Keep alloca alive + + ret void +} + define void @no_asan() { entry: ; CHECK-LABEL: @no_asan( Index: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp === --- llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp +++ llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp @@ -3885,6 +3885,7 @@ // Asan needs to poison memory to detect invalid access which is possible // even for empty lifetime range. if (II->getFunction()->hasFnAttribute(Attribute::SanitizeAddress) || +II->getFunction()->hasFnAttribute(Attribute::SanitizeMemory) || II->getFunction()->hasFnAttribute(Attribute::SanitizeHWAddress)) break; Index: compiler-rt/test/msan/loop-scope.cpp === --- /dev/null +++ compiler-rt/test/msan/loop-scope.cpp @@ -0,0 +1,18 @@ +// RUN: %clangxx_msan -O2 %s -o %t && \ +// RUN: not %run %t 2>&1 | FileCheck %s + +#include + +int *p; + +int main() { + for (int i = 0; i < 3; i++) { +int x; +if (i == 0) + x = 0; +p = + } + return *p; // BOOM + // CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value + // CHECK: #0 0x{{.*}} in main {{.*}}loop-scope.cpp:[[@LINE-2]] +} Index: clang/test/CodeGenCXX/lifetime-sanitizer.cpp === --- clang/test/CodeGenCXX/lifetime-sanitizer.cpp +++ clang/test/CodeGenCXX/lifetime-sanitizer.cpp @@ -3,6 +3,9 @@ // RUN: %clang -w -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 \ // RUN: -fsanitize=address -fsanitize-address-use-after-scope %s | \ // RUN: FileCheck %s -check-prefixes=CHECK,LIFETIME +// RUN: %clang -w -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 \ +// RUN: -fsanitize=memory %s | \ +// RUN: FileCheck %s -check-prefixes=CHECK,LIFETIME extern int bar(char *A, int n); Index: clang/test/CodeGen/lifetime-sanitizer.c === --- clang/test/CodeGen/lifetime-sanitizer.c +++ clang/test/CodeGen/lifetime-sanitizer.c @@ -2,6 +2,9 @@ // RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \ // RUN: -fsanitize=address -fsanitize-address-use-after-scope %s | \ // RUN: FileCheck %s -check-prefix=LIFETIME +// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \ +// RUN: -fsanitize=memory %s | \ +// RUN: FileCheck %s -check-prefix=LIFETIME extern int bar(char *A, int n); Index: clang/lib/CodeGen/CodeGenFunction.cpp === --- clang/lib/CodeGen/CodeGenFunction.cpp +++ clang/lib/CodeGen/CodeGenFunction.cpp @@ -47,13 +47,9 @@ if (CGOpts.DisableLifetimeMarkers) return false; - // Disable lifetime markers in msan builds. - // FIXME: Remove this when msan works with lifetime markers. - if (LangOpts.Sanitize.has(SanitizerKind::Memory)) -return false; - - // Asan uses markers for use-after-scope checks. - if (CGOpts.SanitizeAddressUseAfterScope) + // Sanitizers may use markers. + if (CGOpts.SanitizeAddressUseAfterScope || + LangOpts.Sanitize.has(SanitizerKind::Memory)) return true; // For now, only in optimized builds. Index: clang/lib/CodeGen/CGExpr.cpp === --- clang/lib/CodeGen/CGExpr.cpp +++ clang/lib/CodeGen/CGExpr.cpp @@ -523,6 +523,7 @@ ConditionalEvaluation *OldConditional = nullptr; CGBuilderTy::InsertPoint OldIP; if (isInConditionalBranch() && !E->getType().isDestructedType() && + !SanOpts.has(SanitizerKind::Memory) && !CGM.getCodeGenOpts().SanitizeAddressUseAfterScope) { OldConditional = OutermostConditional;
[PATCH] D66695: msan, codegen, instcombine: Keep more lifetime markers used for msan
vitalybuka created this revision. vitalybuka added a reviewer: eugenis. Herald added subscribers: llvm-commits, Sanitizers, cfe-commits, hiraditya. Herald added projects: clang, Sanitizers, LLVM. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D66695 Files: clang/lib/CodeGen/CGExpr.cpp clang/lib/CodeGen/CodeGenFunction.cpp clang/test/CodeGen/lifetime-sanitizer.c clang/test/CodeGenCXX/lifetime-sanitizer.cpp compiler-rt/test/msan/loop-scope.cpp llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll Index: llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll === --- llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll +++ llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll @@ -34,6 +34,21 @@ ret void } +define void @msan() sanitize_memory { +entry: + ; CHECK-LABEL: @msan( + %text = alloca i8, align 1 + + call void @llvm.lifetime.start.p0i8(i64 1, i8* %text) + call void @llvm.lifetime.end.p0i8(i64 1, i8* %text) + ; CHECK: call void @llvm.lifetime.start + ; CHECK-NEXT: call void @llvm.lifetime.end + + call void @foo(i8* %text) ; Keep alloca alive + + ret void +} + define void @no_asan() { entry: ; CHECK-LABEL: @no_asan( Index: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp === --- llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp +++ llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp @@ -3885,7 +3885,7 @@ // Asan needs to poison memory to detect invalid access which is possible // even for empty lifetime range. if (II->getFunction()->hasFnAttribute(Attribute::SanitizeAddress) || -II->getFunction()->hasFnAttribute(Attribute::SanitizeHWAddress)) +II->getFunction()->hasFnAttribute(Attribute::SanitizeMemory)) break; if (removeTriviallyEmptyRange(*II, Intrinsic::lifetime_start, Index: compiler-rt/test/msan/loop-scope.cpp === --- /dev/null +++ compiler-rt/test/msan/loop-scope.cpp @@ -0,0 +1,18 @@ +// RUN: %clangxx_msan -O2 %s -o %t && \ +// RUN: not %run %t 2>&1 | FileCheck %s + +#include + +int *p; + +int main() { + for (int i = 0; i < 3; i++) { +int x; +if (i == 0) + x = 0; +p = + } + return *p; // BOOM + // CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value + // CHECK: #0 0x{{.*}} in main {{.*}}loop-scope.cpp:[[@LINE-2]] +} Index: clang/test/CodeGenCXX/lifetime-sanitizer.cpp === --- clang/test/CodeGenCXX/lifetime-sanitizer.cpp +++ clang/test/CodeGenCXX/lifetime-sanitizer.cpp @@ -3,6 +3,9 @@ // RUN: %clang -w -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 \ // RUN: -fsanitize=address -fsanitize-address-use-after-scope %s | \ // RUN: FileCheck %s -check-prefixes=CHECK,LIFETIME +// RUN: %clang -w -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 \ +// RUN: -fsanitize=memory %s | \ +// RUN: FileCheck %s -check-prefixes=CHECK,LIFETIME extern int bar(char *A, int n); Index: clang/test/CodeGen/lifetime-sanitizer.c === --- clang/test/CodeGen/lifetime-sanitizer.c +++ clang/test/CodeGen/lifetime-sanitizer.c @@ -2,6 +2,9 @@ // RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \ // RUN: -fsanitize=address -fsanitize-address-use-after-scope %s | \ // RUN: FileCheck %s -check-prefix=LIFETIME +// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \ +// RUN: -fsanitize=memory %s | \ +// RUN: FileCheck %s -check-prefix=LIFETIME extern int bar(char *A, int n); Index: clang/lib/CodeGen/CodeGenFunction.cpp === --- clang/lib/CodeGen/CodeGenFunction.cpp +++ clang/lib/CodeGen/CodeGenFunction.cpp @@ -47,13 +47,9 @@ if (CGOpts.DisableLifetimeMarkers) return false; - // Disable lifetime markers in msan builds. - // FIXME: Remove this when msan works with lifetime markers. - if (LangOpts.Sanitize.has(SanitizerKind::Memory)) -return false; - - // Asan uses markers for use-after-scope checks. - if (CGOpts.SanitizeAddressUseAfterScope) + // Sanitizers may use markers. + if (CGOpts.SanitizeAddressUseAfterScope || + LangOpts.Sanitize.has(SanitizerKind::Memory) return true; // For now, only in optimized builds. Index: clang/lib/CodeGen/CGExpr.cpp === --- clang/lib/CodeGen/CGExpr.cpp +++ clang/lib/CodeGen/CGExpr.cpp @@ -523,6 +523,7 @@ ConditionalEvaluation *OldConditional = nullptr; CGBuilderTy::InsertPoint OldIP; if (isInConditionalBranch() && !E->getType().isDestructedType() && + !SanOpts.has(SanitizerKind::Memory) &&