diff options
author | Fangrui Song <i@maskray.me> | 2024-02-15 10:07:30 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-02-15 10:07:30 -0800 |
commit | aa265c45914fcb80da787be5cb8e97d94f1b2859 (patch) | |
tree | e55547c760c0fb3b6159653dc07985b1a9bc8f93 | |
parent | b1d4f996898becb1867dd2647cf2728bd44170e3 (diff) | |
download | llvm-users/MaskRay/spr/main.asan-isinterestingalloca-remove-the-isallocapromotable-condition.zip llvm-users/MaskRay/spr/main.asan-isinterestingalloca-remove-the-isallocapromotable-condition.tar.gz llvm-users/MaskRay/spr/main.asan-isinterestingalloca-remove-the-isallocapromotable-condition.tar.bz2 |
[asan] isInterestingAlloca: remove the isAllocaPromotable condition (#77221)users/MaskRay/spr/main.asan-isinterestingalloca-remove-the-isallocapromotable-condition
Commit 8ed1d8196bef89c3099be4ce4aa65f613ab819cc made an AllocaInst
interesting only if
`!ClSkipPromotableAllocas || !isAllocaPromotable(&AI)`, which greatly
removed memory operand instrumention for -O0. However, this optimization
is subsumed by StackSafetyAnalysis and therefore unnecessary when we
enable StackSafetyAnalysis by default. With this patch,
-fsanitize=address built clang does not change with -O0 or -O3.
Actually, having the `!ClSkipPromotableAllocas ||
!isAllocaPromotable(&AI)`
condition before `!(SSGI && SSGI->isSafe(AI)))` has an interesting false
positive involving MemIntrinsic (see `hoist-argument-init-insts.ll`):
* `isInterestingAlloca` is transitively called by
`getInterestingMemoryOperands` and `FunctionStackPoisoner`.
* If `getInterestingMemoryOperands` never calls
`StackSafetyGlobalInfo::getInfo`, and a MemIntrinsic is converted to
`__asan_memcpy` by `instrumentMemIntrinsic`, when
`StackSafetyGlobalInfo::getInfo` is called, StackSafetyAnalysis will
consider `__asan_memcpy` as unsafe, leading to an unnecessary
alloca instrumentation
4 files changed, 8 insertions, 15 deletions
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp index 8a2864a..4c29e02 100644 --- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp @@ -345,11 +345,6 @@ static cl::opt<bool> cl::desc("instrument dynamic allocas"), cl::Hidden, cl::init(true)); -static cl::opt<bool> ClSkipPromotableAllocas( - "asan-skip-promotable-allocas", - cl::desc("Do not instrument promotable allocas"), cl::Hidden, - cl::init(true)); - static cl::opt<AsanCtorKind> ClConstructorKind( "asan-constructor-kind", cl::desc("Sets the ASan constructor kind"), @@ -1279,9 +1274,6 @@ bool AddressSanitizer::isInterestingAlloca(const AllocaInst &AI) { (AI.getAllocatedType()->isSized() && // alloca() may be called with 0 size, ignore it. ((!AI.isStaticAlloca()) || !getAllocaSizeInBytes(AI).isZero()) && - // We are only interested in allocas not promotable to registers. - // Promotable allocas are common under -O0. - (!ClSkipPromotableAllocas || !isAllocaPromotable(&AI)) && // inalloca allocas are not treated as static, and we don't want // dynamic alloca instrumentation for them as well. !AI.isUsedWithInAlloca() && @@ -1312,7 +1304,7 @@ bool AddressSanitizer::ignoreAccess(Instruction *Inst, Value *Ptr) { // will not cause memory violations. This greatly speeds up the instrumented // executable at -O0. if (auto AI = dyn_cast_or_null<AllocaInst>(Ptr)) - if (ClSkipPromotableAllocas && !isInterestingAlloca(*AI)) + if (!isInterestingAlloca(*AI)) return true; if (SSGI != nullptr && SSGI->stackAccessIsSafe(*Inst) && diff --git a/llvm/test/Instrumentation/AddressSanitizer/debug_info_noninstrumented_alloca.ll b/llvm/test/Instrumentation/AddressSanitizer/debug_info_noninstrumented_alloca.ll index 6237673..1f1049d 100644 --- a/llvm/test/Instrumentation/AddressSanitizer/debug_info_noninstrumented_alloca.ll +++ b/llvm/test/Instrumentation/AddressSanitizer/debug_info_noninstrumented_alloca.ll @@ -10,19 +10,19 @@ target triple = "x86_64-apple-macosx10.10.0" define i32 @foo() sanitize_address { entry: - ; Won't be instrumented because of asan-skip-promotable-allocas. + ; Won't be instrumented because of asan-use-safety-analysis. %non_instrumented1 = alloca i32, align 4 ; Regular alloca, will get instrumented (forced by the ptrtoint below). %instrumented = alloca i32, align 4 - ; Won't be instrumented because of asan-skip-promotable-allocas. + ; Won't be instrumented because of asan-use-safety-analysis. %non_instrumented2 = alloca i32, align 4 br label %bb0 bb0: - ; Won't be instrumented because of asan-skip-promotable-allocas. + ; Won't be instrumented because of asan-use-safety-analysis. %non_instrumented3 = alloca i32, align 4 %ptr = ptrtoint ptr %instrumented to i32 diff --git a/llvm/test/Instrumentation/AddressSanitizer/hoist-argument-init-insts.ll b/llvm/test/Instrumentation/AddressSanitizer/hoist-argument-init-insts.ll index 5ecd4dc..85bfe76 100644 --- a/llvm/test/Instrumentation/AddressSanitizer/hoist-argument-init-insts.ll +++ b/llvm/test/Instrumentation/AddressSanitizer/hoist-argument-init-insts.ll @@ -1,4 +1,4 @@ -; RUN: opt < %s -passes=asan -asan-use-after-scope -asan-use-stack-safety=1 -S | FileCheck %s +; RUN: opt < %s -passes=asan -asan-use-after-scope -asan-use-stack-safety=1 -S | FileCheck %s --implicit-check-not=__asan_stack_malloc ; Source (-O0 -fsanitize=address -fsanitize-address-use-after-scope): ;; struct S { int x, y; }; @@ -21,13 +21,14 @@ target triple = "x86_64-apple-macosx10.14.0" ; CHECK: %a.addr = alloca ptr, align 8 ; CHECK-NEXT: %b.addr = alloca ptr, align 8 ; CHECK-NEXT: %doit.addr = alloca i8, align 1 +; CHECK-NEXT: %tmp = alloca %struct.S, align 4 ; Next, the stores into the argument allocas. ; CHECK-NEXT: store ptr {{.*}}, ptr %a.addr ; CHECK-NEXT: store ptr {{.*}}, ptr %b.addr ; CHECK-NEXT: [[frombool:%.*]] = zext i1 {{.*}} to i8 ; CHECK-NEXT: store i8 %frombool, ptr %doit.addr, align 1 -; CHECK-NEXT: [[stack_base:%.*]] = alloca i64, align 8 +; CHECK-NOT: alloca i64, align 8 define void @_Z4swapP1SS0_b(ptr %a, ptr %b, i1 zeroext %doit) sanitize_address { entry: diff --git a/llvm/test/Instrumentation/AddressSanitizer/local_stack_base.ll b/llvm/test/Instrumentation/AddressSanitizer/local_stack_base.ll index 4e8466b..8e30fc2 100644 --- a/llvm/test/Instrumentation/AddressSanitizer/local_stack_base.ll +++ b/llvm/test/Instrumentation/AddressSanitizer/local_stack_base.ll @@ -1,4 +1,4 @@ -; RUN: opt -S -passes=asan -asan-use-stack-safety=0 -asan-skip-promotable-allocas=0 %s -o - | FileCheck %s +; RUN: opt -S -passes=asan -asan-use-stack-safety=0 %s -o - | FileCheck %s ; Generated from: ; int bar(int y) { ; return y + 2; |