Skip to content

Commit b41240b

Browse files
steakhalsteakhal
authored
[analyzer][NFC] Introduce APSIntPtr, a safe wrapper of APSInt (1/4) (llvm#120435)
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.
1 parent 056e5ec commit b41240b

File tree

9 files changed

+148
-90
lines changed

9 files changed

+148
-90
lines changed

‎clang/include/clang/StaticAnalyzer/Core/PathSensitive/APSIntPtr.h‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
//== APSIntPtr.h - Wrapper for APSInt objects owned separately -*- C++ -*--==//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_APSIntPtr_H
10+
#define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_APSIntPtr_H
11+
12+
#include "llvm/ADT/APSInt.h"
13+
#include "llvm/Support/Compiler.h"
14+
15+
namespace clang::ento {
16+
17+
/// A safe wrapper around APSInt objects allocated and owned by
18+
/// \c BasicValueFactory. This just wraps a common llvm::APSInt.
19+
class APSIntPtr {
20+
using APSInt = llvm::APSInt;
21+
22+
public:
23+
APSIntPtr() = delete;
24+
APSIntPtr(const APSIntPtr &) = default;
25+
APSIntPtr &operator=(const APSIntPtr &) & = default;
26+
~APSIntPtr() = default;
27+
28+
/// You should not use this API.
29+
/// If do, ensure that the \p Ptr not going to dangle.
30+
/// Prefer using \c BasicValueFactory::getValue() to get an APSIntPtr object.
31+
static APSIntPtr unsafeConstructor(const APSInt *Ptr) {
32+
return APSIntPtr(Ptr);
33+
}
34+
35+
LLVM_ATTRIBUTE_RETURNS_NONNULL
36+
const APSInt *get() const { return Ptr; }
37+
/*implicit*/ operator const APSInt &() const { return *get(); }
38+
39+
APSInt operator-() const { return -*Ptr; }
40+
APSInt operator~() const { return ~*Ptr; }
41+
42+
#define DEFINE_OPERATOR(OP) \
43+
bool operator OP(APSIntPtr Other) const { return (*Ptr)OP(*Other.Ptr); }
44+
DEFINE_OPERATOR(>)
45+
DEFINE_OPERATOR(>=)
46+
DEFINE_OPERATOR(<)
47+
DEFINE_OPERATOR(<=)
48+
DEFINE_OPERATOR(==)
49+
DEFINE_OPERATOR(!=)
50+
#undef DEFINE_OPERATOR
51+
52+
const APSInt &operator*() const { return *Ptr; }
53+
const APSInt *operator->() const { return Ptr; }
54+
55+
private:
56+
explicit APSIntPtr(const APSInt *Ptr) : Ptr(Ptr) {}
57+
58+
/// Owned by \c BasicValueFactory.
59+
const APSInt *Ptr;
60+
};
61+
62+
} // namespace clang::ento
63+
64+
#endif // LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_APSIntPtr_H

‎clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h‎

Lines changed: 26 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,11 @@
1818
#include "clang/AST/ASTContext.h"
1919
#include "clang/AST/Expr.h"
2020
#include "clang/AST/Type.h"
21+
#include "clang/StaticAnalyzer/Core/PathSensitive/APSIntPtr.h"
2122
#include "clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h"
23+
#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
2224
#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
2325
#include "clang/StaticAnalyzer/Core/PathSensitive/StoreRef.h"
24-
#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
2526
#include "llvm/ADT/APSInt.h"
2627
#include "llvm/ADT/FoldingSet.h"
2728
#include "llvm/ADT/ImmutableList.h"
@@ -129,7 +130,7 @@ class BasicValueFactory {
129130

130131
// This is private because external clients should use the factory
131132
// method that takes a QualType.
132-
const llvm::APSInt& getValue(uint64_t X, unsigned BitWidth, bool isUnsigned);
133+
APSIntPtr getValue(uint64_t X, unsigned BitWidth, bool isUnsigned);
133134

134135
public:
135136
BasicValueFactory(ASTContext &ctx, llvm::BumpPtrAllocator &Alloc)
@@ -140,9 +141,9 @@ class BasicValueFactory {
140141

141142
ASTContext &getContext() const { return Ctx; }
142143

143-
const llvm::APSInt& getValue(const llvm::APSInt& X);
144-
const llvm::APSInt& getValue(const llvm::APInt& X, bool isUnsigned);
145-
const llvm::APSInt& getValue(uint64_t X, QualType T);
144+
APSIntPtr getValue(const llvm::APSInt &X);
145+
APSIntPtr getValue(const llvm::APInt &X, bool isUnsigned);
146+
APSIntPtr getValue(uint64_t X, QualType T);
146147

147148
/// Returns the type of the APSInt used to store values of the given QualType.
148149
APSIntType getAPSIntType(QualType T) const {
@@ -165,79 +166,70 @@ class BasicValueFactory {
165166

166167
/// Convert - Create a new persistent APSInt with the same value as 'From'
167168
/// but with the bitwidth and signedness of 'To'.
168-
const llvm::APSInt &Convert(const llvm::APSInt& To,
169-
const llvm::APSInt& From) {
169+
APSIntPtr Convert(const llvm::APSInt &To, const llvm::APSInt &From) {
170170
APSIntType TargetType(To);
171171
if (TargetType == APSIntType(From))
172-
return From;
172+
return getValue(From);
173173

174174
return getValue(TargetType.convert(From));
175175
}
176176

177-
const llvm::APSInt &Convert(QualType T, const llvm::APSInt &From) {
177+
APSIntPtr Convert(QualType T, const llvm::APSInt &From) {
178178
APSIntType TargetType = getAPSIntType(T);
179179
return Convert(TargetType, From);
180180
}
181181

182-
const llvm::APSInt &Convert(APSIntType TargetType, const llvm::APSInt &From) {
182+
APSIntPtr Convert(APSIntType TargetType, const llvm::APSInt &From) {
183183
if (TargetType == APSIntType(From))
184-
return From;
184+
return getValue(From);
185185

186186
return getValue(TargetType.convert(From));
187187
}
188188

189-
const llvm::APSInt &getIntValue(uint64_t X, bool isUnsigned) {
189+
APSIntPtr getIntValue(uint64_t X, bool isUnsigned) {
190190
QualType T = isUnsigned ? Ctx.UnsignedIntTy : Ctx.IntTy;
191191
return getValue(X, T);
192192
}
193193

194-
const llvm::APSInt &getMaxValue(const llvm::APSInt &v) {
194+
APSIntPtr getMaxValue(const llvm::APSInt &v) {
195195
return getValue(APSIntType(v).getMaxValue());
196196
}
197197

198-
const llvm::APSInt &getMinValue(const llvm::APSInt &v) {
198+
APSIntPtr getMinValue(const llvm::APSInt &v) {
199199
return getValue(APSIntType(v).getMinValue());
200200
}
201201

202-
const llvm::APSInt &getMaxValue(QualType T) {
203-
return getMaxValue(getAPSIntType(T));
204-
}
202+
APSIntPtr getMaxValue(QualType T) { return getMaxValue(getAPSIntType(T)); }
205203

206-
const llvm::APSInt &getMinValue(QualType T) {
207-
return getMinValue(getAPSIntType(T));
208-
}
204+
APSIntPtr getMinValue(QualType T) { return getMinValue(getAPSIntType(T)); }
209205

210-
const llvm::APSInt &getMaxValue(APSIntType T) {
211-
return getValue(T.getMaxValue());
212-
}
206+
APSIntPtr getMaxValue(APSIntType T) { return getValue(T.getMaxValue()); }
213207

214-
const llvm::APSInt &getMinValue(APSIntType T) {
215-
return getValue(T.getMinValue());
216-
}
208+
APSIntPtr getMinValue(APSIntType T) { return getValue(T.getMinValue()); }
217209

218-
const llvm::APSInt &Add1(const llvm::APSInt &V) {
210+
APSIntPtr Add1(const llvm::APSInt &V) {
219211
llvm::APSInt X = V;
220212
++X;
221213
return getValue(X);
222214
}
223215

224-
const llvm::APSInt &Sub1(const llvm::APSInt &V) {
216+
APSIntPtr Sub1(const llvm::APSInt &V) {
225217
llvm::APSInt X = V;
226218
--X;
227219
return getValue(X);
228220
}
229221

230-
const llvm::APSInt &getZeroWithTypeSize(QualType T) {
222+
APSIntPtr getZeroWithTypeSize(QualType T) {
231223
assert(T->isScalarType());
232224
return getValue(0, Ctx.getTypeSize(T), true);
233225
}
234226

235-
const llvm::APSInt &getTruthValue(bool b, QualType T) {
227+
APSIntPtr getTruthValue(bool b, QualType T) {
236228
return getValue(b ? 1 : 0, Ctx.getIntWidth(T),
237229
T->isUnsignedIntegerOrEnumerationType());
238230
}
239231

240-
const llvm::APSInt &getTruthValue(bool b) {
232+
APSIntPtr getTruthValue(bool b) {
241233
return getTruthValue(b, Ctx.getLogicalOperationType());
242234
}
243235

@@ -273,9 +265,9 @@ class BasicValueFactory {
273265
accumCXXBase(llvm::iterator_range<CastExpr::path_const_iterator> PathRange,
274266
const nonloc::PointerToMember &PTM, const clang::CastKind &kind);
275267

276-
const llvm::APSInt* evalAPSInt(BinaryOperator::Opcode Op,
277-
const llvm::APSInt& V1,
278-
const llvm::APSInt& V2);
268+
std::optional<APSIntPtr> evalAPSInt(BinaryOperator::Opcode Op,
269+
const llvm::APSInt &V1,
270+
const llvm::APSInt &V2);
279271

280272
const std::pair<SVal, uintptr_t>&
281273
getPersistentSValWithData(const SVal& V, uintptr_t Data);

‎clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ template <typename T> struct ProgramStateTrait {
7070
/// values will never change.
7171
class ProgramState : public llvm::FoldingSetNode {
7272
public:
73-
typedef llvm::ImmutableSet<llvm::APSInt*> IntSetTy;
7473
typedef llvm::ImmutableMap<void*, void*> GenericDataMap;
7574

7675
private:

‎clang/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
#include "clang/Basic/JsonSupport.h"
1818
#include "clang/Basic/TargetInfo.h"
19+
#include "clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h"
1920
#include "clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h"
2021
#include "clang/StaticAnalyzer/Core/PathSensitive/SMTConv.h"
2122
#include <optional>
@@ -154,7 +155,7 @@ class SMTConstraintManager : public clang::ento::SimpleConstraintManager {
154155
return nullptr;
155156

156157
// This is the only solution, store it
157-
return &BVF.getValue(Value);
158+
return BVF.getValue(Value).get();
158159
}
159160

160161
if (const SymbolCast *SC = dyn_cast<SymbolCast>(Sym)) {
@@ -167,7 +168,7 @@ class SMTConstraintManager : public clang::ento::SimpleConstraintManager {
167168
const llvm::APSInt *Value;
168169
if (!(Value = getSymVal(State, CastSym)))
169170
return nullptr;
170-
return &BVF.Convert(SC->getType(), *Value);
171+
return BVF.Convert(SC->getType(), *Value).get();
171172
}
172173

173174
if (const BinarySymExpr *BSE = dyn_cast<BinarySymExpr>(Sym)) {
@@ -195,7 +196,9 @@ class SMTConstraintManager : public clang::ento::SimpleConstraintManager {
195196
std::tie(ConvertedRHS, RTy) = SMTConv::fixAPSInt(Ctx, *RHS);
196197
SMTConv::doIntTypeConversion<llvm::APSInt, &SMTConv::castAPSInt>(
197198
Solver, Ctx, ConvertedLHS, LTy, ConvertedRHS, RTy);
198-
return BVF.evalAPSInt(BSE->getOpcode(), ConvertedLHS, ConvertedRHS);
199+
std::optional<APSIntPtr> Res =
200+
BVF.evalAPSInt(BSE->getOpcode(), ConvertedLHS, ConvertedRHS);
201+
return Res ? Res.value().get() : nullptr;
199202
}
200203

201204
llvm_unreachable("Unsupported expression to get symbol value!");

‎clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1025,8 +1025,8 @@ SVal CStringChecker::getCStringLengthForRegion(CheckerContext &C,
10251025
BasicValueFactory &BVF = svalBuilder.getBasicValueFactory();
10261026
const llvm::APSInt &maxValInt = BVF.getMaxValue(sizeTy);
10271027
llvm::APSInt fourInt = APSIntType(maxValInt).getValue(4);
1028-
const llvm::APSInt *maxLengthInt = BVF.evalAPSInt(BO_Div, maxValInt,
1029-
fourInt);
1028+
std::optional<APSIntPtr> maxLengthInt =
1029+
BVF.evalAPSInt(BO_Div, maxValInt, fourInt);
10301030
NonLoc maxLength = svalBuilder.makeIntVal(*maxLengthInt);
10311031
SVal evalLength = svalBuilder.evalBinOpNN(state, BO_LE, *strLn, maxLength,
10321032
svalBuilder.getConditionType());

‎clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1643,7 +1643,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
16431643
public:
16441644
GetMaxValue(BasicValueFactory &BVF) : BVF(BVF) {}
16451645
std::optional<RangeInt> operator()(QualType Ty) {
1646-
return BVF.getMaxValue(Ty).getLimitedValue();
1646+
return BVF.getMaxValue(Ty)->getLimitedValue();
16471647
}
16481648
std::optional<RangeInt> operator()(std::optional<QualType> Ty) {
16491649
if (Ty) {
@@ -1687,11 +1687,11 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
16871687
const QualType SizePtrTy = getPointerTy(SizeTy);
16881688
const QualType SizePtrRestrictTy = getRestrictTy(SizePtrTy);
16891689

1690-
const RangeInt IntMax = BVF.getMaxValue(IntTy).getLimitedValue();
1690+
const RangeInt IntMax = BVF.getMaxValue(IntTy)->getLimitedValue();
16911691
const RangeInt UnsignedIntMax =
1692-
BVF.getMaxValue(UnsignedIntTy).getLimitedValue();
1693-
const RangeInt LongMax = BVF.getMaxValue(LongTy).getLimitedValue();
1694-
const RangeInt SizeMax = BVF.getMaxValue(SizeTy).getLimitedValue();
1692+
BVF.getMaxValue(UnsignedIntTy)->getLimitedValue();
1693+
const RangeInt LongMax = BVF.getMaxValue(LongTy)->getLimitedValue();
1694+
const RangeInt SizeMax = BVF.getMaxValue(SizeTy)->getLimitedValue();
16951695

16961696
// Set UCharRangeMax to min of int or uchar maximum value.
16971697
// The C standard states that the arguments of functions like isalpha must
@@ -1700,7 +1700,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
17001700
// to be true for commonly used and well tested instruction set
17011701
// architectures, but not for others.
17021702
const RangeInt UCharRangeMax =
1703-
std::min(BVF.getMaxValue(ACtx.UnsignedCharTy).getLimitedValue(), IntMax);
1703+
std::min(BVF.getMaxValue(ACtx.UnsignedCharTy)->getLimitedValue(), IntMax);
17041704

17051705
// Get platform dependent values of some macros.
17061706
// Try our best to parse this from the Preprocessor, otherwise fallback to a
@@ -3704,7 +3704,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
37043704

37053705
// Functions for testing.
37063706
if (AddTestFunctions) {
3707-
const RangeInt IntMin = BVF.getMinValue(IntTy).getLimitedValue();
3707+
const RangeInt IntMin = BVF.getMinValue(IntTy)->getLimitedValue();
37083708

37093709
addToFunctionSummaryMap(
37103710
"__not_null", Signature(ArgTypes{IntPtrTy}, RetType{IntTy}),

‎clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ ProgramStateRef VLASizeChecker::checkVLA(CheckerContext &C,
9696
SValBuilder &SVB = C.getSValBuilder();
9797
CanQualType SizeTy = Ctx.getSizeType();
9898
uint64_t SizeMax =
99-
SVB.getBasicValueFactory().getMaxValue(SizeTy).getZExtValue();
99+
SVB.getBasicValueFactory().getMaxValue(SizeTy)->getZExtValue();
100100

101101
// Get the element size.
102102
CharUnits EleSize = Ctx.getTypeSizeInChars(VLALast->getElementType());

0 commit comments

Comments
 (0)