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 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 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 finalStrLengthNL = finalStrLength.getAs()) { 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 +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/CStringLengthModeling.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringLengthModeling.cpp +++ clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringLengthModeling.cpp @@ -78,16 +78,11 @@ return State->remove(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(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(); - 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(), true); - State = State->set(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(), true); + State = State->set(MR, CStrLen); return CStrLen; } -SVal cstring::getCStringLength(CheckerContext &Ctx, ProgramStateRef &State, - const Expr *Ex, SVal Buf, - bool Hypothetical /*=false*/) { +Optional 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(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(); 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(); + getCStringLength(C, state, StrVal)->castAs(); state = state->set(MR, strLength); }