diff --git a/clang/include/clang/Checker/PathSensitive/GRState.h b/clang/include/clang/Checker/PathSensitive/GRState.h index ac382898d8fa2b09e45492ee4948fcf961dd7263..878f564491700916e67cae280f4abc2aa2865291 100644 --- a/clang/include/clang/Checker/PathSensitive/GRState.h +++ b/clang/include/clang/Checker/PathSensitive/GRState.h @@ -650,7 +650,9 @@ inline SVal GRState::getLValue(const FieldDecl* D, SVal Base) const { } inline SVal GRState::getLValue(QualType ElementType, SVal Idx, SVal Base) const{ - return getStateManager().StoreMgr->getLValueElement(ElementType, Idx, Base); + if (NonLoc *N = dyn_cast(&Idx)) + return getStateManager().StoreMgr->getLValueElement(ElementType, *N, Base); + return UnknownVal(); } inline const llvm::APSInt *GRState::getSymVal(SymbolRef sym) const { diff --git a/clang/include/clang/Checker/PathSensitive/MemRegion.h b/clang/include/clang/Checker/PathSensitive/MemRegion.h index 96f906af28e14716e590a23a2df20cce7b33778e..82b0c14f20cb6378061ca6d0369cdf0584e8d173 100644 --- a/clang/include/clang/Checker/PathSensitive/MemRegion.h +++ b/clang/include/clang/Checker/PathSensitive/MemRegion.h @@ -788,9 +788,9 @@ class ElementRegion : public TypedRegion { friend class MemRegionManager; QualType ElementType; - SVal Index; + NonLoc Index; - ElementRegion(QualType elementType, SVal Idx, const MemRegion* sReg) + ElementRegion(QualType elementType, NonLoc Idx, const MemRegion* sReg) : TypedRegion(sReg, ElementRegionKind), ElementType(elementType), Index(Idx) { assert((!isa(&Idx) || @@ -803,7 +803,7 @@ class ElementRegion : public TypedRegion { public: - SVal getIndex() const { return Index; } + NonLoc getIndex() const { return Index; } QualType getValueType() const { return ElementType; @@ -942,7 +942,7 @@ public: /// getElementRegion - Retrieve the memory region associated with the /// associated element type, index, and super region. - const ElementRegion *getElementRegion(QualType elementType, SVal Idx, + const ElementRegion *getElementRegion(QualType elementType, NonLoc Idx, const MemRegion *superRegion, ASTContext &Ctx); diff --git a/clang/include/clang/Checker/PathSensitive/Store.h b/clang/include/clang/Checker/PathSensitive/Store.h index a1a41847a206abf9814f5b7d78a29810598e84cc..1ead46613699360f9c4e650919cc5fcec004d619 100644 --- a/clang/include/clang/Checker/PathSensitive/Store.h +++ b/clang/include/clang/Checker/PathSensitive/Store.h @@ -112,7 +112,7 @@ public: return getLValueFieldOrIvar(D, Base); } - virtual SVal getLValueElement(QualType elementType, SVal offset, SVal Base); + virtual SVal getLValueElement(QualType elementType, NonLoc offset, SVal Base); // FIXME: This should soon be eliminated altogether; clients should deal with // region extents directly. diff --git a/clang/lib/Checker/MemRegion.cpp b/clang/lib/Checker/MemRegion.cpp index 3f706e145a82adc9b97489973a0d467ff7933d62..ddcb7d2687a65aafcb8a54beb58d6f7283af33df 100644 --- a/clang/lib/Checker/MemRegion.cpp +++ b/clang/lib/Checker/MemRegion.cpp @@ -624,7 +624,7 @@ MemRegionManager::getCompoundLiteralRegion(const CompoundLiteralExpr* CL, } const ElementRegion* -MemRegionManager::getElementRegion(QualType elementType, SVal Idx, +MemRegionManager::getElementRegion(QualType elementType, NonLoc Idx, const MemRegion* superRegion, ASTContext& Ctx){ diff --git a/clang/lib/Checker/RegionStore.cpp b/clang/lib/Checker/RegionStore.cpp index 4051f4d39b8c9f9feeef82843da23cf0e5088401..91f7cfaf6c83df7002dcd2cace03cf9aae1ea06f 100644 --- a/clang/lib/Checker/RegionStore.cpp +++ b/clang/lib/Checker/RegionStore.cpp @@ -786,7 +786,7 @@ SVal RegionStoreManager::ArrayToPointer(Loc Array) { ArrayType *AT = cast(T); T = AT->getElementType(); - SVal ZeroIdx = ValMgr.makeZeroArrayIndex(); + NonLoc ZeroIdx = ValMgr.makeZeroArrayIndex(); return loc::MemRegionVal(MRMgr.getElementRegion(T, ZeroIdx, ArrayR, Ctx)); } @@ -828,14 +828,14 @@ SVal RegionStoreManager::EvalBinOp(BinaryOperator::Opcode Op, Loc L, NonLoc R, else EleTy = T->getAs()->getPointeeType(); - SVal ZeroIdx = ValMgr.makeZeroArrayIndex(); + const NonLoc &ZeroIdx = ValMgr.makeZeroArrayIndex(); ER = MRMgr.getElementRegion(EleTy, ZeroIdx, SR, Ctx); break; } case MemRegion::AllocaRegionKind: { const AllocaRegion *AR = cast(MR); QualType EleTy = Ctx.CharTy; // Create an ElementRegion of bytes. - SVal ZeroIdx = ValMgr.makeZeroArrayIndex(); + NonLoc ZeroIdx = ValMgr.makeZeroArrayIndex(); ER = MRMgr.getElementRegion(EleTy, ZeroIdx, AR, Ctx); break; } @@ -889,8 +889,12 @@ SVal RegionStoreManager::EvalBinOp(BinaryOperator::Opcode Op, Loc L, NonLoc R, SVal NewIdx = Base->evalBinOp(ValMgr, Op, cast(ValMgr.convertToArrayIndex(*Offset))); + + if (!isa(NewIdx)) + return UnknownVal(); + const MemRegion* NewER = - MRMgr.getElementRegion(ER->getElementType(), NewIdx, + MRMgr.getElementRegion(ER->getElementType(), cast(NewIdx), ER->getSuperRegion(), Ctx); return ValMgr.makeLoc(NewER); } @@ -1449,7 +1453,7 @@ Store RegionStoreManager::BindArray(Store store, const TypedRegion* R, if (VI == VE) break; - SVal Idx = ValMgr.makeArrayIndex(i); + const NonLoc &Idx = ValMgr.makeArrayIndex(i); const ElementRegion *ER = MRMgr.getElementRegion(ElementTy, Idx, R, Ctx); if (ElementTy->isStructureOrClassType()) diff --git a/clang/lib/Checker/SVals.cpp b/clang/lib/Checker/SVals.cpp index 97ba74e9487893bfdae7474afe3a40e8c4823d00..937b948fc9c733b529971f2eee6aaa4d6670201e 100644 --- a/clang/lib/Checker/SVals.cpp +++ b/clang/lib/Checker/SVals.cpp @@ -270,7 +270,7 @@ void SVal::dump() const { dumpToStream(llvm::errs()); } void SVal::dumpToStream(llvm::raw_ostream& os) const { switch (getBaseKind()) { case UnknownKind: - os << "Invalid"; + os << "Unknown"; break; case NonLocKind: cast(this)->dumpToStream(os); diff --git a/clang/lib/Checker/Store.cpp b/clang/lib/Checker/Store.cpp index 1cb5cd70cae6957731e05df71536a57178c9522b..aaa518edc8aecb120b0bb0dff12a23c6b694031a 100644 --- a/clang/lib/Checker/Store.cpp +++ b/clang/lib/Checker/Store.cpp @@ -28,7 +28,7 @@ Store StoreManager::EnterStackFrame(const GRState *state, const MemRegion *StoreManager::MakeElementRegion(const MemRegion *Base, QualType EleTy, uint64_t index) { - SVal idx = ValMgr.makeArrayIndex(index); + NonLoc idx = ValMgr.makeArrayIndex(index); return MRMgr.getElementRegion(EleTy, idx, Base, ValMgr.getContext()); } @@ -45,7 +45,7 @@ static bool IsCompleteType(ASTContext &Ctx, QualType Ty) { const ElementRegion *StoreManager::GetElementZeroRegion(const MemRegion *R, QualType T) { - SVal idx = ValMgr.makeZeroArrayIndex(); + NonLoc idx = ValMgr.makeZeroArrayIndex(); assert(!T.isNull()); return MRMgr.getElementRegion(T, idx, R, Ctx); } @@ -267,7 +267,7 @@ SVal StoreManager::getLValueFieldOrIvar(const Decl* D, SVal Base) { return loc::MemRegionVal(MRMgr.getFieldRegion(cast(D), BaseR)); } -SVal StoreManager::getLValueElement(QualType elementType, SVal Offset, +SVal StoreManager::getLValueElement(QualType elementType, NonLoc Offset, SVal Base) { // If the base is an unknown or undefined value, just return it back. @@ -283,7 +283,7 @@ SVal StoreManager::getLValueElement(QualType elementType, SVal Offset, const ElementRegion *ElemR = dyn_cast(BaseRegion); // Convert the offset to the appropriate size and signedness. - Offset = ValMgr.convertToArrayIndex(Offset); + Offset = cast(ValMgr.convertToArrayIndex(Offset)); if (!ElemR) { // @@ -322,8 +322,8 @@ SVal StoreManager::getLValueElement(QualType elementType, SVal Offset, assert(BaseIdxI.isSigned()); // Compute the new index. - SVal NewIdx = nonloc::ConcreteInt( - ValMgr.getBasicValueFactory().getValue(BaseIdxI + OffI)); + nonloc::ConcreteInt NewIdx(ValMgr.getBasicValueFactory().getValue(BaseIdxI + + OffI)); // Construct the new ElementRegion. const MemRegion *ArrayR = ElemR->getSuperRegion(); diff --git a/clang/test/Analysis/idempotent-operations.c b/clang/test/Analysis/idempotent-operations.c index d88bf49485eef9d28b1d6c92100763a4a611cda7..c673f0062f0e35d109138787fe73c61962e162ad 100644 --- a/clang/test/Analysis/idempotent-operations.c +++ b/clang/test/Analysis/idempotent-operations.c @@ -194,3 +194,33 @@ void false8() { a = (short)a; // no-warning test(a); } + +// This test case previously flagged a warning at 'b == c' because the +// analyzer previously allowed 'UnknownVal' as the index for ElementRegions. +typedef struct RDar8431728_F { + int RDar8431728_A; + unsigned char *RDar8431728_B; + int RDar8431728_E[6]; +} RDar8431728_D; +static inline int RDar8431728_C(RDar8431728_D * s, int n, + unsigned char **RDar8431728_B_ptr) { + int xy, wrap, pred, a, b, c; + + xy = s->RDar8431728_E[n]; + wrap = s->RDar8431728_A; + + a = s->RDar8431728_B[xy - 1]; + b = s->RDar8431728_B[xy - 1 - wrap]; + c = s->RDar8431728_B[xy - wrap]; + + if (b == c) { // no-warning + pred = a; + } else { + pred = c; + } + + *RDar8431728_B_ptr = &s->RDar8431728_B[xy]; + + return pred; +} +