-
Notifications
You must be signed in to change notification settings - Fork 14.5k
[Sanitizers] Remove handling for lifetimes on non-alloca insts (NFC) #149994
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
After llvm#149310 the pointer argument of lifetime.start/lifetime.end is guaranteed to be an alloca, so we don't need to go through findAllocaForValue() anymore, and don't have to have special handling for the case where it fails.
@llvm/pr-subscribers-llvm-analysis @llvm/pr-subscribers-backend-aarch64 Author: Nikita Popov (nikic) ChangesAfter #149310 the pointer argument of lifetime.start/lifetime.end is guaranteed to be an alloca, so we don't need to go through findAllocaForValue() anymore, and don't have to have special handling for the case where it fails. Full diff: https://github.com/llvm/llvm-project/pull/149994.diff 8 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Utils/MemoryTaggingSupport.h b/llvm/include/llvm/Transforms/Utils/MemoryTaggingSupport.h
index f288bdfb84f49..e0cdcf84012e8 100644
--- a/llvm/include/llvm/Transforms/Utils/MemoryTaggingSupport.h
+++ b/llvm/include/llvm/Transforms/Utils/MemoryTaggingSupport.h
@@ -57,7 +57,6 @@ struct AllocaInfo {
struct StackInfo {
MapVector<AllocaInst *, AllocaInfo> AllocasToInstrument;
- SmallVector<Instruction *, 4> UnrecognizedLifetimes;
SmallVector<Instruction *, 8> RetVec;
bool CallsReturnTwice = false;
};
diff --git a/llvm/lib/Analysis/StackLifetime.cpp b/llvm/lib/Analysis/StackLifetime.cpp
index 21f54c7cbc849..34a7a0416d290 100644
--- a/llvm/lib/Analysis/StackLifetime.cpp
+++ b/llvm/lib/Analysis/StackLifetime.cpp
@@ -63,10 +63,7 @@ bool StackLifetime::isAliveAfter(const AllocaInst *AI,
// markers has the same size and points to the alloca start.
static const AllocaInst *findMatchingAlloca(const IntrinsicInst &II,
const DataLayout &DL) {
- const AllocaInst *AI = findAllocaForValue(II.getArgOperand(1), true);
- if (!AI)
- return nullptr;
-
+ const AllocaInst *AI = cast<AllocaInst>(II.getArgOperand(1));
auto AllocaSize = AI->getAllocationSize(DL);
if (!AllocaSize)
return nullptr;
diff --git a/llvm/lib/Target/AArch64/AArch64StackTagging.cpp b/llvm/lib/Target/AArch64/AArch64StackTagging.cpp
index 75c7dd944b467..f136a18421524 100644
--- a/llvm/lib/Target/AArch64/AArch64StackTagging.cpp
+++ b/llvm/lib/Target/AArch64/AArch64StackTagging.cpp
@@ -581,7 +581,6 @@ bool AArch64StackTagging::runOnFunction(Function &Fn) {
// statement if return_twice functions are called.
bool StandardLifetime =
!SInfo.CallsReturnTwice &&
- SInfo.UnrecognizedLifetimes.empty() &&
memtag::isStandardLifetime(Info.LifetimeStart, Info.LifetimeEnd, DT, LI,
ClMaxLifetimes);
if (StandardLifetime) {
@@ -616,10 +615,5 @@ bool AArch64StackTagging::runOnFunction(Function &Fn) {
memtag::annotateDebugRecords(Info, Tag);
}
- // If we have instrumented at least one alloca, all unrecognized lifetime
- // intrinsics have to go.
- for (auto *I : SInfo.UnrecognizedLifetimes)
- I->eraseFromParent();
-
return true;
}
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index fbaa651641566..e87bee79a6a69 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -1063,7 +1063,6 @@ struct FunctionStackPoisoner : public InstVisitor<FunctionStackPoisoner> {
};
SmallVector<AllocaPoisonCall, 8> DynamicAllocaPoisonCallVec;
SmallVector<AllocaPoisonCall, 8> StaticAllocaPoisonCallVec;
- bool HasUntracedLifetimeIntrinsic = false;
SmallVector<AllocaInst *, 1> DynamicAllocaVec;
SmallVector<IntrinsicInst *, 1> StackRestoreVec;
@@ -1097,14 +1096,6 @@ struct FunctionStackPoisoner : public InstVisitor<FunctionStackPoisoner> {
initializeCallbacks(*F.getParent());
- if (HasUntracedLifetimeIntrinsic) {
- // If there are lifetime intrinsics which couldn't be traced back to an
- // alloca, we may not know exactly when a variable enters scope, and
- // therefore should "fail safe" by not poisoning them.
- StaticAllocaPoisonCallVec.clear();
- DynamicAllocaPoisonCallVec.clear();
- }
-
processDynamicAllocas();
processStaticAllocas();
@@ -1231,13 +1222,7 @@ struct FunctionStackPoisoner : public InstVisitor<FunctionStackPoisoner> {
!ConstantInt::isValueValidForType(IntptrTy, SizeValue))
return;
// Find alloca instruction that corresponds to llvm.lifetime argument.
- // Currently we can only handle lifetime markers pointing to the
- // beginning of the alloca.
- AllocaInst *AI = findAllocaForValue(II.getArgOperand(1), true);
- if (!AI) {
- HasUntracedLifetimeIntrinsic = true;
- return;
- }
+ AllocaInst *AI = cast<AllocaInst>(II.getArgOperand(1));
// We're interested only in allocas we can handle.
if (!ASan.isInterestingAlloca(*AI))
return;
diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index 2c34bf2157cdd..5849c3e475a6a 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -1500,7 +1500,6 @@ bool HWAddressSanitizer::instrumentStack(memtag::StackInfo &SInfo,
// statement if return_twice functions are called.
bool StandardLifetime =
!SInfo.CallsReturnTwice &&
- SInfo.UnrecognizedLifetimes.empty() &&
memtag::isStandardLifetime(Info.LifetimeStart, Info.LifetimeEnd, &DT,
&LI, ClMaxLifetimes);
if (DetectUseAfterScope && StandardLifetime) {
@@ -1525,8 +1524,6 @@ bool HWAddressSanitizer::instrumentStack(memtag::StackInfo &SInfo,
}
memtag::alignAndPadAlloca(Info, Mapping.getObjectAlignment());
}
- for (auto &I : SInfo.UnrecognizedLifetimes)
- I->eraseFromParent();
return true;
}
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 7b5831652aa6e..599b60dfba988 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -1216,7 +1216,6 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
};
SmallVector<ShadowOriginAndInsertPoint, 16> InstrumentationList;
DenseMap<const DILocation *, int> LazyWarningDebugLocationCount;
- bool InstrumentLifetimeStart = ClHandleLifetimeIntrinsics;
SmallSetVector<AllocaInst *, 16> AllocaSet;
SmallVector<std::pair<IntrinsicInst *, AllocaInst *>, 16> LifetimeStartList;
SmallVector<StoreInst *, 16> StoreList;
@@ -1623,7 +1622,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
// Poison llvm.lifetime.start intrinsics, if we haven't fallen back to
// instrumenting only allocas.
- if (InstrumentLifetimeStart) {
+ if (ClHandleLifetimeIntrinsics) {
for (auto Item : LifetimeStartList) {
instrumentAlloca(*Item.second, Item.first);
AllocaSet.remove(Item.second);
@@ -3303,9 +3302,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
void handleLifetimeStart(IntrinsicInst &I) {
if (!PoisonStack)
return;
- AllocaInst *AI = llvm::findAllocaForValue(I.getArgOperand(1));
- if (!AI)
- InstrumentLifetimeStart = false;
+ AllocaInst *AI = cast<AllocaInst>(I.getArgOperand(1));
LifetimeStartList.push_back(std::make_pair(&I, AI));
}
diff --git a/llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp b/llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp
index 7828571123bca..1d83ddc6b5a96 100644
--- a/llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp
@@ -343,8 +343,7 @@ static bool markTails(Function &F, OptimizationRemarkEmitter *ORE) {
///
static bool canMoveAboveCall(Instruction *I, CallInst *CI, AliasAnalysis *AA) {
if (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(I))
- if (II->getIntrinsicID() == Intrinsic::lifetime_end &&
- llvm::findAllocaForValue(II->getArgOperand(1)))
+ if (II->getIntrinsicID() == Intrinsic::lifetime_end)
return true;
// FIXME: We can move load/store/call/free instructions above the call if the
diff --git a/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp b/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
index 40dc02c546dfa..bea76d39bb216 100644
--- a/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
+++ b/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
@@ -155,11 +155,7 @@ void StackInfoBuilder::visit(OptimizationRemarkEmitter &ORE,
return;
}
if (auto *II = dyn_cast<LifetimeIntrinsic>(&Inst)) {
- AllocaInst *AI = findAllocaForValue(II->getArgOperand(1));
- if (!AI) {
- Info.UnrecognizedLifetimes.push_back(&Inst);
- return;
- }
+ AllocaInst *AI = cast<AllocaInst>(II->getArgOperand(1));
if (getAllocaInterestingness(*AI) != AllocaInterestingness::kInteresting)
return;
if (II->getIntrinsicID() == Intrinsic::lifetime_start)
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Nikita Popov (nikic) ChangesAfter #149310 the pointer argument of lifetime.start/lifetime.end is guaranteed to be an alloca, so we don't need to go through findAllocaForValue() anymore, and don't have to have special handling for the case where it fails. Full diff: https://github.com/llvm/llvm-project/pull/149994.diff 8 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Utils/MemoryTaggingSupport.h b/llvm/include/llvm/Transforms/Utils/MemoryTaggingSupport.h
index f288bdfb84f49..e0cdcf84012e8 100644
--- a/llvm/include/llvm/Transforms/Utils/MemoryTaggingSupport.h
+++ b/llvm/include/llvm/Transforms/Utils/MemoryTaggingSupport.h
@@ -57,7 +57,6 @@ struct AllocaInfo {
struct StackInfo {
MapVector<AllocaInst *, AllocaInfo> AllocasToInstrument;
- SmallVector<Instruction *, 4> UnrecognizedLifetimes;
SmallVector<Instruction *, 8> RetVec;
bool CallsReturnTwice = false;
};
diff --git a/llvm/lib/Analysis/StackLifetime.cpp b/llvm/lib/Analysis/StackLifetime.cpp
index 21f54c7cbc849..34a7a0416d290 100644
--- a/llvm/lib/Analysis/StackLifetime.cpp
+++ b/llvm/lib/Analysis/StackLifetime.cpp
@@ -63,10 +63,7 @@ bool StackLifetime::isAliveAfter(const AllocaInst *AI,
// markers has the same size and points to the alloca start.
static const AllocaInst *findMatchingAlloca(const IntrinsicInst &II,
const DataLayout &DL) {
- const AllocaInst *AI = findAllocaForValue(II.getArgOperand(1), true);
- if (!AI)
- return nullptr;
-
+ const AllocaInst *AI = cast<AllocaInst>(II.getArgOperand(1));
auto AllocaSize = AI->getAllocationSize(DL);
if (!AllocaSize)
return nullptr;
diff --git a/llvm/lib/Target/AArch64/AArch64StackTagging.cpp b/llvm/lib/Target/AArch64/AArch64StackTagging.cpp
index 75c7dd944b467..f136a18421524 100644
--- a/llvm/lib/Target/AArch64/AArch64StackTagging.cpp
+++ b/llvm/lib/Target/AArch64/AArch64StackTagging.cpp
@@ -581,7 +581,6 @@ bool AArch64StackTagging::runOnFunction(Function &Fn) {
// statement if return_twice functions are called.
bool StandardLifetime =
!SInfo.CallsReturnTwice &&
- SInfo.UnrecognizedLifetimes.empty() &&
memtag::isStandardLifetime(Info.LifetimeStart, Info.LifetimeEnd, DT, LI,
ClMaxLifetimes);
if (StandardLifetime) {
@@ -616,10 +615,5 @@ bool AArch64StackTagging::runOnFunction(Function &Fn) {
memtag::annotateDebugRecords(Info, Tag);
}
- // If we have instrumented at least one alloca, all unrecognized lifetime
- // intrinsics have to go.
- for (auto *I : SInfo.UnrecognizedLifetimes)
- I->eraseFromParent();
-
return true;
}
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index fbaa651641566..e87bee79a6a69 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -1063,7 +1063,6 @@ struct FunctionStackPoisoner : public InstVisitor<FunctionStackPoisoner> {
};
SmallVector<AllocaPoisonCall, 8> DynamicAllocaPoisonCallVec;
SmallVector<AllocaPoisonCall, 8> StaticAllocaPoisonCallVec;
- bool HasUntracedLifetimeIntrinsic = false;
SmallVector<AllocaInst *, 1> DynamicAllocaVec;
SmallVector<IntrinsicInst *, 1> StackRestoreVec;
@@ -1097,14 +1096,6 @@ struct FunctionStackPoisoner : public InstVisitor<FunctionStackPoisoner> {
initializeCallbacks(*F.getParent());
- if (HasUntracedLifetimeIntrinsic) {
- // If there are lifetime intrinsics which couldn't be traced back to an
- // alloca, we may not know exactly when a variable enters scope, and
- // therefore should "fail safe" by not poisoning them.
- StaticAllocaPoisonCallVec.clear();
- DynamicAllocaPoisonCallVec.clear();
- }
-
processDynamicAllocas();
processStaticAllocas();
@@ -1231,13 +1222,7 @@ struct FunctionStackPoisoner : public InstVisitor<FunctionStackPoisoner> {
!ConstantInt::isValueValidForType(IntptrTy, SizeValue))
return;
// Find alloca instruction that corresponds to llvm.lifetime argument.
- // Currently we can only handle lifetime markers pointing to the
- // beginning of the alloca.
- AllocaInst *AI = findAllocaForValue(II.getArgOperand(1), true);
- if (!AI) {
- HasUntracedLifetimeIntrinsic = true;
- return;
- }
+ AllocaInst *AI = cast<AllocaInst>(II.getArgOperand(1));
// We're interested only in allocas we can handle.
if (!ASan.isInterestingAlloca(*AI))
return;
diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index 2c34bf2157cdd..5849c3e475a6a 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -1500,7 +1500,6 @@ bool HWAddressSanitizer::instrumentStack(memtag::StackInfo &SInfo,
// statement if return_twice functions are called.
bool StandardLifetime =
!SInfo.CallsReturnTwice &&
- SInfo.UnrecognizedLifetimes.empty() &&
memtag::isStandardLifetime(Info.LifetimeStart, Info.LifetimeEnd, &DT,
&LI, ClMaxLifetimes);
if (DetectUseAfterScope && StandardLifetime) {
@@ -1525,8 +1524,6 @@ bool HWAddressSanitizer::instrumentStack(memtag::StackInfo &SInfo,
}
memtag::alignAndPadAlloca(Info, Mapping.getObjectAlignment());
}
- for (auto &I : SInfo.UnrecognizedLifetimes)
- I->eraseFromParent();
return true;
}
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 7b5831652aa6e..599b60dfba988 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -1216,7 +1216,6 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
};
SmallVector<ShadowOriginAndInsertPoint, 16> InstrumentationList;
DenseMap<const DILocation *, int> LazyWarningDebugLocationCount;
- bool InstrumentLifetimeStart = ClHandleLifetimeIntrinsics;
SmallSetVector<AllocaInst *, 16> AllocaSet;
SmallVector<std::pair<IntrinsicInst *, AllocaInst *>, 16> LifetimeStartList;
SmallVector<StoreInst *, 16> StoreList;
@@ -1623,7 +1622,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
// Poison llvm.lifetime.start intrinsics, if we haven't fallen back to
// instrumenting only allocas.
- if (InstrumentLifetimeStart) {
+ if (ClHandleLifetimeIntrinsics) {
for (auto Item : LifetimeStartList) {
instrumentAlloca(*Item.second, Item.first);
AllocaSet.remove(Item.second);
@@ -3303,9 +3302,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
void handleLifetimeStart(IntrinsicInst &I) {
if (!PoisonStack)
return;
- AllocaInst *AI = llvm::findAllocaForValue(I.getArgOperand(1));
- if (!AI)
- InstrumentLifetimeStart = false;
+ AllocaInst *AI = cast<AllocaInst>(I.getArgOperand(1));
LifetimeStartList.push_back(std::make_pair(&I, AI));
}
diff --git a/llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp b/llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp
index 7828571123bca..1d83ddc6b5a96 100644
--- a/llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp
@@ -343,8 +343,7 @@ static bool markTails(Function &F, OptimizationRemarkEmitter *ORE) {
///
static bool canMoveAboveCall(Instruction *I, CallInst *CI, AliasAnalysis *AA) {
if (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(I))
- if (II->getIntrinsicID() == Intrinsic::lifetime_end &&
- llvm::findAllocaForValue(II->getArgOperand(1)))
+ if (II->getIntrinsicID() == Intrinsic::lifetime_end)
return true;
// FIXME: We can move load/store/call/free instructions above the call if the
diff --git a/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp b/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
index 40dc02c546dfa..bea76d39bb216 100644
--- a/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
+++ b/llvm/lib/Transforms/Utils/MemoryTaggingSupport.cpp
@@ -155,11 +155,7 @@ void StackInfoBuilder::visit(OptimizationRemarkEmitter &ORE,
return;
}
if (auto *II = dyn_cast<LifetimeIntrinsic>(&Inst)) {
- AllocaInst *AI = findAllocaForValue(II->getArgOperand(1));
- if (!AI) {
- Info.UnrecognizedLifetimes.push_back(&Inst);
- return;
- }
+ AllocaInst *AI = cast<AllocaInst>(II->getArgOperand(1));
if (getAllocaInterestingness(*AI) != AllocaInterestingness::kInteresting)
return;
if (II->getIntrinsicID() == Intrinsic::lifetime_start)
|
After #149310 the pointer argument of lifetime.start/lifetime.end is guaranteed to be an alloca, so we don't need to go through findAllocaForValue() anymore, and don't have to have special handling for the case where it fails.