Skip to content

Commit

Permalink
MSan: handle llvm.lifetime.start intrinsic
Browse files Browse the repository at this point in the history 
Summary:
When a variable goes into scope several times within a single function
or when two variables from different scopes share a stack slot it may
be incorrect to poison such scoped locals at the beginning of the
function.
In the former case it may lead to false negatives (see
google/sanitizers#590), in the latter - to
incorrect reports (because only one origin remains on the stack).

If Clang emits lifetime intrinsics for such scoped variables we insert
code poisoning them after each call to llvm.lifetime.start().
If for a certain intrinsic we fail to find a corresponding alloca, we
fall back to poisoning allocas for the whole function, as it's now
impossible to tell which alloca was missed.

The new instrumentation may slow down hot loops containing local
variables with lifetime intrinsics, so we allow disabling it with
-mllvm -msan-handle-lifetime-intrinsics=false.

Reviewers: eugenis, pcc

Subscribers: hiraditya, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D60617

llvm-svn: 359536
  • Loading branch information
@ramosian-glider
ramosian-glider committed Apr 30, 2019
1 parent a475da3 commit 06d00af
Show file tree
Hide file tree
Showing 2 changed files with 183 additions and 8 deletions.
62 changes: 54 additions & 8 deletions llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
View file
Expand Up @@ -143,6 +143,7 @@
#include "llvm/ADT/APInt.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DepthFirstIterator.h"
#include "llvm/ADT/SmallSet.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringExtras.h"
Expand Down Expand Up @@ -247,6 +248,13 @@ static cl::opt<bool> ClHandleICmpExact("msan-handle-icmp-exact",
cl::desc("exact handling of relational integer ICmp"),
cl::Hidden, cl::init(false));

static cl::opt<bool> ClHandleLifetimeIntrinsics(
"msan-handle-lifetime-intrinsics",
cl::desc(
"when possible, poison scoped variables at the beginning of the scope "
"(slower, but more precise)"),
cl::Hidden, cl::init(true));

// When compiling the Linux kernel, we sometimes see false positives related to
// MSan being unable to understand that inline assembly calls may initialize
// local variables.
Expand Down Expand Up @@ -1023,6 +1031,9 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
: Shadow(S), Origin(O), OrigIns(I) {}
};
SmallVector<ShadowOriginAndInsertPoint, 16> InstrumentationList;
bool InstrumentLifetimeStart = ClHandleLifetimeIntrinsics;
SmallSet<AllocaInst *, 16> AllocaSet;
SmallVector<std::pair<IntrinsicInst *, AllocaInst *>, 16> LifetimeStartList;
SmallVector<StoreInst *, 16> StoreList;

