[llvm] Use computeConstantRange to improve llvm.objectsize computation #114673
+206
−12
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-llvm-transforms Author: None (serge-sans-paille) ChangesUsing LazyValueInfo, it is possible to compute valuable information for allocation functions, GEP and alloca, even in the presence of dynamic information. llvm.objectsize plays an important role in _FORTIFY_SOURCE definitions, so improving its diagnostic in turns improves the security of compiled application. As a side note, as a result of recent optimization improvements, clang no longer passes https://github.com/serge-sans-paille/builtin_object_size-test-suite This commit restores the situation and greatly improves the scope of code handled by the static version of __builtin_object_size. Patch is 46.40 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/114673.diff 17 Files Affected:
diff --git a/llvm/include/llvm/Analysis/MemoryBuiltins.h b/llvm/include/llvm/Analysis/MemoryBuiltins.h
index c3b11cdf5cf5db..feb4bc7b8c98da 100644
--- a/llvm/include/llvm/Analysis/MemoryBuiltins.h
+++ b/llvm/include/llvm/Analysis/MemoryBuiltins.h
@@ -42,6 +42,7 @@ class IntegerType;
class IntrinsicInst;
class IntToPtrInst;
class LLVMContext;
+class LazyValueInfo;
class LoadInst;
class PHINode;
class SelectInst;
@@ -160,8 +161,10 @@ struct ObjectSizeOpts {
/// though they can't be evaluated. Otherwise, null is always considered to
/// point to a 0 byte region of memory.
bool NullIsUnknownSize = false;
- /// If set, used for more accurate evaluation
+ /// If set, used for more accurate evaluation.
AAResults *AA = nullptr;
+ /// If set, used for more accurate evaluation.
+ LazyValueInfo *LVI = nullptr;
};
/// Compute the size of the object pointed by Ptr. Returns true and the
@@ -186,6 +189,12 @@ Value *lowerObjectSizeCall(
const TargetLibraryInfo *TLI, AAResults *AA, bool MustSucceed,
SmallVectorImpl<Instruction *> *InsertedInstructions = nullptr);
+Value *lowerObjectSizeCall(
+ IntrinsicInst *ObjectSize, const DataLayout &DL,
+ const TargetLibraryInfo *TLI, AAResults *AA, LazyValueInfo *LVI,
+ bool MustSucceed,
+ SmallVectorImpl<Instruction *> *InsertedInstructions = nullptr);
+
/// SizeOffsetType - A base template class for the object size visitors. Used
/// here as a self-documenting way to handle the values rather than using a
/// \p std::pair.
@@ -275,6 +284,7 @@ class ObjectSizeOffsetVisitor
OffsetSpan visitExtractValueInst(ExtractValueInst &I);
OffsetSpan visitGlobalAlias(GlobalAlias &GA);
OffsetSpan visitGlobalVariable(GlobalVariable &GV);
+ OffsetSpan visitGetElementPtr(GetElementPtrInst &GEP);
OffsetSpan visitIntToPtrInst(IntToPtrInst &);
OffsetSpan visitLoadInst(LoadInst &I);
OffsetSpan visitPHINode(PHINode &);
diff --git a/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h b/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
index 3075b7ebae59e6..72861684a7f754 100644
--- a/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
+++ b/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
@@ -37,6 +37,7 @@ namespace llvm {
class AAResults;
class AssumptionCache;
class OptimizationRemarkEmitter;
+class LazyValueInfo;
class ProfileSummaryInfo;
class TargetLibraryInfo;
class TargetTransformInfo;
@@ -72,6 +73,7 @@ class LLVM_LIBRARY_VISIBILITY InstCombiner {
// Required analyses.
AssumptionCache &AC;
TargetLibraryInfo &TLI;
+ LazyValueInfo &LVI;
DominatorTree &DT;
const DataLayout &DL;
SimplifyQuery SQ;
@@ -101,14 +103,15 @@ class LLVM_LIBRARY_VISIBILITY InstCombiner {
InstCombiner(InstructionWorklist &Worklist, BuilderTy &Builder,
bool MinimizeSize, AAResults *AA, AssumptionCache &AC,
TargetLibraryInfo &TLI, TargetTransformInfo &TTI,
- DominatorTree &DT, OptimizationRemarkEmitter &ORE,
- BlockFrequencyInfo *BFI, BranchProbabilityInfo *BPI,
- ProfileSummaryInfo *PSI, const DataLayout &DL,
+ LazyValueInfo &LVI, DominatorTree &DT,
+ OptimizationRemarkEmitter &ORE, BlockFrequencyInfo *BFI,
+ BranchProbabilityInfo *BPI, ProfileSummaryInfo *PSI,
+ const DataLayout &DL,
ReversePostOrderTraversal<BasicBlock *> &RPOT)
: TTIForTargetIntrinsicsOnly(TTI), Builder(Builder), Worklist(Worklist),
- MinimizeSize(MinimizeSize), AA(AA), AC(AC), TLI(TLI), DT(DT), DL(DL),
- SQ(DL, &TLI, &DT, &AC, nullptr, /*UseInstrInfo*/ true,
- /*CanUseUndef*/ true, &DC),
+ MinimizeSize(MinimizeSize), AA(AA), AC(AC), TLI(TLI), LVI(LVI), DT(DT),
+ DL(DL), SQ(DL, &TLI, &DT, &AC, nullptr, /*UseInstrInfo*/ true,
+ /*CanUseUndef*/ true, &DC),
ORE(ORE), BFI(BFI), BPI(BPI), PSI(PSI), RPOT(RPOT) {}
virtual ~InstCombiner() = default;
diff --git a/llvm/include/llvm/Transforms/Scalar/LowerConstantIntrinsics.h b/llvm/include/llvm/Transforms/Scalar/LowerConstantIntrinsics.h
index 3d4d1034c0b24d..86f542109c1cf0 100644
--- a/llvm/include/llvm/Transforms/Scalar/LowerConstantIntrinsics.h
+++ b/llvm/include/llvm/Transforms/Scalar/LowerConstantIntrinsics.h
@@ -22,9 +22,10 @@ namespace llvm {
class DominatorTree;
class Function;
class TargetLibraryInfo;
+class LazyValueInfo;
bool lowerConstantIntrinsics(Function &F, const TargetLibraryInfo &TLI,
- DominatorTree *DT);
+ DominatorTree *DT, LazyValueInfo *LVI);
struct LowerConstantIntrinsicsPass :
PassInfoMixin<LowerConstantIntrinsicsPass> {
diff --git a/llvm/lib/Analysis/MemoryBuiltins.cpp b/llvm/lib/Analysis/MemoryBuiltins.cpp
index 71400ac46bdcbf..a934fb7cc52558 100644
--- a/llvm/lib/Analysis/MemoryBuiltins.cpp
+++ b/llvm/lib/Analysis/MemoryBuiltins.cpp
@@ -16,6 +16,8 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/Statistic.h"
#include "llvm/Analysis/AliasAnalysis.h"
+#include "llvm/Analysis/AssumptionCache.h"
+#include "llvm/Analysis/LazyValueInfo.h"
#include "llvm/Analysis/TargetFolder.h"
#include "llvm/Analysis/TargetLibraryInfo.h"
#include "llvm/Analysis/Utils/Local.h"
@@ -590,19 +592,28 @@ Value *llvm::lowerObjectSizeCall(IntrinsicInst *ObjectSize,
const TargetLibraryInfo *TLI,
bool MustSucceed) {
return lowerObjectSizeCall(ObjectSize, DL, TLI, /*AAResults=*/nullptr,
- MustSucceed);
+ /*LazyValueInfo=*/nullptr, MustSucceed);
}
Value *llvm::lowerObjectSizeCall(
IntrinsicInst *ObjectSize, const DataLayout &DL,
const TargetLibraryInfo *TLI, AAResults *AA, bool MustSucceed,
SmallVectorImpl<Instruction *> *InsertedInstructions) {
+ return lowerObjectSizeCall(ObjectSize, DL, TLI, AA, /*LazyValueInfo=*/nullptr,
+ MustSucceed, InsertedInstructions);
+}
+
+Value *llvm::lowerObjectSizeCall(
+ IntrinsicInst *ObjectSize, const DataLayout &DL,
+ const TargetLibraryInfo *TLI, AAResults *AA, LazyValueInfo *LVI,
+ bool MustSucceed, SmallVectorImpl<Instruction *> *InsertedInstructions) {
assert(ObjectSize->getIntrinsicID() == Intrinsic::objectsize &&
"ObjectSize must be a call to llvm.objectsize!");
bool MaxVal = cast<ConstantInt>(ObjectSize->getArgOperand(1))->isZero();
ObjectSizeOpts EvalOptions;
EvalOptions.AA = AA;
+ EvalOptions.LVI = LVI;
// Unless we have to fold this to something, try to be as accurate as
// possible.
@@ -716,7 +727,6 @@ OffsetSpan ObjectSizeOffsetVisitor::computeImpl(Value *V) {
// value that is passed to computeImpl.
IntTyBits = DL.getIndexTypeSizeInBits(V->getType());
Zero = APInt::getZero(IntTyBits);
-
OffsetSpan ORT = computeValue(V);
bool IndexTypeSizeChanged = InitialIntTyBits != IntTyBits;
@@ -794,6 +804,17 @@ OffsetSpan ObjectSizeOffsetVisitor::visitAllocaInst(AllocaInst &I) {
Size = Size.umul_ov(NumElems, Overflow);
return Overflow ? ObjectSizeOffsetVisitor::unknown()
: OffsetSpan(Zero, align(Size, I.getAlign()));
+ } else if (Options.LVI) {
+ ConstantRange CR =
+ Options.LVI->getConstantRange(const_cast<Value *>(ArraySize), &I, true);
+ if (CR.isFullSet())
+ return ObjectSizeOffsetVisitor::unknown();
+ APInt Bound;
+ if (Options.EvalMode == ObjectSizeOpts::Mode::Max)
+ Bound = CR.getUnsignedMax();
+ else
+ Bound = CR.getUnsignedMin();
+ return OffsetSpan(Zero, align(Bound, I.getAlign()));
}
return ObjectSizeOffsetVisitor::unknown();
}
@@ -811,7 +832,23 @@ OffsetSpan ObjectSizeOffsetVisitor::visitArgument(Argument &A) {
}
OffsetSpan ObjectSizeOffsetVisitor::visitCallBase(CallBase &CB) {
- if (std::optional<APInt> Size = getAllocSize(&CB, TLI))
+ if (std::optional<APInt> Size =
+ getAllocSize(&CB, TLI, [&CB, this](const Value *V) -> const Value * {
+ if (!Options.LVI)
+ return V;
+ if (!V->getType()->isIntegerTy())
+ return V;
+ ConstantRange CR = Options.LVI->getConstantRange(
+ const_cast<Value *>(V), &CB, true);
+ if (CR.isFullSet())
+ return V;
+ APInt Bound;
+ if (Options.EvalMode == ObjectSizeOpts::Mode::Max)
+ Bound = CR.getUnsignedMax();
+ else
+ Bound = CR.getUnsignedMin();
+ return ConstantInt::get(V->getType(), Bound);
+ }))
return OffsetSpan(Zero, *Size);
return ObjectSizeOffsetVisitor::unknown();
}
@@ -856,6 +893,42 @@ OffsetSpan ObjectSizeOffsetVisitor::visitGlobalVariable(GlobalVariable &GV) {
return OffsetSpan(Zero, align(Size, GV.getAlign()));
}
+OffsetSpan ObjectSizeOffsetVisitor::visitGetElementPtr(GetElementPtrInst &GEP) {
+ OffsetSpan PtrData = computeImpl(GEP.getPointerOperand());
+ if (!PtrData.bothKnown())
+ return ObjectSizeOffsetVisitor::unknown();
+
+ if (!Options.LVI)
+ return ObjectSizeOffsetVisitor::unknown();
+
+ if (Options.EvalMode == ObjectSizeOpts::Mode::Min ||
+ Options.EvalMode == ObjectSizeOpts::Mode::Max) {
+ unsigned BitWidth = PtrData.After.getBitWidth();
+ APInt ConstantOffset = Zero;
+ SmallMapVector<Value *, APInt, 4> VariableOffsets;
+ if (!GEP.collectOffset(DL, BitWidth, VariableOffsets, ConstantOffset))
+ return ObjectSizeOffsetVisitor::unknown();
+
+ ConstantRange AccumulatedRange = ConstantOffset;
+ for (auto const &VO : VariableOffsets) {
+ ConstantRange CR = Options.LVI->getConstantRange(VO.first, &GEP, true);
+ if (CR.isFullSet())
+ return ObjectSizeOffsetVisitor::unknown();
+
+ AccumulatedRange = AccumulatedRange.add(CR.multiply(VO.second));
+ }
+
+ APInt Bound;
+ if (Options.EvalMode == ObjectSizeOpts::Mode::Min)
+ Bound = AccumulatedRange.getSignedMax();
+ else
+ Bound = AccumulatedRange.getSignedMin();
+
+ return {PtrData.Before + Bound, PtrData.After - Bound};
+ }
+ return ObjectSizeOffsetVisitor::unknown();
+}
+
OffsetSpan ObjectSizeOffsetVisitor::visitIntToPtrInst(IntToPtrInst &) {
// clueless
return ObjectSizeOffsetVisitor::unknown();
diff --git a/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp b/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
index 3373b76edb268f..ba42af6151e27b 100644
--- a/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
+++ b/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
@@ -350,7 +350,8 @@ bool PreISelIntrinsicLowering::lowerIntrinsics(Module &M) const {
Function *Parent = CI->getParent()->getParent();
TargetLibraryInfo &TLI = LookupTLI(*Parent);
// Intrinsics in unreachable code are not lowered.
- bool Changed = lowerConstantIntrinsics(*Parent, TLI, /*DT=*/nullptr);
+ bool Changed = lowerConstantIntrinsics(*Parent, TLI, /*DT=*/nullptr,
+ /*LVI=*/nullptr);
return Changed;
});
break;
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
index adbd9186c59c5a..820be6fc2bb1f6 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
+++ b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
@@ -63,12 +63,13 @@ class LLVM_LIBRARY_VISIBILITY InstCombinerImpl final
InstCombinerImpl(InstructionWorklist &Worklist, BuilderTy &Builder,
bool MinimizeSize, AAResults *AA, AssumptionCache &AC,
TargetLibraryInfo &TLI, TargetTransformInfo &TTI,
- DominatorTree &DT, OptimizationRemarkEmitter &ORE,
- BlockFrequencyInfo *BFI, BranchProbabilityInfo *BPI,
- ProfileSummaryInfo *PSI, const DataLayout &DL,
+ LazyValueInfo &LVI, DominatorTree &DT,
+ OptimizationRemarkEmitter &ORE, BlockFrequencyInfo *BFI,
+ BranchProbabilityInfo *BPI, ProfileSummaryInfo *PSI,
+ const DataLayout &DL,
ReversePostOrderTraversal<BasicBlock *> &RPOT)
- : InstCombiner(Worklist, Builder, MinimizeSize, AA, AC, TLI, TTI, DT, ORE,
- BFI, BPI, PSI, DL, RPOT) {}
+ : InstCombiner(Worklist, Builder, MinimizeSize, AA, AC, TLI, TTI, LVI, DT,
+ ORE, BFI, BPI, PSI, DL, RPOT) {}
virtual ~InstCombinerImpl() = default;
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 2a54390c0f1882..9466d17272888c 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -48,6 +48,7 @@
#include "llvm/Analysis/GlobalsModRef.h"
#include "llvm/Analysis/InstructionSimplify.h"
#include "llvm/Analysis/LazyBlockFrequencyInfo.h"
+#include "llvm/Analysis/LazyValueInfo.h"
#include "llvm/Analysis/MemoryBuiltins.h"
#include "llvm/Analysis/OptimizationRemarkEmitter.h"
#include "llvm/Analysis/ProfileSummaryInfo.h"
@@ -3317,8 +3318,9 @@ Instruction *InstCombinerImpl::visitAllocSite(Instruction &MI) {
if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(I)) {
if (II->getIntrinsicID() == Intrinsic::objectsize) {
SmallVector<Instruction *> InsertedInstructions;
- Value *Result = lowerObjectSizeCall(
- II, DL, &TLI, AA, /*MustSucceed=*/true, &InsertedInstructions);
+ Value *Result =
+ lowerObjectSizeCall(II, DL, &TLI, AA, &LVI, /*MustSucceed=*/true,
+ &InsertedInstructions);
for (Instruction *Inserted : InsertedInstructions)
Worklist.add(Inserted);
replaceInstUsesWith(*I, Result);
@@ -5460,9 +5462,9 @@ void InstCombiner::computeBackEdges() {
static bool combineInstructionsOverFunction(
Function &F, InstructionWorklist &Worklist, AliasAnalysis *AA,
AssumptionCache &AC, TargetLibraryInfo &TLI, TargetTransformInfo &TTI,
- DominatorTree &DT, OptimizationRemarkEmitter &ORE, BlockFrequencyInfo *BFI,
- BranchProbabilityInfo *BPI, ProfileSummaryInfo *PSI,
- const InstCombineOptions &Opts) {
+ LazyValueInfo &LVI, DominatorTree &DT, OptimizationRemarkEmitter &ORE,
+ BlockFrequencyInfo *BFI, BranchProbabilityInfo *BPI,
+ ProfileSummaryInfo *PSI, const InstCombineOptions &Opts) {
auto &DL = F.getDataLayout();
bool VerifyFixpoint = Opts.VerifyFixpoint &&
!F.hasFnAttribute("instcombine-no-verify-fixpoint");
@@ -5501,8 +5503,8 @@ static bool combineInstructionsOverFunction(
LLVM_DEBUG(dbgs() << "\n\nINSTCOMBINE ITERATION #" << Iteration << " on "
<< F.getName() << "\n");
- InstCombinerImpl IC(Worklist, Builder, F.hasMinSize(), AA, AC, TLI, TTI, DT,
- ORE, BFI, BPI, PSI, DL, RPOT);
+ InstCombinerImpl IC(Worklist, Builder, F.hasMinSize(), AA, AC, TLI, TTI,
+ LVI, DT, ORE, BFI, BPI, PSI, DL, RPOT);
IC.MaxArraySizeForCombine = MaxArraySize;
bool MadeChangeInThisIteration = IC.prepareWorklist(F);
MadeChangeInThisIteration |= IC.run();
@@ -5552,6 +5554,7 @@ PreservedAnalyses InstCombinePass::run(Function &F,
auto &TLI = AM.getResult<TargetLibraryAnalysis>(F);
auto &ORE = AM.getResult<OptimizationRemarkEmitterAnalysis>(F);
auto &TTI = AM.getResult<TargetIRAnalysis>(F);
+ auto &LVI = AM.getResult<LazyValueAnalysis>(F);
auto *AA = &AM.getResult<AAManager>(F);
auto &MAMProxy = AM.getResult<ModuleAnalysisManagerFunctionProxy>(F);
@@ -5561,8 +5564,8 @@ PreservedAnalyses InstCombinePass::run(Function &F,
&AM.getResult<BlockFrequencyAnalysis>(F) : nullptr;
auto *BPI = AM.getCachedResult<BranchProbabilityAnalysis>(F);
- if (!combineInstructionsOverFunction(F, Worklist, AA, AC, TLI, TTI, DT, ORE,
- BFI, BPI, PSI, Options))
+ if (!combineInstructionsOverFunction(F, Worklist, AA, AC, TLI, TTI, LVI, DT,
+ ORE, BFI, BPI, PSI, Options))
// No changes, all analyses are preserved.
return PreservedAnalyses::all();
@@ -5580,6 +5583,7 @@ void InstructionCombiningPass::getAnalysisUsage(AnalysisUsage &AU) const {
AU.addRequired<TargetTransformInfoWrapperPass>();
AU.addRequired<DominatorTreeWrapperPass>();
AU.addRequired<OptimizationRemarkEmitterWrapperPass>();
+ AU.addRequired<LazyValueInfoWrapperPass>();
AU.addPreserved<DominatorTreeWrapperPass>();
AU.addPreserved<AAResultsWrapperPass>();
AU.addPreserved<BasicAAWrapperPass>();
@@ -5597,6 +5601,7 @@ bool InstructionCombiningPass::runOnFunction(Function &F) {
auto &AC = getAnalysis<AssumptionCacheTracker>().getAssumptionCache(F);
auto &TLI = getAnalysis<TargetLibraryInfoWrapperPass>().getTLI(F);
auto &TTI = getAnalysis<TargetTransformInfoWrapperPass>().getTTI(F);
+ auto &LVI = getAnalysis<LazyValueInfoWrapperPass>().getLVI();
auto &DT = getAnalysis<DominatorTreeWrapperPass>().getDomTree();
auto &ORE = getAnalysis<OptimizationRemarkEmitterWrapperPass>().getORE();
@@ -5612,8 +5617,9 @@ bool InstructionCombiningPass::runOnFunction(Function &F) {
getAnalysisIfAvailable<BranchProbabilityInfoWrapperPass>())
BPI = &WrapperPass->getBPI();
- return combineInstructionsOverFunction(F, Worklist, AA, AC, TLI, TTI, DT, ORE,
- BFI, BPI, PSI, InstCombineOptions());
+ return combineInstructionsOverFunction(F, Worklist, AA, AC, TLI, TTI, LVI, DT,
+ ORE, BFI, BPI, PSI,
+ InstCombineOptions());
}
char InstructionCombiningPass::ID = 0;
@@ -5630,6 +5636,7 @@ INITIALIZE_PASS_DEPENDENCY(TargetTransformInfoWrapperPass)
INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)
INITIALIZE_PASS_DEPENDENCY(AAResultsWrapperPass)
INITIALIZE_PASS_DEPENDENCY(GlobalsAAWrapperPass)
+INITIALIZE_PASS_DEPENDENCY(LazyValueInfoWrapperPass)
INITIALIZE_PASS_DEPENDENCY(OptimizationRemarkEmitterWrapperPass)
INITIALIZE_PASS_DEPENDENCY(LazyBlockFrequencyInfoPass)
INITIALIZE_PASS_DEPENDENCY(ProfileSummaryInfoWrapperPass)
diff --git a/llvm/lib/Transforms/Scalar/LowerConstantIntrinsics.cpp b/llvm/lib/Transforms/Scalar/LowerConstantIntrinsics.cpp
index 8dd72b8f1414ed..f764752695afdc 100644
--- a/llvm/lib/Transforms/Scalar/LowerConstantIntrinsics.cpp
+++ b/llvm/lib/Transforms/Scalar/LowerConstantIntrinsics.cpp
@@ -18,6 +18,7 @@
#include "llvm/Analysis/DomTreeUpdater.h"
#include "llvm/Analysis/GlobalsModRef.h"
#include "llvm/Analysis/InstructionSimplify.h"
+#include "llvm/Analysis/LazyValueInfo.h"
#include "llvm/Analysis/MemoryBuiltins.h"
#include "llvm/Analysis/TargetLibraryInfo.h"
#include "llvm/IR/BasicBlock.h"
@@ -100,7 +101,7 @@ static bool replaceConditionalBranchesOnConstant(Instruction *II,
}
bool llvm::lowerConstantIntrinsics(Function &F, const TargetLibraryInfo &TLI,
- DominatorTree *DT) {
+ DominatorTree *DT, LazyValueInfo *LVI) {
std::optional<DomTreeUpdater> DTU;
if (DT)
DTU.emplace(DT, DomTreeUpdater::UpdateStrategy::Lazy);
@@ -144,7 +145,7 @@ bool llvm::lowerConstantIntrinsics(Function &F, const TargetLibraryInfo &TLI,
IsConstantIntrinsicsHandled++;
break;
case Intrinsic::objectsize:
- NewValue = lowerObjectSizeCall(II, DL, &TLI, true);
+ NewValue = lowerObjectSizeCall(II, DL, &TLI, nullptr, LVI, true);
LLVM_DEBUG(dbgs() << "Folding " << *II << " to " << *NewValue << "\n");
ObjectSizeIntrinsicsHandled++;
break;
@@ -160,7 +161,8 @@ bool llvm::lowerConstantIntrinsics(Function &F, const TargetLibraryInfo &TLI,
PreservedAnalyses
LowerConstantIntrinsicsPass::run(Function &F, FunctionAnalysisManager &AM) {
if (lowerConstantIntrinsics(F, AM.getResult<TargetLibraryAnalysis>(F),
- AM.getCachedResult<DominatorTreeAnalysis>(F))) {
+ AM.getCachedResult<DominatorTreeAnalysis>(F),
+ &AM.getResult<LazyValueAnalysis>(F))) {
PreservedAnalyses PA;
PA.preserve<DominatorTreeAnalysis>();
return PA;
diff --git a/llvm/test/Other/new-pm-defaults.ll b/llvm/test/Other/new-pm-defaults.ll
index 55dbdb1b8366d6..17211...
[truncated]
|
nikic
requested changes
Nov 2, 2024
There was a problem hiding this comment.
We definitely cannot use LVI inside InstCombine for invalidation reasons. Using it in LowerConstantIntrinsics may be theoretically possible, but I really don't want to go there.
Can your motivating cases be covered by ValueTracking helpers, either computeConstantRange() or computeKnownBits()?
serge-sans-paille
force-pushed
the
feature/lvi-bos
branch
from
f64ca30
to
b1277d2
Compare
|
I've just submitted a version that uses
I'm not familiar with this. Thanks! |
serge-sans-paille
force-pushed
the
feature/lvi-bos
branch
from
b1277d2
to
59dd36c
Compare
InstCombine has a lot of different folds, and some of them do things like modifying instructions in place. It's essentially impossible to keep analyses that have non-trivial state up-to-date in InstCombine. |
|
more than 9k extra |
|
It feels a bit risky to use |
|
The problem I hinted at in my previous comment turns out to be a problem already even without your PR. Consider something like int f(int n) {
__builtin_assume(n >= 20);
int buf[n];
buf[10] = 0;
return buf[10];
}
int main(void) {
return f(3);
}It is good that in recent versions of Clang, with It is less good that without that I am not sure what the intended behaviour is here. Is this something we want to diagnose, or is this something we want to optimise away? |
agreed.
It's indeed tricky. From a strict I'll update the patch to not use assumptions, that's already a strict improvement. |
serge-sans-paille
force-pushed
the
feature/lvi-bos
branch
from
59dd36c
to
95739c6
Compare
hvdijk
reviewed
Nov 5, 2024
That is a good idea, we can always add assumptions in a later follow-up PR if we do want them. This approach looks good, thanks, just left a small comment about handling overflow but that should not change the main idea of your PR. |
serge-sans-paille
force-pushed
the
feature/lvi-bos
branch
from
95739c6
to
8842c80
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
hvdijk
reviewed
Nov 5, 2024
serge-sans-paille
force-pushed
the
feature/lvi-bos
branch
from
8842c80
to
5cc9b2e
Compare
hvdijk
approved these changes
Nov 6, 2024
There was a problem hiding this comment.
Thanks, looks good to me!
You updated the commit according to the change requested by @nikic, but the PR title and description are still the original, so just make sure that when you merge and squash, you do not accidentally overwrite your correct commit message with the older PR description :)
serge-sans-paille
changed the title
Using computeConstantRange, it is possible to compute valuable information for allocation functions, GEP and alloca, even in the presence of some dynamic information. llvm.objectsize plays an important role in _FORTIFY_SOURCE definitions, so improving its diagnostic in turns improves the security of compiled application. As a side note, as a result of recent optimization improvements, clang no longer passes https://github.com/serge-sans-paille/builtin_object_size-test-suite This commit restores the situation and greatly improves the scope of code handled by the static version of __builtin_object_size.
serge-sans-paille
force-pushed
the
feature/lvi-bos
branch
from
5cc9b2e
to
f30f00a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Using LazyValueInfo, it is possible to compute valuable information for allocation functions, GEP and alloca, even in the presence of dynamic information.
llvm.objectsize plays an important role in _FORTIFY_SOURCE definitions, so improving its diagnostic in turns improves the security of compiled application.
As a side note, as a result of recent optimization improvements, clang no longer passes https://github.com/serge-sans-paille/builtin_object_size-test-suite This commit restores the situation and greatly improves the scope of code handled by the static version of __builtin_object_size.