steakhal created this revision.
steakhal added reviewers: NoQ, Szelethus, xazax.hun, baloghadamsoftware,
vsavchenko, martong, gamesh411.
Herald added subscribers: cfe-commits, ASDenysPetrov, Charusso, dkrupp,
donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, whisperity.
Herald added a project: clang.
steakhal requested review of this revision.
Refines the CStringLength modeling API to be pure.
Also removes the magic //hypothetical// parameter from the CStringChecker.
Now the getters and setters are behaving as expected.
This is enforced by taking the State as `const` pointer.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D84979
Files:
clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringChecker.cpp
clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringChecker.h
clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringLength.h
clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringLengthModeling.cpp
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringLengthModeling.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringLengthModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringLengthModeling.cpp
@@ -78,16 +78,11 @@
return State->remove<CStringLengthMap>(MR);
}
-static SVal getCStringLengthForRegion(CheckerContext &Ctx,
- ProgramStateRef &State, const Expr *Ex,
- const MemRegion *MR, bool Hypothetical) {
- if (!Hypothetical) {
- // If there's a recorded length, go ahead and return it.
- if (const SVal *Recorded = State->get<CStringLengthMap>(MR))
- return *Recorded;
- }
+NonLoc cstring::createCStringLength(ProgramStateRef &State, CheckerContext &Ctx,
+ const Expr *Ex, const MemRegion *MR) {
+ assert(Ex);
+ assert(MR);
- // Otherwise, get a new symbol and update the state.
SValBuilder &SVB = Ctx.getSValBuilder();
QualType SizeTy = SVB.getContext().getSizeType();
NonLoc CStrLen =
@@ -95,25 +90,22 @@
Ctx.getLocationContext(), Ctx.blockCount())
.castAs<NonLoc>();
- if (!Hypothetical) {
- // In case of unbounded calls strlen etc bound the range to SIZE_MAX/4
- BasicValueFactory &BVF = SVB.getBasicValueFactory();
- const llvm::APSInt &MaxValue = BVF.getMaxValue(SizeTy);
- const llvm::APSInt Four = APSIntType(MaxValue).getValue(4);
- const llvm::APSInt *MaxLength = BVF.evalAPSInt(BO_Div, MaxValue, Four);
- const NonLoc MaxLengthSVal = SVB.makeIntVal(*MaxLength);
- SVal Constrained =
- SVB.evalBinOpNN(State, BO_LE, CStrLen, MaxLengthSVal, SizeTy);
- State = State->assume(Constrained.castAs<DefinedOrUnknownSVal>(), true);
- State = State->set<CStringLengthMap>(MR, CStrLen);
- }
-
+ // Implicitly constrain the range to SIZE_MAX/4
+ BasicValueFactory &BVF = SVB.getBasicValueFactory();
+ const llvm::APSInt &MaxValue = BVF.getMaxValue(SizeTy);
+ const llvm::APSInt Four = APSIntType(MaxValue).getValue(4);
+ const llvm::APSInt *MaxLength = BVF.evalAPSInt(BO_Div, MaxValue, Four);
+ const NonLoc MaxLengthSVal = SVB.makeIntVal(*MaxLength);
+ SVal Constrained =
+ SVB.evalBinOpNN(State, BO_LE, CStrLen, MaxLengthSVal, SizeTy);
+ State = State->assume(Constrained.castAs<DefinedOrUnknownSVal>(), true);
+ State = State->set<CStringLengthMap>(MR, CStrLen);
return CStrLen;
}
-SVal cstring::getCStringLength(CheckerContext &Ctx, ProgramStateRef &State,
- const Expr *Ex, SVal Buf,
- bool Hypothetical /*=false*/) {
+Optional<SVal> cstring::getCStringLength(CheckerContext &Ctx,
+ const ProgramStateRef &State,
+ SVal Buf) {
if (Buf.isUnknownOrUndef())
return Buf;
@@ -145,7 +137,9 @@
case MemRegion::ParamVarRegionKind:
case MemRegion::FieldRegionKind:
case MemRegion::ObjCIvarRegionKind:
- return getCStringLengthForRegion(Ctx, State, Ex, MR, Hypothetical);
+ if (const SVal *RecordedLength = State->get<CStringLengthMap>(MR))
+ return *RecordedLength;
+ return llvm::None;
case MemRegion::CompoundLiteralRegionKind:
// FIXME: Can we track this? Is it necessary?
return UnknownVal();
@@ -159,7 +153,7 @@
}
}
-void cstring::dumpCStringLengths(ProgramStateRef State, raw_ostream &Out,
+void cstring::dumpCStringLengths(const ProgramStateRef State, raw_ostream &Out,
const char *NL, const char *Sep) {
const CStringLengthMapTy Items = State->get<CStringLengthMap>();
if (!Items.isEmpty())
@@ -212,7 +206,7 @@
SVal StrVal = C.getSVal(Init);
assert(StrVal.isValid() && "Initializer string is unknown or undefined");
DefinedOrUnknownSVal strLength =
- getCStringLength(C, state, Init, StrVal).castAs<DefinedOrUnknownSVal>();
+ getCStringLength(C, state, StrVal)->castAs<DefinedOrUnknownSVal>();
state = state->set<CStringLengthMap>(MR, strLength);
}
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringLength.h
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringLength.h
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringLength.h
@@ -36,13 +36,18 @@
LLVM_NODISCARD ProgramStateRef removeCStringLength(ProgramStateRef State,
const MemRegion *MR);
-// FIXME: Eventually rework the interface of this function.
-// Especially the magic 'Hypothetical' parameter.
-LLVM_NODISCARD SVal getCStringLength(CheckerContext &Ctx,
- ProgramStateRef &State, const Expr *Ex,
- SVal Buf, bool Hypothetical = false);
-
-LLVM_DUMP_METHOD void dumpCStringLengths(ProgramStateRef State,
+/// Gets the associated cstring length of a region.
+/// If no such exists, None returned.
+LLVM_NODISCARD Optional<SVal>
+getCStringLength(CheckerContext &Ctx, const ProgramStateRef &State, SVal Buf);
+
+/// Creates a metadata symbol, tracking the cstring length of the given region.
+/// It implicitly applies certain constraints to the created value.
+LLVM_NODISCARD NonLoc createCStringLength(ProgramStateRef &State,
+ CheckerContext &Ctx, const Expr *Ex,
+ const MemRegion *MR);
+
+LLVM_DUMP_METHOD void dumpCStringLengths(const ProgramStateRef State,
raw_ostream &Out = llvm::errs(),
const char *NL = "\n",
const char *Sep = " : ");
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringChecker.h
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringChecker.h
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringChecker.h
@@ -168,8 +168,7 @@
/// Simply wraps the cstring::getCStringLength function to emit warnings.
SVal getCStringLengthChecked(CheckerContext &Ctx, ProgramStateRef &State,
- const Expr *Ex, SVal Buf,
- bool hypothetical = false) const;
+ const Expr *Ex, SVal Buf) const;
std::pair<ProgramStateRef, ProgramStateRef> static assumeZero(
CheckerContext &C, ProgramStateRef state, SVal V, QualType Ty);
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringChecker.cpp
@@ -72,9 +72,15 @@
SVal CStringChecker::getCStringLengthChecked(CheckerContext &Ctx,
ProgramStateRef &State,
- const Expr *Ex, SVal Buf,
- bool hypothetical) const {
- SVal CStrLen = cstring::getCStringLength(Ctx, State, Ex, Buf, hypothetical);
+ const Expr *Ex, SVal Buf) const {
+ // Try to get the associated cstring length, if fails, create a new one.
+ const SVal CStrLen = [&]() -> SVal {
+ Optional<SVal> Tmp = cstring::getCStringLength(Ctx, State, Buf);
+ if (Tmp.hasValue())
+ return Tmp.getValue();
+ return cstring::createCStringLength(State, Ctx, Ex,
+ Buf.getAsRegion()->StripCasts());
+ }();
// Simply return if everything goes well.
// Otherwise we shall investigate why did it fail.
@@ -1456,9 +1462,10 @@
// If we couldn't get a single value for the final string length,
// we can at least bound it by the individual lengths.
if (finalStrLength.isUnknown()) {
- // Try to get a "hypothetical" string length symbol, which we can later
+ // Get a //hypothetical// string length symbol, which we can later
// set as a real value if that turns out to be the case.
- finalStrLength = getCStringLengthChecked(C, state, CE, DstVal, true);
+ finalStrLength =
+ cstring::createCStringLength(state, C, CE, DstVal.getAsRegion());
assert(!finalStrLength.isUndef());
if (Optional<NonLoc> finalStrLengthNL = finalStrLength.getAs<NonLoc>()) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
