Skip to content

Commit 1c44d49

Browse files
nikicakadutta
authored andcommitted
[AddressSanitizer] Avoid unnecessary ptr<->int casts for stack poisoning (llvm#162634)
Instead of casting pointers to integers to perform arithmetic on them, use ptradd. We still need some casts when interfacing with the asan runtime.
1 parent 37fa304 commit 1c44d49

File tree

7 files changed

+201
-246
lines changed

7 files changed

+201
-246
lines changed

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

Lines changed: 30 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -3337,7 +3337,7 @@ PHINode *FunctionStackPoisoner::createPHI(IRBuilder<> &IRB, Value *Cond,
33373337
Value *ValueIfTrue,
33383338
Instruction *ThenTerm,
33393339
Value *ValueIfFalse) {
3340-
PHINode *PHI = IRB.CreatePHI(IntptrTy, 2);
3340+
PHINode *PHI = IRB.CreatePHI(ValueIfTrue->getType(), 2);
33413341
BasicBlock *CondBlock = cast<Instruction>(Cond)->getParent();
33423342
PHI->addIncoming(ValueIfFalse, CondBlock);
33433343
BasicBlock *ThenBlock = ThenTerm->getParent();
@@ -3360,7 +3360,7 @@ Value *FunctionStackPoisoner::createAllocaForLayout(
33603360
assert((ClRealignStack & (ClRealignStack - 1)) == 0);
33613361
uint64_t FrameAlignment = std::max(L.FrameAlignment, uint64_t(ClRealignStack));
33623362
Alloca->setAlignment(Align(FrameAlignment));
3363-
return IRB.CreatePointerCast(Alloca, IntptrTy);
3363+
return Alloca;
33643364
}
33653365

33663366
void FunctionStackPoisoner::createDynamicAllocasInitStorage() {
@@ -3572,10 +3572,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
35723572
DoDynamicAlloca &= !HasInlineAsm && !HasReturnsTwiceCall;
35733573
DoStackMalloc &= !HasInlineAsm && !HasReturnsTwiceCall;
35743574

3575+
Type *PtrTy = F.getDataLayout().getAllocaPtrType(F.getContext());
35753576
Value *StaticAlloca =
35763577
DoDynamicAlloca ? nullptr : createAllocaForLayout(IRB, L, false);
35773578

3578-
Value *FakeStack;
3579+
Value *FakeStackPtr;
3580+
Value *FakeStackInt;
35793581
Value *LocalStackBase;
35803582
Value *LocalStackBaseAlloca;
35813583
uint8_t DIExprFlags = DIExpression::ApplyOffset;
@@ -3603,88 +3605,75 @@ void FunctionStackPoisoner::processStaticAllocas() {
36033605
RTCI.createRuntimeCall(IRBIf, AsanStackMallocFunc[StackMallocIdx],
36043606
ConstantInt::get(IntptrTy, LocalStackSize));
36053607
IRB.SetInsertPoint(InsBefore);
3606-
FakeStack = createPHI(IRB, UseAfterReturnIsEnabled, FakeStackValue, Term,
3607-
ConstantInt::get(IntptrTy, 0));
3608+
FakeStackInt = createPHI(IRB, UseAfterReturnIsEnabled, FakeStackValue,
3609+
Term, ConstantInt::get(IntptrTy, 0));
36083610
} else {
36093611
// assert(ASan.UseAfterReturn == AsanDetectStackUseAfterReturnMode:Always)
36103612
// void *FakeStack = __asan_stack_malloc_N(LocalStackSize);
36113613
// void *LocalStackBase = (FakeStack) ? FakeStack :
36123614
// alloca(LocalStackSize);
36133615
StackMallocIdx = StackMallocSizeClass(LocalStackSize);
3614-
FakeStack =
3616+
FakeStackInt =
36153617
RTCI.createRuntimeCall(IRB, AsanStackMallocFunc[StackMallocIdx],
36163618
ConstantInt::get(IntptrTy, LocalStackSize));
36173619
}
3620+
FakeStackPtr = IRB.CreateIntToPtr(FakeStackInt, PtrTy);
36183621
Value *NoFakeStack =
3619-
IRB.CreateICmpEQ(FakeStack, Constant::getNullValue(IntptrTy));
3622+
IRB.CreateICmpEQ(FakeStackInt, Constant::getNullValue(IntptrTy));
36203623
Instruction *Term =
36213624
SplitBlockAndInsertIfThen(NoFakeStack, InsBefore, false);
36223625
IRBuilder<> IRBIf(Term);
36233626
Value *AllocaValue =
36243627
DoDynamicAlloca ? createAllocaForLayout(IRBIf, L, true) : StaticAlloca;
36253628

36263629
IRB.SetInsertPoint(InsBefore);
3627-
LocalStackBase = createPHI(IRB, NoFakeStack, AllocaValue, Term, FakeStack);
3630+
LocalStackBase =
3631+
createPHI(IRB, NoFakeStack, AllocaValue, Term, FakeStackPtr);
36283632
IRB.CreateStore(LocalStackBase, LocalStackBaseAlloca);
36293633
DIExprFlags |= DIExpression::DerefBefore;
36303634
} else {
36313635
// void *FakeStack = nullptr;
36323636
// void *LocalStackBase = alloca(LocalStackSize);
3633-
FakeStack = ConstantInt::get(IntptrTy, 0);
3637+
FakeStackInt = Constant::getNullValue(IntptrTy);
3638+
FakeStackPtr = Constant::getNullValue(PtrTy);
36343639
LocalStackBase =
36353640
DoDynamicAlloca ? createAllocaForLayout(IRB, L, true) : StaticAlloca;
36363641
LocalStackBaseAlloca = LocalStackBase;
36373642
}
36383643

3639-
// It shouldn't matter whether we pass an `alloca` or a `ptrtoint` as the
3640-
// dbg.declare address opereand, but passing a `ptrtoint` seems to confuse
3641-
// later passes and can result in dropped variable coverage in debug info.
3642-
Value *LocalStackBaseAllocaPtr =
3643-
isa<PtrToIntInst>(LocalStackBaseAlloca)
3644-
? cast<PtrToIntInst>(LocalStackBaseAlloca)->getPointerOperand()
3645-
: LocalStackBaseAlloca;
3646-
assert(isa<AllocaInst>(LocalStackBaseAllocaPtr) &&
3647-
"Variable descriptions relative to ASan stack base will be dropped");
3648-
36493644
// Replace Alloca instructions with base+offset.
36503645
SmallVector<Value *> NewAllocaPtrs;
36513646
for (const auto &Desc : SVD) {
36523647
AllocaInst *AI = Desc.AI;
3653-
replaceDbgDeclare(AI, LocalStackBaseAllocaPtr, DIB, DIExprFlags,
3654-
Desc.Offset);
3655-
Value *NewAllocaPtr = IRB.CreateIntToPtr(
3656-
IRB.CreateAdd(LocalStackBase, ConstantInt::get(IntptrTy, Desc.Offset)),
3657-
AI->getType());
3648+
replaceDbgDeclare(AI, LocalStackBaseAlloca, DIB, DIExprFlags, Desc.Offset);
3649+
Value *NewAllocaPtr = IRB.CreatePtrAdd(
3650+
LocalStackBase, ConstantInt::get(IntptrTy, Desc.Offset));
36583651
AI->replaceAllUsesWith(NewAllocaPtr);
36593652
NewAllocaPtrs.push_back(NewAllocaPtr);
36603653
}
36613654

36623655
// The left-most redzone has enough space for at least 4 pointers.
36633656
// Write the Magic value to redzone[0].
3664-
Value *BasePlus0 = IRB.CreateIntToPtr(LocalStackBase, IntptrPtrTy);
36653657
IRB.CreateStore(ConstantInt::get(IntptrTy, kCurrentStackFrameMagic),
3666-
BasePlus0);
3658+
LocalStackBase);
36673659
// Write the frame description constant to redzone[1].
3668-
Value *BasePlus1 = IRB.CreateIntToPtr(
3669-
IRB.CreateAdd(LocalStackBase,
3670-
ConstantInt::get(IntptrTy, ASan.LongSize / 8)),
3671-
IntptrPtrTy);
3660+
Value *BasePlus1 = IRB.CreatePtrAdd(
3661+
LocalStackBase, ConstantInt::get(IntptrTy, ASan.LongSize / 8));
36723662
GlobalVariable *StackDescriptionGlobal =
36733663
createPrivateGlobalForString(*F.getParent(), DescriptionString,
36743664
/*AllowMerging*/ true, genName("stack"));
36753665
Value *Description = IRB.CreatePointerCast(StackDescriptionGlobal, IntptrTy);
36763666
IRB.CreateStore(Description, BasePlus1);
36773667
// Write the PC to redzone[2].
3678-
Value *BasePlus2 = IRB.CreateIntToPtr(
3679-
IRB.CreateAdd(LocalStackBase,
3680-
ConstantInt::get(IntptrTy, 2 * ASan.LongSize / 8)),
3681-
IntptrPtrTy);
3668+
Value *BasePlus2 = IRB.CreatePtrAdd(
3669+
LocalStackBase, ConstantInt::get(IntptrTy, 2 * ASan.LongSize / 8));
36823670
IRB.CreateStore(IRB.CreatePointerCast(&F, IntptrTy), BasePlus2);
36833671

36843672
const auto &ShadowAfterScope = GetShadowBytesAfterScope(SVD, L);
36853673

36863674
// Poison the stack red zones at the entry.
3687-
Value *ShadowBase = ASan.memToShadow(LocalStackBase, IRB);
3675+
Value *ShadowBase =
3676+
ASan.memToShadow(IRB.CreatePtrToInt(LocalStackBase, IntptrTy), IRB);
36883677
// As mask we must use most poisoned case: red zones and after scope.
36893678
// As bytes we can use either the same or just red zones only.
36903679
copyToShadow(ShadowAfterScope, ShadowAfterScope, IRB, ShadowBase);
@@ -3723,7 +3712,7 @@ void FunctionStackPoisoner::processStaticAllocas() {
37233712
IRBuilder<> IRBRet(Ret);
37243713
// Mark the current frame as retired.
37253714
IRBRet.CreateStore(ConstantInt::get(IntptrTy, kRetiredStackFrameMagic),
3726-
BasePlus0);
3715+
LocalStackBase);
37273716
if (DoStackMalloc) {
37283717
assert(StackMallocIdx >= 0);
37293718
// if FakeStack != 0 // LocalStackBase == FakeStack
@@ -3737,7 +3726,7 @@ void FunctionStackPoisoner::processStaticAllocas() {
37373726
// else
37383727
// <This is not a fake stack; unpoison the redzones>
37393728
Value *Cmp =
3740-
IRBRet.CreateICmpNE(FakeStack, Constant::getNullValue(IntptrTy));
3729+
IRBRet.CreateICmpNE(FakeStackInt, Constant::getNullValue(IntptrTy));
37413730
Instruction *ThenTerm, *ElseTerm;
37423731
SplitBlockAndInsertIfThenElse(Cmp, Ret, &ThenTerm, &ElseTerm);
37433732

@@ -3748,19 +3737,18 @@ void FunctionStackPoisoner::processStaticAllocas() {
37483737
kAsanStackUseAfterReturnMagic);
37493738
copyToShadow(ShadowAfterReturn, ShadowAfterReturn, IRBPoison,
37503739
ShadowBase);
3751-
Value *SavedFlagPtrPtr = IRBPoison.CreateAdd(
3752-
FakeStack,
3740+
Value *SavedFlagPtrPtr = IRBPoison.CreatePtrAdd(
3741+
FakeStackPtr,
37533742
ConstantInt::get(IntptrTy, ClassSize - ASan.LongSize / 8));
3754-
Value *SavedFlagPtr = IRBPoison.CreateLoad(
3755-
IntptrTy, IRBPoison.CreateIntToPtr(SavedFlagPtrPtr, IntptrPtrTy));
3743+
Value *SavedFlagPtr = IRBPoison.CreateLoad(IntptrTy, SavedFlagPtrPtr);
37563744
IRBPoison.CreateStore(
37573745
Constant::getNullValue(IRBPoison.getInt8Ty()),
37583746
IRBPoison.CreateIntToPtr(SavedFlagPtr, IRBPoison.getPtrTy()));
37593747
} else {
37603748
// For larger frames call __asan_stack_free_*.
37613749
RTCI.createRuntimeCall(
37623750
IRBPoison, AsanStackFreeFunc[StackMallocIdx],
3763-
{FakeStack, ConstantInt::get(IntptrTy, LocalStackSize)});
3751+
{FakeStackInt, ConstantInt::get(IntptrTy, LocalStackSize)});
37643752
}
37653753

37663754
IRBuilder<> IRBElse(ElseTerm);

0 commit comments

Comments
 (0)