MemorySanitizerVisitor(Function &F, MemorySanitizer &MS,
Expand Down Expand Up @@ -1279,6 +1290,19 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {

VAHelper->finalizeInstrumentation();

// Poison llvm.lifetime.start intrinsics, if we haven't fallen back to
// instrumenting only allocas.
if (InstrumentLifetimeStart) {
for (auto Item : LifetimeStartList) {
instrumentAlloca(*Item.second, Item.first);
AllocaSet.erase(Item.second);
}
}
// Poison the allocas for which we didn't instrument the corresponding
// lifetime intrinsics.
for (AllocaInst *AI : AllocaSet)
instrumentAlloca(*AI);

bool InstrumentWithCalls = ClInstrumentationWithCallThreshold >= 0 &&
InstrumentationList.size() + StoreList.size() >
(unsigned)ClInstrumentationWithCallThreshold;
Expand Down Expand Up @@ -2536,6 +2560,17 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
return false;
}

void handleLifetimeStart(IntrinsicInst &I) {
if (!PoisonStack)
return;
DenseMap<Value *, AllocaInst *> AllocaForValue;
AllocaInst *AI =
llvm::findAllocaForValue(I.getArgOperand(1), AllocaForValue);
if (!AI)
InstrumentLifetimeStart = false;
LifetimeStartList.push_back(std::make_pair(&I, AI));
}

void handleBswap(IntrinsicInst &I) {
IRBuilder<> IRB(&I);
Value *Op = I.getArgOperand(0);
Expand Down Expand Up @@ -2951,6 +2986,9 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {

void visitIntrinsicInst(IntrinsicInst &I) {
switch (I.getIntrinsicID()) {
case Intrinsic::lifetime_start:
handleLifetimeStart(I);
break;
case Intrinsic::bswap:
handleBswap(I);
break;
Expand Down Expand Up @@ -3379,7 +3417,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
StackDescription.str());
}

void instrumentAllocaUserspace(AllocaInst &I, IRBuilder<> &IRB, Value *Len) {
void poisonAllocaUserspace(AllocaInst &I, IRBuilder<> &IRB, Value *Len) {
if (PoisonStack && ClPoisonStackWithCall) {
IRB.CreateCall(MS.MsanPoisonStackFn,
{IRB.CreatePointerCast(&I, IRB.getInt8PtrTy()), Len});
Expand All @@ -3401,7 +3439,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
}
}

void instrumentAllocaKmsan(AllocaInst &I, IRBuilder<> &IRB, Value *Len) {
void poisonAllocaKmsan(AllocaInst &I, IRBuilder<> &IRB, Value *Len) {
Value *Descr = getLocalVarDescription(I);
if (PoisonStack) {
IRB.CreateCall(MS.MsanPoisonAllocaFn,
Expand All @@ -3413,20 +3451,28 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
}
}

void visitAllocaInst(AllocaInst &I) {
setShadow(&I, getCleanShadow(&I));
setOrigin(&I, getCleanOrigin());
IRBuilder<> IRB(I.getNextNode());
void instrumentAlloca(AllocaInst &I, Instruction *InsPoint = nullptr) {
if (!InsPoint)
InsPoint = &I;
IRBuilder<> IRB(InsPoint->getNextNode());
const DataLayout &DL = F.getParent()->getDataLayout();
uint64_t TypeSize = DL.getTypeAllocSize(I.getAllocatedType());
Value *Len = ConstantInt::get(MS.IntptrTy, TypeSize);
if (I.isArrayAllocation())
Len = IRB.CreateMul(Len, I.getArraySize());

if (MS.CompileKernel)
instrumentAllocaKmsan(I, IRB, Len);
poisonAllocaKmsan(I, IRB, Len);
else
instrumentAllocaUserspace(I, IRB, Len);
poisonAllocaUserspace(I, IRB, Len);
}

void visitAllocaInst(AllocaInst &I) {
setShadow(&I, getCleanShadow(&I));
setOrigin(&I, getCleanOrigin());
// We'll get to this alloca later unless it's poisoned at the corresponding
// llvm.lifetime.start.
AllocaSet.insert(&I);
}

void visitSelectInst(SelectInst& I) {
Expand Down
129 changes: 129 additions & 0 deletions llvm/test/Instrumentation/MemorySanitizer/alloca.ll
View file
Expand Up @@ -89,3 +89,132 @@ entry:
; KMSAN: call void @__msan_unpoison_alloca(i8* {{.*}}, i64 20)
; CHECK: ret void

; Check that every llvm.lifetime.start() causes poisoning of locals.
define void @lifetime_start() sanitize_memory {
entry:
%x = alloca i32, align 4
%c = bitcast i32* %x to i8*
br label %another_bb

another_bb:
call void @llvm.lifetime.start.p0i8(i64 4, i8* nonnull %c)
store i32 7, i32* %x
call void @llvm.lifetime.end.p0i8(i64 4, i8* nonnull %c)
call void @llvm.lifetime.start.p0i8(i64 4, i8* nonnull %c)
store i32 8, i32* %x
call void @llvm.lifetime.end.p0i8(i64 4, i8* nonnull %c)
ret void
}

; CHECK-LABEL: define void @lifetime_start(
; CHECK-LABEL: entry:
; CHECK: %x = alloca i32
; CHECK-LABEL: another_bb:

; CHECK: call void @llvm.lifetime.start
; INLINE: call void @llvm.memset.p0i8.i64(i8* align 4 {{.*}}, i8 -1, i64 4, i1 false)
; CALL: call void @__msan_poison_stack(i8* {{.*}}, i64 4)
; ORIGIN: call void @__msan_set_alloca_origin4(i8* {{.*}}, i64 4,
; KMSAN: call void @__msan_poison_alloca(i8* {{.*}}, i64 4,

; CHECK: call void @llvm.lifetime.start
; INLINE: call void @llvm.memset.p0i8.i64(i8* align 4 {{.*}}, i8 -1, i64 4, i1 false)
; CALL: call void @__msan_poison_stack(i8* {{.*}}, i64 4)
; ORIGIN: call void @__msan_set_alloca_origin4(i8* {{.*}}, i64 4,
; KMSAN: call void @__msan_poison_alloca(i8* {{.*}}, i64 4,
; CHECK: ret void

; Make sure variable-length arrays are handled correctly.
define void @lifetime_start_var(i64 %cnt) sanitize_memory {
entry:
%x = alloca i32, i64 %cnt, align 4
%c = bitcast i32* %x to i8*
call void @llvm.lifetime.start.p0i8(i64 -1, i8* nonnull %c)
call void @llvm.lifetime.end.p0i8(i64 -1, i8* nonnull %c)
ret void
}

; CHECK-LABEL: define void @lifetime_start_var(
; CHECK-LABEL: entry:
; CHECK: %x = alloca i32, i64 %cnt
; CHECK: call void @llvm.lifetime.start
; CHECK: %[[A:.*]] = mul i64 4, %cnt
; INLINE: call void @llvm.memset.p0i8.i64(i8* align 4 {{.*}}, i8 -1, i64 %[[A]], i1 false)
; CALL: call void @__msan_poison_stack(i8* {{.*}}, i64 %[[A]])
; ORIGIN: call void @__msan_set_alloca_origin4(i8* {{.*}}, i64 %[[A]],
; KMSAN: call void @__msan_poison_alloca(i8* {{.*}}, i64 %[[A]],
; CHECK: call void @llvm.lifetime.end
; CHECK: ret void


; If we can't trace one of the lifetime markers to a single alloca, fall back
; to poisoning allocas at the beginning of the function.
; Each alloca must be poisoned only once.
define void @lifetime_no_alloca(i8 %v) sanitize_memory {
entry:
%x = alloca i32, align 4
%y = alloca i32, align 4
%z = alloca i32, align 4
%cx = bitcast i32* %x to i8*
%cy = bitcast i32* %y to i8*
%cz = bitcast i32* %z to i8*
%tobool = icmp eq i8 %v, 0
%xy = select i1 %tobool, i32* %x, i32* %y
%cxcy = select i1 %tobool, i8* %cx, i8* %cy
br label %another_bb

another_bb:
call void @llvm.lifetime.start.p0i8(i64 4, i8* nonnull %cz)
store i32 7, i32* %z
call void @llvm.lifetime.end.p0i8(i64 4, i8* nonnull %cz)
call void @llvm.lifetime.start.p0i8(i64 4, i8* nonnull %cz)
store i32 7, i32* %z
call void @llvm.lifetime.end.p0i8(i64 4, i8* nonnull %cz)
call void @llvm.lifetime.start.p0i8(i64 4, i8* nonnull %cxcy)
store i32 8, i32* %xy
call void @llvm.lifetime.end.p0i8(i64 4, i8* nonnull %cxcy)
ret void
}

; CHECK-LABEL: define void @lifetime_no_alloca(
; CHECK-LABEL: entry:
; CHECK: %x = alloca i32
; INLINE: call void @llvm.memset.p0i8.i64(i8* align 4 {{.*}}, i8 -1, i64 4, i1 false)
; CALL: call void @__msan_poison_stack(i8* {{.*}}, i64 4)
; ORIGIN: call void @__msan_set_alloca_origin4(i8* {{.*}}, i64 4,
; KMSAN: call void @__msan_poison_alloca(i8* {{.*}}, i64 4,
; CHECK: %y = alloca i32
; INLINE: call void @llvm.memset.p0i8.i64(i8* align 4 {{.*}}, i8 -1, i64 4, i1 false)
; CALL: call void @__msan_poison_stack(i8* {{.*}}, i64 4)
; ORIGIN: call void @__msan_set_alloca_origin4(i8* {{.*}}, i64 4,
; KMSAN: call void @__msan_poison_alloca(i8* {{.*}}, i64 4,
; CHECK: %z = alloca i32
; INLINE: call void @llvm.memset.p0i8.i64(i8* align 4 {{.*}}, i8 -1, i64 4, i1 false)
; CALL: call void @__msan_poison_stack(i8* {{.*}}, i64 4)
; ORIGIN: call void @__msan_set_alloca_origin4(i8* {{.*}}, i64 4,
; KMSAN: call void @__msan_poison_alloca(i8* {{.*}}, i64 4,

; There're two lifetime intrinsics for %z, but we must instrument it only once.
; INLINE-NOT: call void @llvm.memset.p0i8.i64(i8* align 4 {{.*}}, i8 -1, i64 4, i1 false)
; CALL-NOT: call void @__msan_poison_stack(i8* {{.*}}, i64 4)
; ORIGIN-NOT: call void @__msan_set_alloca_origin4(i8* {{.*}}, i64 4,
; KMSAN-NOT: call void @__msan_poison_alloca(i8* {{.*}}, i64 4,
; CHECK-LABEL: another_bb:

; CHECK: call void @llvm.lifetime.start
; INLINE-NOT: call void @llvm.memset.p0i8.i64(i8* align 4 {{.*}}, i8 -1, i64 4, i1 false)
; CALL-NOT: call void @__msan_poison_stack(i8* {{.*}}, i64 4)
; ORIGIN-NOT: call void @__msan_set_alloca_origin4(i8* {{.*}}, i64 4,
; KMSAN-NOT: call void @__msan_poison_alloca(i8* {{.*}}, i64 4,
; CHECK: call void @llvm.lifetime.end
; CHECK: call void @llvm.lifetime.start
; INLINE-NOT: call void @llvm.memset.p0i8.i64(i8* align 4 {{.*}}, i8 -1, i64 4, i1 false)
; CALL-NOT: call void @__msan_poison_stack(i8* {{.*}}, i64 4)
; ORIGIN-NOT: call void @__msan_set_alloca_origin4(i8* {{.*}}, i64 4,
; KMSAN-NOT: call void @__msan_poison_alloca(i8* {{.*}}, i64 4,
; CHECK: call void @llvm.lifetime.end



declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture)
declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)

0 comments on commit 06d00af

Please sign in to comment.