[analyzer][NFC] Introduce APSIntPtr, a safe wrapper of APSInt (1/4) #120435
Conversation
This file contains hidden or 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
This was referenced Dec 18, 2024
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-static-analyzer-1 Author: Balazs Benics (steakhal) ChangesOnce could create dangling APSInt references in various ways in the past, that were sometimes assumed to be persisted in the BasicValueFactor. One should always use BasicValueFactory to create persistent APSInts, that could be used by ConcreteInts or SymIntExprs and similar long-living objects. The idea is that APSIntPtr is always owned by the BasicValueFactory, and that is the only component that can construct it. These PRs are all NFC - besides fixing dangling APSInt references. Patch is 23.28 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/120435.diff 9 Files Affected:
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/APSIntPtr.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/APSIntPtr.h
new file mode 100644
index 00000000000000..7b6cf13e7df1c4
--- /dev/null
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/APSIntPtr.h
@@ -0,0 +1,64 @@
+//== APSIntPtr.h - Wrapper for APSInt objects owned separately -*- C++ -*--==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_APSIntPtr_H
+#define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_APSIntPtr_H
+
+#include "llvm/ADT/APSInt.h"
+
+namespace clang::ento {
+class BasicValueFactory;
+
+/// A safe wrapper around APSInt objects allocated and owned by
+/// \c BasicValueFactory. This just wraps a common llvm::APSInt.
+class APSIntPtr {
+ using APSInt = llvm::APSInt;
+
+public:
+ APSIntPtr() = delete;
+ APSIntPtr(const APSIntPtr &) = default;
+ APSIntPtr &operator=(const APSIntPtr &) & = default;
+ ~APSIntPtr() = default;
+
+ /// You should not use this API.
+ /// If do, ensure that the \p Ptr not going to dangle.
+ /// Prefer using \c BasicValueFactory::getValue() to get an APSIntPtr object.
+ static APSIntPtr unsafeConstructor(const APSInt *Ptr) {
+ return APSIntPtr(Ptr);
+ }
+
+ const APSInt *get() const { return Ptr; }
+ /*implicit*/ operator const APSInt &() const { return *get(); }
+
+ APSInt operator-() const { return -*Ptr; }
+ APSInt operator~() const { return ~*Ptr; }
+
+#define DEFINE_OPERATOR(OP) \
+ bool operator OP(APSIntPtr Other) const { return (*Ptr)OP(*Other.Ptr); }
+ DEFINE_OPERATOR(>)
+ DEFINE_OPERATOR(>=)
+ DEFINE_OPERATOR(<)
+ DEFINE_OPERATOR(<=)
+ DEFINE_OPERATOR(==)
+ DEFINE_OPERATOR(!=)
+#undef DEFINE_OPERATOR
+
+ const APSInt &operator*() const { return *Ptr; }
+ const APSInt *operator->() const { return Ptr; }
+
+private:
+ friend class BasicValueFactory;
+ explicit APSIntPtr(const APSInt *Ptr) : Ptr(Ptr) {}
+
+ /// Owned by \c BasicValueFactory.
+ const APSInt *Ptr;
+};
+
+} // namespace clang::ento
+
+#endif // LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_APSIntPtr_H
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
index ec503b41b381a5..e5a256d1749900 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
@@ -18,10 +18,11 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/Expr.h"
#include "clang/AST/Type.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/APSIntPtr.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/StoreRef.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
#include "llvm/ADT/APSInt.h"
#include "llvm/ADT/FoldingSet.h"
#include "llvm/ADT/ImmutableList.h"
@@ -129,7 +130,7 @@ class BasicValueFactory {
// This is private because external clients should use the factory
// method that takes a QualType.
- const llvm::APSInt& getValue(uint64_t X, unsigned BitWidth, bool isUnsigned);
+ APSIntPtr getValue(uint64_t X, unsigned BitWidth, bool isUnsigned);
public:
BasicValueFactory(ASTContext &ctx, llvm::BumpPtrAllocator &Alloc)
@@ -140,9 +141,9 @@ class BasicValueFactory {
ASTContext &getContext() const { return Ctx; }
- const llvm::APSInt& getValue(const llvm::APSInt& X);
- const llvm::APSInt& getValue(const llvm::APInt& X, bool isUnsigned);
- const llvm::APSInt& getValue(uint64_t X, QualType T);
+ APSIntPtr getValue(const llvm::APSInt &X);
+ APSIntPtr getValue(const llvm::APInt &X, bool isUnsigned);
+ APSIntPtr getValue(uint64_t X, QualType T);
/// Returns the type of the APSInt used to store values of the given QualType.
APSIntType getAPSIntType(QualType T) const {
@@ -165,79 +166,70 @@ class BasicValueFactory {
/// Convert - Create a new persistent APSInt with the same value as 'From'
/// but with the bitwidth and signedness of 'To'.
- const llvm::APSInt &Convert(const llvm::APSInt& To,
- const llvm::APSInt& From) {
+ APSIntPtr Convert(const llvm::APSInt &To, const llvm::APSInt &From) {
APSIntType TargetType(To);
if (TargetType == APSIntType(From))
- return From;
+ return APSIntPtr(&From);
return getValue(TargetType.convert(From));
}
- const llvm::APSInt &Convert(QualType T, const llvm::APSInt &From) {
+ APSIntPtr Convert(QualType T, const llvm::APSInt &From) {
APSIntType TargetType = getAPSIntType(T);
return Convert(TargetType, From);
}
- const llvm::APSInt &Convert(APSIntType TargetType, const llvm::APSInt &From) {
+ APSIntPtr Convert(APSIntType TargetType, const llvm::APSInt &From) {
if (TargetType == APSIntType(From))
- return From;
+ return APSIntPtr(&From);
return getValue(TargetType.convert(From));
}
- const llvm::APSInt &getIntValue(uint64_t X, bool isUnsigned) {
+ APSIntPtr getIntValue(uint64_t X, bool isUnsigned) {
QualType T = isUnsigned ? Ctx.UnsignedIntTy : Ctx.IntTy;
return getValue(X, T);
}
- const llvm::APSInt &getMaxValue(const llvm::APSInt &v) {
+ APSIntPtr getMaxValue(const llvm::APSInt &v) {
return getValue(APSIntType(v).getMaxValue());
}
- const llvm::APSInt &getMinValue(const llvm::APSInt &v) {
+ APSIntPtr getMinValue(const llvm::APSInt &v) {
return getValue(APSIntType(v).getMinValue());
}
- const llvm::APSInt &getMaxValue(QualType T) {
- return getMaxValue(getAPSIntType(T));
- }
+ APSIntPtr getMaxValue(QualType T) { return getMaxValue(getAPSIntType(T)); }
- const llvm::APSInt &getMinValue(QualType T) {
- return getMinValue(getAPSIntType(T));
- }
+ APSIntPtr getMinValue(QualType T) { return getMinValue(getAPSIntType(T)); }
- const llvm::APSInt &getMaxValue(APSIntType T) {
- return getValue(T.getMaxValue());
- }
+ APSIntPtr getMaxValue(APSIntType T) { return getValue(T.getMaxValue()); }
- const llvm::APSInt &getMinValue(APSIntType T) {
- return getValue(T.getMinValue());
- }
+ APSIntPtr getMinValue(APSIntType T) { return getValue(T.getMinValue()); }
- const llvm::APSInt &Add1(const llvm::APSInt &V) {
+ APSIntPtr Add1(const llvm::APSInt &V) {
llvm::APSInt X = V;
++X;
return getValue(X);
}
- const llvm::APSInt &Sub1(const llvm::APSInt &V) {
+ APSIntPtr Sub1(const llvm::APSInt &V) {
llvm::APSInt X = V;
--X;
return getValue(X);
}
- const llvm::APSInt &getZeroWithTypeSize(QualType T) {
+ APSIntPtr getZeroWithTypeSize(QualType T) {
assert(T->isScalarType());
return getValue(0, Ctx.getTypeSize(T), true);
}
- const llvm::APSInt &getTruthValue(bool b, QualType T) {
+ APSIntPtr getTruthValue(bool b, QualType T) {
return getValue(b ? 1 : 0, Ctx.getIntWidth(T),
T->isUnsignedIntegerOrEnumerationType());
}
- const llvm::APSInt &getTruthValue(bool b) {
+ APSIntPtr getTruthValue(bool b) {
return getTruthValue(b, Ctx.getLogicalOperationType());
}
@@ -273,9 +265,9 @@ class BasicValueFactory {
accumCXXBase(llvm::iterator_range<CastExpr::path_const_iterator> PathRange,
const nonloc::PointerToMember &PTM, const clang::CastKind &kind);
- const llvm::APSInt* evalAPSInt(BinaryOperator::Opcode Op,
- const llvm::APSInt& V1,
- const llvm::APSInt& V2);
+ std::optional<APSIntPtr> evalAPSInt(BinaryOperator::Opcode Op,
+ const llvm::APSInt &V1,
+ const llvm::APSInt &V2);
const std::pair<SVal, uintptr_t>&
getPersistentSValWithData(const SVal& V, uintptr_t Data);
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
index 29f534eba2a265..a20516b003c7d9 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
@@ -70,7 +70,6 @@ template <typename T> struct ProgramStateTrait {
/// values will never change.
class ProgramState : public llvm::FoldingSetNode {
public:
- typedef llvm::ImmutableSet<llvm::APSInt*> IntSetTy;
typedef llvm::ImmutableMap<void*, void*> GenericDataMap;
private:
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
index 5766af1fc78a4f..72038b92f8edfe 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
@@ -16,6 +16,7 @@
#include "clang/Basic/JsonSupport.h"
#include "clang/Basic/TargetInfo.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SMTConv.h"
#include <optional>
@@ -154,7 +155,7 @@ class SMTConstraintManager : public clang::ento::SimpleConstraintManager {
return nullptr;
// This is the only solution, store it
- return &BVF.getValue(Value);
+ return BVF.getValue(Value).get();
}
if (const SymbolCast *SC = dyn_cast<SymbolCast>(Sym)) {
@@ -167,7 +168,7 @@ class SMTConstraintManager : public clang::ento::SimpleConstraintManager {
const llvm::APSInt *Value;
if (!(Value = getSymVal(State, CastSym)))
return nullptr;
- return &BVF.Convert(SC->getType(), *Value);
+ return BVF.Convert(SC->getType(), *Value).get();
}
if (const BinarySymExpr *BSE = dyn_cast<BinarySymExpr>(Sym)) {
@@ -195,7 +196,9 @@ class SMTConstraintManager : public clang::ento::SimpleConstraintManager {
std::tie(ConvertedRHS, RTy) = SMTConv::fixAPSInt(Ctx, *RHS);
SMTConv::doIntTypeConversion<llvm::APSInt, &SMTConv::castAPSInt>(
Solver, Ctx, ConvertedLHS, LTy, ConvertedRHS, RTy);
- return BVF.evalAPSInt(BSE->getOpcode(), ConvertedLHS, ConvertedRHS);
+ std::optional<APSIntPtr> Res =
+ BVF.evalAPSInt(BSE->getOpcode(), ConvertedLHS, ConvertedRHS);
+ return Res ? Res.value().get() : nullptr;
}
llvm_unreachable("Unsupported expression to get symbol value!");
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 21a2d8828249d1..1a14f38e34f0e1 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -1025,8 +1025,8 @@ SVal CStringChecker::getCStringLengthForRegion(CheckerContext &C,
BasicValueFactory &BVF = svalBuilder.getBasicValueFactory();
const llvm::APSInt &maxValInt = BVF.getMaxValue(sizeTy);
llvm::APSInt fourInt = APSIntType(maxValInt).getValue(4);
- const llvm::APSInt *maxLengthInt = BVF.evalAPSInt(BO_Div, maxValInt,
- fourInt);
+ std::optional<APSIntPtr> maxLengthInt =
+ BVF.evalAPSInt(BO_Div, maxValInt, fourInt);
NonLoc maxLength = svalBuilder.makeIntVal(*maxLengthInt);
SVal evalLength = svalBuilder.evalBinOpNN(state, BO_LE, *strLn, maxLength,
svalBuilder.getConditionType());
diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 4f30b2a0e7e7da..356d63e3e8b80f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -1643,7 +1643,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
public:
GetMaxValue(BasicValueFactory &BVF) : BVF(BVF) {}
std::optional<RangeInt> operator()(QualType Ty) {
- return BVF.getMaxValue(Ty).getLimitedValue();
+ return BVF.getMaxValue(Ty)->getLimitedValue();
}
std::optional<RangeInt> operator()(std::optional<QualType> Ty) {
if (Ty) {
@@ -1687,11 +1687,11 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
const QualType SizePtrTy = getPointerTy(SizeTy);
const QualType SizePtrRestrictTy = getRestrictTy(SizePtrTy);
- const RangeInt IntMax = BVF.getMaxValue(IntTy).getLimitedValue();
+ const RangeInt IntMax = BVF.getMaxValue(IntTy)->getLimitedValue();
const RangeInt UnsignedIntMax =
- BVF.getMaxValue(UnsignedIntTy).getLimitedValue();
- const RangeInt LongMax = BVF.getMaxValue(LongTy).getLimitedValue();
- const RangeInt SizeMax = BVF.getMaxValue(SizeTy).getLimitedValue();
+ BVF.getMaxValue(UnsignedIntTy)->getLimitedValue();
+ const RangeInt LongMax = BVF.getMaxValue(LongTy)->getLimitedValue();
+ const RangeInt SizeMax = BVF.getMaxValue(SizeTy)->getLimitedValue();
// Set UCharRangeMax to min of int or uchar maximum value.
// The C standard states that the arguments of functions like isalpha must
@@ -1700,7 +1700,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
// to be true for commonly used and well tested instruction set
// architectures, but not for others.
const RangeInt UCharRangeMax =
- std::min(BVF.getMaxValue(ACtx.UnsignedCharTy).getLimitedValue(), IntMax);
+ std::min(BVF.getMaxValue(ACtx.UnsignedCharTy)->getLimitedValue(), IntMax);
// Get platform dependent values of some macros.
// Try our best to parse this from the Preprocessor, otherwise fallback to a
@@ -3704,7 +3704,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
// Functions for testing.
if (AddTestFunctions) {
- const RangeInt IntMin = BVF.getMinValue(IntTy).getLimitedValue();
+ const RangeInt IntMin = BVF.getMinValue(IntTy)->getLimitedValue();
addToFunctionSummaryMap(
"__not_null", Signature(ArgTypes{IntPtrTy}, RetType{IntTy}),
diff --git a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
index 8d17ba5d690b90..ba91b3632abbfe 100644
--- a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
@@ -96,7 +96,7 @@ ProgramStateRef VLASizeChecker::checkVLA(CheckerContext &C,
SValBuilder &SVB = C.getSValBuilder();
CanQualType SizeTy = Ctx.getSizeType();
uint64_t SizeMax =
- SVB.getBasicValueFactory().getMaxValue(SizeTy).getZExtValue();
+ SVB.getBasicValueFactory().getMaxValue(SizeTy)->getZExtValue();
// Get the element size.
CharUnits EleSize = Ctx.getTypeSizeInChars(VLALast->getElementType());
diff --git a/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp b/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
index 827c04143e6588..de4879ab714220 100644
--- a/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
@@ -87,7 +87,7 @@ BasicValueFactory::~BasicValueFactory() {
delete (PersistentSValPairsTy*) PersistentSValPairs;
}
-const llvm::APSInt& BasicValueFactory::getValue(const llvm::APSInt& X) {
+APSIntPtr BasicValueFactory::getValue(const llvm::APSInt &X) {
llvm::FoldingSetNodeID ID;
void *InsertPos;
@@ -101,23 +101,22 @@ const llvm::APSInt& BasicValueFactory::getValue(const llvm::APSInt& X) {
APSIntSet.InsertNode(P, InsertPos);
}
- return *P;
+ return APSIntPtr(&P->getValue());
}
-const llvm::APSInt& BasicValueFactory::getValue(const llvm::APInt& X,
- bool isUnsigned) {
+APSIntPtr BasicValueFactory::getValue(const llvm::APInt &X, bool isUnsigned) {
llvm::APSInt V(X, isUnsigned);
return getValue(V);
}
-const llvm::APSInt& BasicValueFactory::getValue(uint64_t X, unsigned BitWidth,
- bool isUnsigned) {
+APSIntPtr BasicValueFactory::getValue(uint64_t X, unsigned BitWidth,
+ bool isUnsigned) {
llvm::APSInt V(BitWidth, isUnsigned);
V = X;
return getValue(V);
}
-const llvm::APSInt& BasicValueFactory::getValue(uint64_t X, QualType T) {
+APSIntPtr BasicValueFactory::getValue(uint64_t X, QualType T) {
return getValue(getAPSIntType(T).getValue(X));
}
@@ -242,45 +241,45 @@ const PointerToMemberData *BasicValueFactory::accumCXXBase(
return getPointerToMemberData(ND, BaseSpecList);
}
-const llvm::APSInt*
-BasicValueFactory::evalAPSInt(BinaryOperator::Opcode Op,
- const llvm::APSInt& V1, const llvm::APSInt& V2) {
+std::optional<APSIntPtr>
+BasicValueFactory::evalAPSInt(BinaryOperator::Opcode Op, const llvm::APSInt &V1,
+ const llvm::APSInt &V2) {
switch (Op) {
default:
llvm_unreachable("Invalid Opcode.");
case BO_Mul:
- return &getValue( V1 * V2 );
+ return getValue(V1 * V2);
case BO_Div:
if (V2 == 0) // Avoid division by zero
- return nullptr;
- return &getValue( V1 / V2 );
+ return std::nullopt;
+ return getValue(V1 / V2);
case BO_Rem:
if (V2 == 0) // Avoid division by zero
- return nullptr;
- return &getValue( V1 % V2 );
+ return std::nullopt;
+ return getValue(V1 % V2);
case BO_Add:
- return &getValue( V1 + V2 );
+ return getValue(V1 + V2);
case BO_Sub:
- return &getValue( V1 - V2 );
+ return getValue(V1 - V2);
case BO_Shl: {
// FIXME: This logic should probably go higher up, where we can
// test these conditions symbolically.
if (V2.isNegative() || V2.getBitWidth() > 64)
- return nullptr;
+ return std::nullopt;
uint64_t Amt = V2.getZExtValue();
if (Amt >= V1.getBitWidth())
- return nullptr;
+ return std::nullopt;
- return &getValue( V1.operator<<( (unsigned) Amt ));
+ return getValue(V1.operator<<((unsigned)Amt));
}
case BO_Shr: {
@@ -288,44 +287,44 @@ BasicValueFactory::evalAPSInt(BinaryOperator::Opcode Op,
// test these conditions symbolically.
if (V2.isNegative() || V2.getBitWidth() > 64)
- return nullptr;
+ return std::nullopt;
uint64_t Amt = V2.getZExtValue();
if (Amt >= V1.getBitWidth())
- return nullptr;
+ return std::nullopt;
- return &getValue( V1.operator>>( (unsigned) Amt ));
+ return getValue(V1.operator>>((unsigned)Amt));
}
case BO_LT:
- return &getTruthValue( V1 < V2 );
+ return getTruthValue(V1 < V2);
case BO_GT:
- return &getTruthValue( V1 > V2 );
+ return getTruthValue(V1 > V2);
case BO_LE:
- return &getTruthValue( V1 <= V2 );
+ return getTruthValue(V1 <= V2);
case BO_GE:
- return &getTruthValue( V1 >= V2 );
+ return getTruthValue(V1 >= V2);
case BO_EQ:
- return &getTruthValue( V1 == V2 );
+ return getTruthValue(V1 == V2);
case BO_NE:
- return &getTruthValue( V1 != V2 );
+ return getTruthValue(V1 != V2);
// Note: LAnd, LOr, Comma are handled specially by higher-level logic.
case BO_And:
- return &getValue( V1 & V2 );
+ return getValue(V1 & V2);
case BO_Or:
- return &getValue( V1 | V2 );
+ return getValue(V1 | V2);
case BO_Xor:
- return &getValue( V1 ^ V2 );
+ return getValue(V1 ^ V2);
}
}
diff --git a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
index 229169f848e228..7b7fc801ec7f4a 100644
--- a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -193,7 +193,7 @@ SVal SimpleSValBuilder::MakeSymIntVal(const SymExpr *LHS,
// If we reach this point, the expression cannot be simplified.
// Make a SymbolVal for...
[truncated]
|
Xazax-hun
reviewed
Dec 18, 2024
Xazax-hun
approved these changes
Dec 18, 2024
|
I see that |
I think I'd prefer keeping it here because this API would be here all at once. |
|
OK, the dependency argument is fair. I am with this as is. |
|
Thank you @Xazax-hun for so swiftly reviewing all these PRs. It means a lot <3 |
necto
suggested changes
Dec 19, 2024
clang/include/clang/StaticAnalyzer/Core/PathSensitive/APSIntPtr.h
Outdated
Show resolved
Hide resolved
clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
Outdated
Show resolved
Hide resolved
steakhal
force-pushed
the
users/steakhal/bb/fix-dangling-aps-ints-1
branch
from
eddf348 to
587368a
Compare
necto
approved these changes
Dec 19, 2024
NagyDonat
reviewed
Dec 19, 2024
There was a problem hiding this comment.
Thanks for doing this NFC code fortification, I appreciate the effort!
I quickly read through all comments in this stack, and I approve their goals and high-level structure. I'm not (yet) giving a formal approval only because I didn't dig into the small details to uncover potential bugs and I see that others already gave more through reviews. (If it helps, I can review and presumably approve this or some follow-up commits, although I'm on vacation after Dec 20 until the end of this year.)
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.
One could create dangling APSInt references in various ways in the past, that were sometimes assumed to be persisted in the BasicValueFactor.
One should always use BasicValueFactory to create persistent APSInts, that could be used by ConcreteInts or SymIntExprs and similar long-living objects.
If one used a temporary or local variables for this, these would dangle.
To enforce the contract of the analyzer BasicValueFactory and the uses of APSInts, let's have a dedicated strong-type for this.
The idea is that APSIntPtr is always owned by the BasicValueFactory, and that is the only component that can construct it.
These PRs are all NFC - besides fixing dangling APSInt references.