Skip to content

Commit aa265c4

Browse files
authored
[asan] isInterestingAlloca: remove the isAllocaPromotable condition (#77221)
Commit 8ed1d81 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
1 parent b1d4f99 commit aa265c4

File tree

4 files changed

+8
-15
lines changed

4 files changed

+8
-15
lines changed

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -345,11 +345,6 @@ static cl::opt<bool>
345345
cl::desc("instrument dynamic allocas"),
346346
cl::Hidden, cl::init(true));
347347

348-
static cl::opt<bool> ClSkipPromotableAllocas(
349-
"asan-skip-promotable-allocas",
350-
cl::desc("Do not instrument promotable allocas"), cl::Hidden,
351-
cl::init(true));
352-
353348
static cl::opt<AsanCtorKind> ClConstructorKind(
354349
"asan-constructor-kind",
355350
cl::desc("Sets the ASan constructor kind"),
@@ -1279,9 +1274,6 @@ bool AddressSanitizer::isInterestingAlloca(const AllocaInst &AI) {
12791274
(AI.getAllocatedType()->isSized() &&
12801275
// alloca() may be called with 0 size, ignore it.
12811276
((!AI.isStaticAlloca()) || !getAllocaSizeInBytes(AI).isZero()) &&
1282-
// We are only interested in allocas not promotable to registers.
1283-
// Promotable allocas are common under -O0.
1284-
(!ClSkipPromotableAllocas || !isAllocaPromotable(&AI)) &&
12851277
// inalloca allocas are not treated as static, and we don't want
12861278
// dynamic alloca instrumentation for them as well.
12871279
!AI.isUsedWithInAlloca() &&
@@ -1312,7 +1304,7 @@ bool AddressSanitizer::ignoreAccess(Instruction *Inst, Value *Ptr) {
13121304
// will not cause memory violations. This greatly speeds up the instrumented
13131305
// executable at -O0.
13141306
if (auto AI = dyn_cast_or_null<AllocaInst>(Ptr))
1315-
if (ClSkipPromotableAllocas && !isInterestingAlloca(*AI))
1307+
if (!isInterestingAlloca(*AI))
13161308
return true;
13171309

13181310
if (SSGI != nullptr && SSGI->stackAccessIsSafe(*Inst) &&

llvm/test/Instrumentation/AddressSanitizer/debug_info_noninstrumented_alloca.ll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,19 @@ target triple = "x86_64-apple-macosx10.10.0"
1010

1111
define i32 @foo() sanitize_address {
1212
entry:
13-
; Won't be instrumented because of asan-skip-promotable-allocas.
13+
; Won't be instrumented because of asan-use-safety-analysis.
1414
%non_instrumented1 = alloca i32, align 4
1515

1616
; Regular alloca, will get instrumented (forced by the ptrtoint below).
1717
%instrumented = alloca i32, align 4
1818

19-
; Won't be instrumented because of asan-skip-promotable-allocas.
19+
; Won't be instrumented because of asan-use-safety-analysis.
2020
%non_instrumented2 = alloca i32, align 4
2121

2222
br label %bb0
2323

2424
bb0:
25-
; Won't be instrumented because of asan-skip-promotable-allocas.
25+
; Won't be instrumented because of asan-use-safety-analysis.
2626
%non_instrumented3 = alloca i32, align 4
2727

2828
%ptr = ptrtoint ptr %instrumented to i32

llvm/test/Instrumentation/AddressSanitizer/hoist-argument-init-insts.ll

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
; RUN: opt < %s -passes=asan -asan-use-after-scope -asan-use-stack-safety=1 -S | FileCheck %s
1+
; RUN: opt < %s -passes=asan -asan-use-after-scope -asan-use-stack-safety=1 -S | FileCheck %s --implicit-check-not=__asan_stack_malloc
22

33
; Source (-O0 -fsanitize=address -fsanitize-address-use-after-scope):
44
;; struct S { int x, y; };
@@ -21,13 +21,14 @@ target triple = "x86_64-apple-macosx10.14.0"
2121
; CHECK: %a.addr = alloca ptr, align 8
2222
; CHECK-NEXT: %b.addr = alloca ptr, align 8
2323
; CHECK-NEXT: %doit.addr = alloca i8, align 1
24+
; CHECK-NEXT: %tmp = alloca %struct.S, align 4
2425

2526
; Next, the stores into the argument allocas.
2627
; CHECK-NEXT: store ptr {{.*}}, ptr %a.addr
2728
; CHECK-NEXT: store ptr {{.*}}, ptr %b.addr
2829
; CHECK-NEXT: [[frombool:%.*]] = zext i1 {{.*}} to i8
2930
; CHECK-NEXT: store i8 %frombool, ptr %doit.addr, align 1
30-
; CHECK-NEXT: [[stack_base:%.*]] = alloca i64, align 8
31+
; CHECK-NOT: alloca i64, align 8
3132

3233
define void @_Z4swapP1SS0_b(ptr %a, ptr %b, i1 zeroext %doit) sanitize_address {
3334
entry:

llvm/test/Instrumentation/AddressSanitizer/local_stack_base.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
; RUN: opt -S -passes=asan -asan-use-stack-safety=0 -asan-skip-promotable-allocas=0 %s -o - | FileCheck %s
1+
; RUN: opt -S -passes=asan -asan-use-stack-safety=0 %s -o - | FileCheck %s
22
; Generated from:
33
; int bar(int y) {
44
; return y + 2;

0 commit comments

Comments
 (0)