Skip to content

Commit 36daf07

Browse files
fmayerfmayer
committed
[hwasan] also omit safe mem[cpy|mov|set].
Reviewed By: eugenis Differential Revision: https://reviews.llvm.org/D109816
1 parent 4ca1fbe commit 36daf07

File tree

8 files changed

+237
-19
lines changed

8 files changed

+237
-19
lines changed

‎llvm/include/llvm/Analysis/StackSafetyAnalysis.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,12 @@ class StackSafetyGlobalInfo {
7878
// Whether we can prove that all accesses to this Alloca are in-range and
7979
// during its lifetime.
8080
bool isSafe(const AllocaInst &AI) const;
81-
// Whether we can prove that an instruction only accesses a live alloca in
82-
// range.
83-
bool accessIsSafe(const Instruction &I) const;
81+
82+
// Returns true if the instruction can be proven to do only two types of
83+
// memory accesses:
84+
// (1) live stack locations in-bounds or
85+
// (2) non-stack locations.
86+
bool stackAccessIsSafe(const Instruction &I) const;
8487
void print(raw_ostream &O) const;
8588
void dump() const;
8689
};

‎llvm/lib/Analysis/StackSafetyAnalysis.cpp

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ struct StackSafetyInfo::InfoTy {
230230
struct StackSafetyGlobalInfo::InfoTy {
231231
GVToSSI Info;
232232
SmallPtrSet<const AllocaInst *, 8> SafeAllocas;
233-
SmallPtrSet<const Instruction *, 8> SafeAccesses;
233+
std::map<const Instruction *, bool> AccessIsUnsafe;
234234
};
235235

236236
namespace {
@@ -820,7 +820,6 @@ const StackSafetyGlobalInfo::InfoTy &StackSafetyGlobalInfo::getInfo() const {
820820
Info.reset(new InfoTy{
821821
createGlobalStackSafetyInfo(std::move(Functions), Index), {}, {}});
822822

823-
std::map<const Instruction *, bool> AccessIsUnsafe;
824823
for (auto &FnKV : Info->Info) {
825824
for (auto &KV : FnKV.second.Allocas) {
826825
++NumAllocaTotal;
@@ -831,14 +830,10 @@ const StackSafetyGlobalInfo::InfoTy &StackSafetyGlobalInfo::getInfo() const {
831830
++NumAllocaStackSafe;
832831
}
833832
for (const auto &A : KV.second.Accesses)
834-
AccessIsUnsafe[A.first] |= !AIRange.contains(A.second);
833+
Info->AccessIsUnsafe[A.first] |= !AIRange.contains(A.second);
835834
}
836835
}
837836

838-
for (const auto &KV : AccessIsUnsafe)
839-
if (!KV.second)
840-
Info->SafeAccesses.insert(KV.first);
841-
842837
if (StackSafetyPrint)
843838
print(errs());
844839
}
@@ -908,9 +903,13 @@ bool StackSafetyGlobalInfo::isSafe(const AllocaInst &AI) const {
908903
return Info.SafeAllocas.count(&AI);
909904
}
910905

911-
bool StackSafetyGlobalInfo::accessIsSafe(const Instruction &I) const {
906+
bool StackSafetyGlobalInfo::stackAccessIsSafe(const Instruction &I) const {
912907
const auto &Info = getInfo();
913-
return Info.SafeAccesses.count(&I);
908+
auto It = Info.AccessIsUnsafe.find(&I);
909+
if (It == Info.AccessIsUnsafe.end()) {
910+
return true;
911+
}
912+
return !It->second;
914913
}
915914

916915
void StackSafetyGlobalInfo::print(raw_ostream &O) const {
@@ -924,7 +923,10 @@ void StackSafetyGlobalInfo::print(raw_ostream &O) const {
924923
O << " safe accesses:"
925924
<< "\n";
926925
for (const auto &I : instructions(F)) {
927-
if (accessIsSafe(I)) {
926+
const CallInst *Call = dyn_cast<CallInst>(&I);
927+
if ((isa<StoreInst>(I) || isa<LoadInst>(I) || isa<MemIntrinsic>(I) ||
928+
(Call && Call->hasByValArgument())) &&
929+
stackAccessIsSafe(I)) {
928930
O << " " << I << "\n";
929931
}
930932
}

‎llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,7 @@ class HWAddressSanitizer {
290290
void instrumentMemAccessInline(Value *Ptr, bool IsWrite,
291291
unsigned AccessSizeIndex,
292292
Instruction *InsertBefore);
293+
bool ignoreMemIntrinsic(MemIntrinsic *MI);
293294
void instrumentMemIntrinsic(MemIntrinsic *MI);
294295
bool instrumentMemAccess(InterestingMemoryOperand &O);
295296
bool ignoreAccess(Instruction *Inst, Value *Ptr);
@@ -798,11 +799,11 @@ bool HWAddressSanitizer::ignoreAccess(Instruction *Inst, Value *Ptr) {
798799
if (Ptr->isSwiftError())
799800
return true;
800801

801-
if (!InstrumentStack) {
802-
if (findAllocaForValue(Ptr))
802+
if (findAllocaForValue(Ptr)) {
803+
if (!InstrumentStack)
804+
return true;
805+
if (SSI && SSI->stackAccessIsSafe(*Inst))
803806
return true;
804-
} else if (SSI && SSI->accessIsSafe(*Inst)) {
805-
return true;
806807
}
807808
return false;
808809
}
@@ -994,6 +995,16 @@ void HWAddressSanitizer::instrumentMemAccessInline(Value *Ptr, bool IsWrite,
994995
cast<BranchInst>(CheckFailTerm)->setSuccessor(0, CheckTerm->getParent());
995996
}
996997

998+
bool HWAddressSanitizer::ignoreMemIntrinsic(MemIntrinsic *MI) {
999+
if (MemTransferInst *MTI = dyn_cast<MemTransferInst>(MI)) {
1000+
return (!ClInstrumentWrites || ignoreAccess(MTI, MTI->getDest())) &&
1001+
(!ClInstrumentReads || ignoreAccess(MTI, MTI->getSource()));
1002+
}
1003+
if (isa<MemSetInst>(MI))
1004+
return !ClInstrumentWrites || ignoreAccess(MI, MI->getDest());
1005+
return false;
1006+
}
1007+
9971008
void HWAddressSanitizer::instrumentMemIntrinsic(MemIntrinsic *MI) {
9981009
IRBuilder<> IRB(MI);
9991010
if (isa<MemTransferInst>(MI)) {
@@ -1542,7 +1553,8 @@ bool HWAddressSanitizer::sanitizeFunction(
15421553
getInterestingMemoryOperands(&Inst, OperandsToInstrument);
15431554

15441555
if (MemIntrinsic *MI = dyn_cast<MemIntrinsic>(&Inst))
1545-
IntrinToInstrument.push_back(MI);
1556+
if (!ignoreMemIntrinsic(MI))
1557+
IntrinToInstrument.push_back(MI);
15461558
}
15471559
}
15481560

‎llvm/test/Analysis/StackSafetyAnalysis/ipa-alias.ll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,4 +136,5 @@ entry:
136136
; CHECK-NEXT: p[]: [0,1){{$}}
137137
; CHECK-NEXT: allocas uses:
138138
; GLOBAL-NEXT: safe accesses:
139+
; GLOBAL-NEXT: store i8 0, i8* %p, align 1
139140
; CHECK-EMPTY:

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,7 @@ define private void @PrivateWrite1(i8* %p) #0 {
272272
; CHECK-NEXT: p[]: [0,1){{$}}
273273
; CHECK-NEXT: allocas uses:
274274
; GLOBAL-NEXT: safe accesses:
275+
; GLOBAL-NEXT: store i8 0, i8* %p, align 1
275276
; CHECK-EMPTY:
276277
entry:
277278
store i8 0, i8* %p, align 1
@@ -563,13 +564,15 @@ entry:
563564
; CHECK-NEXT: p[]: [0,1){{$}}
564565
; CHECK-NEXT: allocas uses:
565566
; GLOBAL-NEXT: safe accesses:
567+
; GLOBAL-NEXT: store i8 0, i8* %p, align 1
566568
; CHECK-EMPTY:
567569

568570
; CHECK-LABEL: @Write4{{$}}
569571
; CHECK-NEXT: args uses:
570572
; CHECK-NEXT: p[]: [0,4){{$}}
571573
; CHECK-NEXT: allocas uses:
572574
; GLOBAL-NEXT: safe accesses:
575+
; GLOBAL-NEXT: store i32 0, i32* %0, align 1
573576
; CHECK-EMPTY:
574577

575578
; CHECK-LABEL: @Write4_2{{$}}
@@ -578,34 +581,40 @@ entry:
578581
; CHECK-NEXT: q[]: [0,4){{$}}
579582
; CHECK-NEXT: allocas uses:
580583
; GLOBAL-NEXT: safe accesses:
584+
; GLOBAL-NEXT: store i32 0, i32* %0, align 1
585+
; GLOBAL-NEXT: store i32 0, i32* %1, align 1
581586
; CHECK-EMPTY:
582587

583588
; CHECK-LABEL: @Write8{{$}}
584589
; CHECK-NEXT: args uses:
585590
; CHECK-NEXT: p[]: [0,8){{$}}
586591
; CHECK-NEXT: allocas uses:
587592
; GLOBAL-NEXT: safe accesses:
593+
; GLOBAL-NEXT: store i64 0, i64* %0, align 1
588594
; CHECK-EMPTY:
589595

590596
; CHECK-LABEL: @WriteAndReturn8{{$}}
591597
; CHECK-NEXT: args uses:
592598
; CHECK-NEXT: p[]: full-set{{$}}
593599
; CHECK-NEXT: allocas uses:
594600
; GLOBAL-NEXT: safe accesses:
601+
; GLOBAL-NEXT: store i8 0, i8* %p, align 1
595602
; CHECK-EMPTY:
596603

597604
; CHECK-LABEL: @PreemptableWrite1 dso_preemptable{{$}}
598605
; CHECK-NEXT: args uses:
599606
; CHECK-NEXT: p[]: [0,1){{$}}
600607
; CHECK-NEXT: allocas uses:
601608
; GLOBAL-NEXT: safe accesses:
609+
; GLOBAL-NEXT: store i8 0, i8* %p, align 1
602610
; CHECK-EMPTY:
603611

604612
; CHECK-LABEL: @InterposableWrite1 interposable{{$}}
605613
; CHECK-NEXT: args uses:
606614
; CHECK-NEXT: p[]: [0,1){{$}}
607615
; CHECK-NEXT: allocas uses:
608616
; GLOBAL-NEXT: safe accesses:
617+
; GLOBAL-NEXT: store i8 0, i8* %p, align 1
609618
; CHECK-EMPTY:
610619

611620
; CHECK-LABEL: @ReturnDependent{{$}}
@@ -646,6 +655,9 @@ entry:
646655
; CHECK-NEXT: acc[]: [0,4), @RecursiveNoOffset(arg2, [0,1)){{$}}
647656
; CHECK-NEXT: allocas uses:
648657
; GLOBAL-NEXT: safe accesses:
658+
; GLOBAL-NEXT: %0 = load i32, i32* %p, align 4
659+
; GLOBAL-NEXT: %1 = load i32, i32* %acc, align 4
660+
; GLOBAL-NEXT: store i32 %add, i32* %acc, align 4
649661
; CHECK-EMPTY:
650662

651663
; CHECK-LABEL: @RecursiveWithOffset{{$}}
@@ -654,6 +666,7 @@ entry:
654666
; GLOBAL-NEXT: acc[]: full-set, @RecursiveWithOffset(arg1, [4,5)){{$}}
655667
; CHECK-NEXT: allocas uses:
656668
; GLOBAL-NEXT: safe accesses:
669+
; GLOBAL-NEXT: store i32 0, i32* %acc, align 4
657670
; CHECK-EMPTY:
658671

659672
; CHECK-LABEL: @ReturnAlloca

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

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,8 @@ define dso_local void @WriteMinMax(i8* %p) {
114114
; CHECK-NEXT: p[]: full-set
115115
; CHECK-NEXT: allocas uses:
116116
; GLOBAL-NEXT: safe accesses:
117+
; GLOBAL-NEXT: store i8 0, i8* %p1, align 1
118+
; GLOBAL-NEXT: store i8 0, i8* %p2, align 1
117119
; CHECK-EMPTY:
118120
entry:
119121
%p1 = getelementptr i8, i8* %p, i64 9223372036854775805
@@ -129,6 +131,8 @@ define dso_local void @WriteMax(i8* %p) {
129131
; CHECK-NEXT: p[]: [-9223372036854775807,9223372036854775806)
130132
; CHECK-NEXT: allocas uses:
131133
; GLOBAL-NEXT: safe accesses:
134+
; GLOBAL-NEXT: call void @llvm.memset.p0i8.i64(i8* %p, i8 1, i64 9223372036854775806, i1 false)
135+
; GLOBAL-NEXT: call void @llvm.memset.p0i8.i64(i8* %p2, i8 1, i64 9223372036854775806, i1 false)
132136
; CHECK-EMPTY:
133137
entry:
134138
call void @llvm.memset.p0i8.i64(i8* %p, i8 1, i64 9223372036854775806, i1 0)
@@ -476,6 +480,7 @@ define void @Scalable(<vscale x 4 x i32>* %p, <vscale x 4 x i32>* %unused, <vsca
476480
; CHECK-NEXT: allocas uses:
477481
; CHECK-NEXT: x[0]: [0,1){{$}}
478482
; GLOBAL-NEXT: safe accesses:
483+
; GLOBAL-NEXT: store <vscale x 4 x i32> %v, <vscale x 4 x i32>* %p, align 4
479484
; CHECK-EMPTY:
480485
entry:
481486
%x = alloca <vscale x 4 x i32>, align 4
@@ -495,6 +500,8 @@ define void @ZeroSize(%zerosize_type *%p) {
495500
; CHECK-NEXT: x[0]: empty-set
496501
; GLOBAL-NEXT: safe accesses:
497502
; GLOBAL-NEXT: store %zerosize_type undef, %zerosize_type* %x, align 4
503+
; GLOBAL-NEXT: store %zerosize_type undef, %zerosize_type* undef, align 4
504+
; GLOBAL-NEXT: load %zerosize_type, %zerosize_type* %p, align
498505
; CHECK-EMPTY:
499506
entry:
500507
%x = alloca %zerosize_type, align 4
@@ -572,6 +579,7 @@ define dso_local i8 @LoadMinInt64(i8* %p) {
572579
; CHECK-NEXT: p[]: [-9223372036854775808,-9223372036854775807){{$}}
573580
; CHECK-NEXT: allocas uses:
574581
; GLOBAL-NEXT: safe accesses:
582+
; GLOBAL-NEXT: load i8, i8* %p2, align 1
575583
; CHECK-EMPTY:
576584
%p2 = getelementptr i8, i8* %p, i64 -9223372036854775808
577585
%v = load i8, i8* %p2, align 1
@@ -600,6 +608,8 @@ define void @DeadBlock(i64* %p) {
600608
; CHECK-NEXT: allocas uses:
601609
; CHECK-NEXT: x[1]: empty-set{{$}}
602610
; GLOBAL-NEXT: safe accesses:
611+
; GLOBAL-NEXT: store i8 5, i8* %x
612+
; GLOBAL-NEXT: store i64 -5, i64* %p
603613
; CHECK-EMPTY:
604614
entry:
605615
%x = alloca i8, align 4
@@ -846,6 +856,58 @@ tlabel:
846856
ret i32* %a
847857
}
848858

859+
define void @MixedAccesses6(i8* %arg) {
860+
; CHECK-LABEL: @MixedAccesses6
861+
; CHECK-NEXT: args uses:
862+
; CHECK-NEXT: arg[]: [0,4)
863+
; CHECK-NEXT: allocas uses:
864+
; CHECK: a[4]: [0,4)
865+
; GLOBAL-NEXT: safe accesses:
866+
; GLOBAL-NEXT: call void @llvm.memcpy.p0i8.p0i8.i32(i8* %x, i8* %arg, i32 4, i1 false)
867+
; CHECK-EMPTY:
868+
entry:
869+
%a = alloca i32, align 4
870+
%x = bitcast i32* %a to i8*
871+
call void @llvm.memcpy.p0i8.p0i8.i32(i8* %x, i8* %arg, i32 4, i1 false)
872+
ret void
873+
}
874+
875+
define void @MixedAccesses7(i1 %cond, i8* %arg) {
876+
; SECV doesn't support select, so we consider this non-stack-safe, even through
877+
; it is.
878+
;
879+
; CHECK-LABEL: @MixedAccesses7
880+
; CHECK-NEXT: args uses:
881+
; CHECK-NEXT: arg[]: full-set
882+
; CHECK-NEXT: allocas uses:
883+
; CHECK: a[4]: full-set
884+
; GLOBAL-NEXT: safe accesses:
885+
; CHECK-EMPTY:
886+
entry:
887+
%a = alloca i32, align 4
888+
%x = bitcast i32* %a to i8*
889+
%x1 = select i1 %cond, i8* %arg, i8* %x
890+
call void @llvm.memcpy.p0i8.p0i8.i32(i8* %x1, i8* %arg, i32 4, i1 false)
891+
ret void
892+
}
893+
894+
define void @NoStackAccess(i8* %arg1, i8* %arg2) {
895+
; CHECK-LABEL: @NoStackAccess
896+
; CHECK-NEXT: args uses:
897+
; CHECK-NEXT: arg1[]: [0,4)
898+
; CHECK-NEXT: arg2[]: [0,4)
899+
; CHECK-NEXT: allocas uses:
900+
; CHECK: a[4]: empty-set{{$}}
901+
; GLOBAL-NEXT: safe accesses:
902+
; GLOBAL-NEXT: call void @llvm.memcpy.p0i8.p0i8.i32(i8* %arg1, i8* %arg2, i32 4, i1 false)
903+
; CHECK-EMPTY:
904+
entry:
905+
%a = alloca i32, align 4
906+
%x = bitcast i32* %a to i8*
907+
call void @llvm.memcpy.p0i8.p0i8.i32(i8* %arg1, i8* %arg2, i32 4, i1 false)
908+
ret void
909+
}
910+
849911
define void @DoubleLifetime() {
850912
; CHECK-LABEL: @DoubleLifetime
851913
; CHECK-NEXT: args uses:

‎llvm/test/Instrumentation/HWAddressSanitizer/mem-intrinsics.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
; RUN: opt -S -passes=hwasan %s | FileCheck %s
1+
; RUN: opt -S -passes=hwasan -hwasan-use-stack-safety=0 %s | FileCheck %s
22

33
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
44
target triple = "x86_64-unknown-linux-gnu"

0 commit comments

Comments
 (0)