Skip to content

Commit 7f12000

Browse files
fmayerfmayer
committed
Support atomic write operations in stack safety
This has two benefits: * we can now mark allocas that are used in atomic operations as safe * this fixes a bug that would incorrectly mark all atomic writes as safe in HWASan instrumentation. this is because stack safety keeps a list of all *unsafe* operations that are reachable from an alloca, but it did not analyze atomic writes, so it would always mark them as safe. Reviewed By: vitalybuka Differential Revision: https://reviews.llvm.org/D159153
1 parent 7ce67d3 commit 7f12000

File tree

3 files changed

+162
-16
lines changed

3 files changed

+162
-16
lines changed

‎llvm/lib/Analysis/StackSafetyAnalysis.cpp

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ bool StackSafetyLocalAnalysis::isSafeAccess(const Use &U, AllocaInst *AI,
356356
const SCEV *AccessSize) {
357357

358358
if (!AI)
359-
return true;
359+
return true; // This only judges whether it is a safe *stack* access.
360360
if (isa<SCEVCouldNotCompute>(AccessSize))
361361
return false;
362362

@@ -408,6 +408,23 @@ void StackSafetyLocalAnalysis::analyzeAllUses(Value *Ptr,
408408

409409
assert(V == UI.get());
410410

411+
auto RecordStore = [&](const Value* StoredVal) {
412+
if (V == StoredVal) {
413+
// Stored the pointer - conservatively assume it may be unsafe.
414+
US.addRange(I, UnknownRange, /*IsSafe=*/false);
415+
return;
416+
}
417+
if (AI && !SL.isAliveAfter(AI, I)) {
418+
US.addRange(I, UnknownRange, /*IsSafe=*/false);
419+
return;
420+
}
421+
auto TypeSize = DL.getTypeStoreSize(StoredVal->getType());
422+
auto AccessRange = getAccessRange(UI, Ptr, TypeSize);
423+
bool Safe = isSafeAccess(UI, AI, TypeSize);
424+
US.addRange(I, AccessRange, Safe);
425+
return;
426+
};
427+
411428
switch (I->getOpcode()) {
412429
case Instruction::Load: {
413430
if (AI && !SL.isAliveAfter(AI, I)) {
@@ -424,22 +441,15 @@ void StackSafetyLocalAnalysis::analyzeAllUses(Value *Ptr,
424441
case Instruction::VAArg:
425442
// "va-arg" from a pointer is safe.
426443
break;
427-
case Instruction::Store: {
428-
if (V == I->getOperand(0)) {
429-
// Stored the pointer - conservatively assume it may be unsafe.
430-
US.addRange(I, UnknownRange, /*IsSafe=*/false);
431-
break;
432-
}
433-
if (AI && !SL.isAliveAfter(AI, I)) {
434-
US.addRange(I, UnknownRange, /*IsSafe=*/false);
435-
break;
436-
}
437-
auto TypeSize = DL.getTypeStoreSize(I->getOperand(0)->getType());
438-
auto AccessRange = getAccessRange(UI, Ptr, TypeSize);
439-
bool Safe = isSafeAccess(UI, AI, TypeSize);
440-
US.addRange(I, AccessRange, Safe);
444+
case Instruction::Store:
445+
RecordStore(cast<StoreInst>(I)->getValueOperand());
446+
break;
447+
case Instruction::AtomicCmpXchg:
448+
RecordStore(cast<AtomicCmpXchgInst>(I)->getNewValOperand());
449+
break;
450+
case Instruction::AtomicRMW:
451+
RecordStore(cast<AtomicRMWInst>(I)->getValOperand());
441452
break;
442-
}
443453

444454
case Instruction::Ret:
445455
// Information leak.
@@ -986,6 +996,7 @@ void StackSafetyGlobalInfo::print(raw_ostream &O) const {
986996
for (const auto &I : instructions(F)) {
987997
const CallInst *Call = dyn_cast<CallInst>(&I);
988998
if ((isa<StoreInst>(I) || isa<LoadInst>(I) || isa<MemIntrinsic>(I) ||
999+
isa<AtomicCmpXchgInst>(I) || isa<AtomicRMWInst>(I) ||
9891000
(Call && Call->hasByValArgument())) &&
9901001
stackAccessIsSafe(I)) {
9911002
O << " " << I << "\n";

‎llvm/test/Analysis/StackSafetyAnalysis/local.ll

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1039,5 +1039,72 @@ entry:
10391039
ret void
10401040
}
10411041

1042+
define void @Cmpxchg4Arg(ptr %p) {
1043+
; CHECK-LABEL: @Cmpxchg4Arg
1044+
; CHECK-NEXT: args uses:
1045+
; CHECK-NEXT: p[]: [0,4){{$}}
1046+
; CHECK-NEXT: allocas uses:
1047+
; GLOBAL-NEXT: safe accesses:
1048+
; GLOBAL-NEXT: cmpxchg ptr %p, i32 0, i32 1 monotonic monotonic, align 1
1049+
; CHECK-EMPTY:
1050+
entry:
1051+
cmpxchg ptr %p, i32 0, i32 1 monotonic monotonic, align 1
1052+
ret void
1053+
}
1054+
1055+
define void @AtomicRMW4Arg(ptr %p) {
1056+
; CHECK-LABEL: @AtomicRMW4Arg
1057+
; CHECK-NEXT: args uses:
1058+
; CHECK-NEXT: p[]: [0,4){{$}}
1059+
; CHECK-NEXT: allocas uses:
1060+
; GLOBAL-NEXT: safe accesses:
1061+
; GLOBAL-NEXT: atomicrmw add ptr %p, i32 1 monotonic, align 1
1062+
; CHECK-EMPTY:
1063+
entry:
1064+
atomicrmw add ptr %p, i32 1 monotonic, align 1
1065+
ret void
1066+
}
1067+
1068+
define void @Cmpxchg4Alloca() {
1069+
; CHECK-LABEL: @Cmpxchg4Alloca
1070+
; CHECK-NEXT: args uses:
1071+
; CHECK-NEXT: allocas uses:
1072+
; CHECK-NEXT: x[4]: [0,4){{$}}
1073+
; GLOBAL-NEXT: safe accesses:
1074+
; GLOBAL-NEXT: cmpxchg ptr %x, i32 0, i32 1 monotonic monotonic, align 1
1075+
; CHECK-EMPTY:
1076+
entry:
1077+
%x = alloca i32, align 4
1078+
cmpxchg ptr %x, i32 0, i32 1 monotonic monotonic, align 1
1079+
ret void
1080+
}
1081+
1082+
define void @AtomicRMW4Alloca() {
1083+
; CHECK-LABEL: @AtomicRMW4Alloca
1084+
; CHECK-NEXT: args uses:
1085+
; CHECK-NEXT: allocas uses:
1086+
; CHECK-NEXT: x[4]: [0,4){{$}}
1087+
; GLOBAL-NEXT: safe accesses:
1088+
; GLOBAL-NEXT: atomicrmw add ptr %x, i32 1 monotonic, align 1
1089+
; CHECK-EMPTY:
1090+
entry:
1091+
%x = alloca i32, align 4
1092+
atomicrmw add ptr %x, i32 1 monotonic, align 1
1093+
ret void
1094+
}
1095+
1096+
define void @StoreArg(ptr %p) {
1097+
; CHECK-LABEL: @StoreArg
1098+
; CHECK-NEXT: args uses:
1099+
; CHECK-NEXT: p[]: [0,4){{$}}
1100+
; CHECK-NEXT: allocas uses:
1101+
; GLOBAL-NEXT: safe accesses:
1102+
; GLOBAL-NEXT: store i32 1, ptr %p
1103+
; CHECK-EMPTY:
1104+
entry:
1105+
store i32 1, ptr %p
1106+
ret void
1107+
}
1108+
10421109
declare void @llvm.lifetime.start.p0(i64, ptr nocapture)
10431110
declare void @llvm.lifetime.end.p0(i64, ptr nocapture)

‎llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,40 @@ entry:
2323
ret i32 0
2424
}
2525

26+
; Check a safe alloca to ensure it does not get a tag.
27+
define i32 @test_cmpxchg(ptr %a) sanitize_hwaddress {
28+
entry:
29+
; CHECK-LABEL: @test_cmpxchg
30+
; NOSAFETY: call {{.*}}__hwasan_generate_tag
31+
; NOSAFETY: call {{.*}}__hwasan_store
32+
; SAFETY-NOT: call {{.*}}__hwasan_generate_tag
33+
; SAFETY-NOT: call {{.*}}__hwasan_store
34+
; NOSTACK-NOT: call {{.*}}__hwasan_generate_tag
35+
; NOSTACK-NOT: call {{.*}}__hwasan_store
36+
%buf.sroa.0 = alloca i8, align 4
37+
call void @llvm.lifetime.start.p0(i64 1, ptr nonnull %buf.sroa.0)
38+
%0 = cmpxchg ptr %buf.sroa.0, i8 1, i8 2 monotonic monotonic, align 4
39+
call void @llvm.lifetime.end.p0(i64 1, ptr nonnull %buf.sroa.0)
40+
ret i32 0
41+
}
42+
43+
; Check a safe alloca to ensure it does not get a tag.
44+
define i32 @test_atomicrwm(ptr %a) sanitize_hwaddress {
45+
entry:
46+
; CHECK-LABEL: @test_atomicrwm
47+
; NOSAFETY: call {{.*}}__hwasan_generate_tag
48+
; NOSAFETY: call {{.*}}__hwasan_store
49+
; SAFETY-NOT: call {{.*}}__hwasan_generate_tag
50+
; SAFETY-NOT: call {{.*}}__hwasan_store
51+
; NOSTACK-NOT: call {{.*}}__hwasan_generate_tag
52+
; NOSTACK-NOT: call {{.*}}__hwasan_store
53+
%buf.sroa.0 = alloca i8, align 4
54+
call void @llvm.lifetime.start.p0(i64 1, ptr nonnull %buf.sroa.0)
55+
%0 = atomicrmw add ptr %buf.sroa.0, i8 1 monotonic, align 4
56+
call void @llvm.lifetime.end.p0(i64 1, ptr nonnull %buf.sroa.0)
57+
ret i32 0
58+
}
59+
2660
; Check a non-safe alloca to ensure it gets a tag.
2761
define i32 @test_use(ptr %a) sanitize_hwaddress {
2862
entry:
@@ -141,6 +175,23 @@ entry:
141175
ret i32 0
142176
}
143177

178+
define i32 @test_out_of_range2(ptr %a) sanitize_hwaddress {
179+
entry:
180+
; CHECK-LABEL: @test_out_of_range2
181+
; NOSAFETY: call {{.*}}__hwasan_generate_tag
182+
; NOSAFETY: call {{.*}}__hwasan_store
183+
; SAFETY: call {{.*}}__hwasan_generate_tag
184+
; SAFETY: call {{.*}}__hwasan_store
185+
; NOSTACK-NOT: call {{.*}}__hwasan_generate_tag
186+
; NOSTACK-NOT: call {{.*}}__hwasan_store
187+
%buf.sroa.0 = alloca [10 x i8], align 4
188+
%ptr = getelementptr [10 x i8], ptr %buf.sroa.0, i32 0, i32 10
189+
call void @llvm.lifetime.start.p0(i64 10, ptr nonnull %buf.sroa.0)
190+
%0 = cmpxchg ptr %ptr, i8 1, i8 2 monotonic monotonic, align 4
191+
call void @llvm.lifetime.end.p0(i64 10, ptr nonnull %buf.sroa.0)
192+
ret i32 0
193+
}
194+
144195
define i32 @test_out_of_range3(ptr %a) sanitize_hwaddress {
145196
entry:
146197
; CHECK-LABEL: @test_out_of_range3
@@ -192,6 +243,23 @@ entry:
192243
ret i32 0
193244
}
194245

246+
define i32 @test_out_of_range6(ptr %a) sanitize_hwaddress {
247+
entry:
248+
; CHECK-LABEL: @test_out_of_range6
249+
; NOSAFETY: call {{.*}}__hwasan_generate_tag
250+
; NOSAFETY: call {{.*}}__hwasan_store
251+
; SAFETY: call {{.*}}__hwasan_generate_tag
252+
; SAFETY: call {{.*}}__hwasan_store
253+
; NOSTACK-NOT: call {{.*}}__hwasan_generate_tag
254+
; NOSTACK-NOT: call {{.*}}__hwasan_store
255+
%buf.sroa.0 = alloca [10 x i8], align 4
256+
%ptr = getelementptr [10 x i8], ptr %buf.sroa.0, i32 0, i32 10
257+
call void @llvm.lifetime.start.p0(i64 10, ptr nonnull %buf.sroa.0)
258+
%0 = atomicrmw add ptr %ptr, i32 1 monotonic, align 4
259+
call void @llvm.lifetime.end.p0(i64 10, ptr nonnull %buf.sroa.0)
260+
ret i32 0
261+
}
262+
195263
; Check an alloca with potentially out of range GEP to ensure it gets a tag and
196264
; check.
197265
define i32 @test_potentially_out_of_range(ptr %a) sanitize_hwaddress {

0 commit comments

Comments
 (0)