From aace9ef279be3dadd53b481aee568bd7701178b4 Mon Sep 17 00:00:00 2001 From: Anna Zaks Date: Tue, 6 Dec 2011 23:12:27 +0000 Subject: [PATCH] [analyzer] Propagate taint through NonLoc to NonLoc casts. - Created a new SymExpr type - SymbolCast. - SymbolCast is created when we don't know how to simplify a NonLoc to NonLoc casts. - A bit of code refactoring: introduced dispatchCast to have better code reuse, remove a goto. - Updated the test case to showcase the new taint flow. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@145985 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Core/PathSensitive/SValBuilder.h | 9 +++- .../StaticAnalyzer/Core/PathSensitive/SVals.h | 1 + .../Core/PathSensitive/SymbolManager.h | 41 ++++++++++++++++++- lib/StaticAnalyzer/Core/ProgramState.cpp | 7 ++++ lib/StaticAnalyzer/Core/RegionStore.cpp | 2 +- lib/StaticAnalyzer/Core/SValBuilder.cpp | 31 ++++++-------- lib/StaticAnalyzer/Core/SVals.cpp | 5 +++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp | 8 ++++ lib/StaticAnalyzer/Core/Store.cpp | 10 ++--- lib/StaticAnalyzer/Core/SymbolManager.cpp | 22 ++++++++++ test/Analysis/taint-tester.c | 8 ++++ 11 files changed, 114 insertions(+), 30 deletions(-) diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h b/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h index b7b2345503..bbc5b660d5 100644 --- a/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h +++ b/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h @@ -48,11 +48,13 @@ protected: /// The width of the scalar type used for array indices. const unsigned ArrayIndexWidth; + virtual SVal evalCastFromNonLoc(NonLoc val, QualType castTy) = 0; + virtual SVal evalCastFromLoc(Loc val, QualType castTy) = 0; + public: // FIXME: Make these protected again once RegionStoreManager correctly // handles loads from different bound value types. - virtual SVal evalCastFromNonLoc(NonLoc val, QualType castTy) = 0; - virtual SVal evalCastFromLoc(Loc val, QualType castTy) = 0; + virtual SVal dispatchCast(SVal val, QualType castTy) = 0; public: SValBuilder(llvm::BumpPtrAllocator &alloc, ASTContext &context, @@ -243,6 +245,9 @@ public: NonLoc makeNonLoc(const SymExpr *lhs, BinaryOperator::Opcode op, const SymExpr *rhs, QualType type); + /// \brief Create a NonLoc value for cast. + NonLoc makeNonLoc(const SymExpr *operand, QualType fromTy, QualType toTy); + nonloc::ConcreteInt makeTruthVal(bool b, QualType type) { return nonloc::ConcreteInt(BasicVals.getTruthValue(b, type)); } diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h b/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h index 698a05adad..a6005ffd96 100644 --- a/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h +++ b/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h @@ -299,6 +299,7 @@ public: } }; +/// \brief Value representing integer constant. class ConcreteInt : public NonLoc { public: explicit ConcreteInt(const llvm::APSInt& V) : NonLoc(ConcreteIntKind, &V) {} diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h b/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h index e4bbf6766d..addc0d3988 100644 --- a/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h +++ b/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h @@ -48,7 +48,7 @@ public: MetadataKind, BEGIN_SYMBOLS = RegionValueKind, END_SYMBOLS = MetadataKind, - SymIntKind, SymSymKind }; + SymIntKind, SymSymKind, CastSymbolKind }; private: Kind K; @@ -276,6 +276,42 @@ public: } }; +/// \brief Represents a cast expression. +class SymbolCast : public SymExpr { + const SymExpr *Operand; + /// Type of the operand. + QualType FromTy; + /// The type of the result. + QualType ToTy; + +public: + SymbolCast(const SymExpr *In, QualType From, QualType To) : + SymExpr(CastSymbolKind), Operand(In), FromTy(From), ToTy(To) { } + + QualType getType(ASTContext &C) const { return ToTy; } + + const SymExpr *getOperand() const { return Operand; }; + + void dumpToStream(raw_ostream &os) const; + + static void Profile(llvm::FoldingSetNodeID& ID, + const SymExpr *In, QualType From, QualType To) { + ID.AddInteger((unsigned) CastSymbolKind); + ID.AddPointer(In); + ID.Add(From); + ID.Add(To); + } + + void Profile(llvm::FoldingSetNodeID& ID) { + Profile(ID, Operand, FromTy, ToTy); + } + + // Implement isa support. + static inline bool classof(const SymExpr *SE) { + return SE->getKind() == CastSymbolKind; + } +}; + /// SymIntExpr - Represents symbolic expression like 'x' + 3. class SymIntExpr : public SymExpr { const SymExpr *LHS; @@ -408,6 +444,9 @@ public: QualType T, unsigned VisitCount, const void *SymbolTag = 0); + const SymbolCast* getCastSymbol(const SymExpr *Operand, + QualType From, QualType To); + const SymIntExpr *getSymIntExpr(const SymExpr *lhs, BinaryOperator::Opcode op, const llvm::APSInt& rhs, QualType t); diff --git a/lib/StaticAnalyzer/Core/ProgramState.cpp b/lib/StaticAnalyzer/Core/ProgramState.cpp index 79f4348b7c..2dafeeee00 100644 --- a/lib/StaticAnalyzer/Core/ProgramState.cpp +++ b/lib/StaticAnalyzer/Core/ProgramState.cpp @@ -560,6 +560,8 @@ bool ScanReachableSymbols::scan(const SymExpr *sym) { case SymExpr::ExtentKind: case SymExpr::MetadataKind: break; + case SymExpr::CastSymbolKind: + return scan(cast(sym)->getOperand()); case SymExpr::SymIntKind: return scan(cast(sym)->getLHS()); case SymExpr::SymSymKind: { @@ -672,10 +674,15 @@ bool ProgramState::isTainted(const SymExpr* Sym, TaintTagType Kind) const { if (!Sym) return false; + // TODO: Can we use symbol_iterator (like removeDeadBindingsWorker) here? + // Check taint on derived symbols. if (const SymbolDerived *SD = dyn_cast(Sym)) return isTainted(SD->getParentSymbol(), Kind); + if (const SymbolCast *SC = dyn_cast(Sym)) + return (isTainted(SC->getOperand(), Kind)); + if (const SymIntExpr *SIE = dyn_cast(Sym)) return isTainted(SIE->getLHS(), Kind); diff --git a/lib/StaticAnalyzer/Core/RegionStore.cpp b/lib/StaticAnalyzer/Core/RegionStore.cpp index 27077afada..4f811dfa73 100644 --- a/lib/StaticAnalyzer/Core/RegionStore.cpp +++ b/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -1720,7 +1720,7 @@ void removeDeadBindingsWorker::VisitBinding(SVal V) { if (const MemRegion *R = V.getAsRegion()) AddToWorkList(R); - // Update the set of live symbols. + // Update the set of live symbols. for (SVal::symbol_iterator SI=V.symbol_begin(), SE=V.symbol_end(); SI!=SE;++SI) SymReaper.markLive(*SI); diff --git a/lib/StaticAnalyzer/Core/SValBuilder.cpp b/lib/StaticAnalyzer/Core/SValBuilder.cpp index 617e0ddc72..ff0a85a572 100644 --- a/lib/StaticAnalyzer/Core/SValBuilder.cpp +++ b/lib/StaticAnalyzer/Core/SValBuilder.cpp @@ -56,6 +56,12 @@ NonLoc SValBuilder::makeNonLoc(const SymExpr *lhs, BinaryOperator::Opcode op, return nonloc::SymbolVal(SymMgr.getSymSymExpr(lhs, op, rhs, type)); } +NonLoc SValBuilder::makeNonLoc(const SymExpr *operand, + QualType fromTy, QualType toTy) { + assert(operand); + assert(!Loc::isLocType(toTy)); + return nonloc::SymbolVal(SymMgr.getCastSymbol(operand, fromTy, toTy)); +} SVal SValBuilder::convertToArrayIndex(SVal val) { if (val.isUnknownOrUndef()) @@ -220,21 +226,7 @@ SVal SValBuilder::evalCast(SVal val, QualType castTy, QualType originalTy) { if (!castTy->isVariableArrayType() && !originalTy->isVariableArrayType()) if (Context.hasSameUnqualifiedType(castTy, originalTy)) return val; - - // Check for casts to real or complex numbers. We don't handle these at all - // right now. - if (castTy->isFloatingType() || castTy->isAnyComplexType()) - return UnknownVal(); - // Check for casts from integers to integers. - if (castTy->isIntegerType() && originalTy->isIntegerType()) { - if (isa(val)) - // This can be a cast to ObjC property of type int. - return evalCastFromLoc(cast(val), castTy); - else - return evalCastFromNonLoc(cast(val), castTy); - } - // Check for casts from pointers to integers. if (castTy->isIntegerType() && Loc::isLocType(originalTy)) return evalCastFromLoc(cast(val), castTy); @@ -249,7 +241,7 @@ SVal SValBuilder::evalCast(SVal val, QualType castTy, QualType originalTy) { } return LV->getLoc(); } - goto DispatchCast; + return dispatchCast(val, castTy); } // Just pass through function and block pointers. @@ -323,8 +315,9 @@ SVal SValBuilder::evalCast(SVal val, QualType castTy, QualType originalTy) { return R ? SVal(loc::MemRegionVal(R)) : UnknownVal(); } -DispatchCast: - // All other cases. - return isa(val) ? evalCastFromLoc(cast(val), castTy) - : evalCastFromNonLoc(cast(val), castTy); + // Check for casts from integers to integers. + if (castTy->isIntegerType() && originalTy->isIntegerType()) + return dispatchCast(val, castTy); + + return dispatchCast(val, castTy); } diff --git a/lib/StaticAnalyzer/Core/SVals.cpp b/lib/StaticAnalyzer/Core/SVals.cpp index 4bdea1d9e1..97e5a1be4c 100644 --- a/lib/StaticAnalyzer/Core/SVals.cpp +++ b/lib/StaticAnalyzer/Core/SVals.cpp @@ -91,6 +91,7 @@ SymbolRef SVal::getLocSymbolInBase() const { return 0; } +// TODO: The next 3 functions have to be simplified. /// getAsSymbol - If this Sval wraps a symbol return that SymbolRef. /// Otherwise return 0. // FIXME: should we consider SymbolRef wrapped in CodeTextRegion? @@ -168,6 +169,10 @@ void SVal::symbol_iterator::expand() { const SymExpr *SE = itr.back(); itr.pop_back(); + if (const SymbolCast *SC = dyn_cast(SE)) { + itr.push_back(SC->getOperand()); + return; + } if (const SymIntExpr *SIE = dyn_cast(SE)) { itr.push_back(SIE->getLHS()); return; diff --git a/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp b/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp index 495f6f20b6..da07d8ad0f 100644 --- a/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp +++ b/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp @@ -20,6 +20,7 @@ using namespace ento; namespace { class SimpleSValBuilder : public SValBuilder { protected: + virtual SVal dispatchCast(SVal val, QualType castTy); virtual SVal evalCastFromNonLoc(NonLoc val, QualType castTy); virtual SVal evalCastFromLoc(Loc val, QualType castTy); @@ -57,6 +58,11 @@ SValBuilder *ento::createSimpleSValBuilder(llvm::BumpPtrAllocator &alloc, // Transfer function for Casts. //===----------------------------------------------------------------------===// +SVal SimpleSValBuilder::dispatchCast(SVal val, QualType castTy) { + return isa(val) ? evalCastFromLoc(cast(val), castTy) + : evalCastFromNonLoc(cast(val), castTy); +} + SVal SimpleSValBuilder::evalCastFromNonLoc(NonLoc val, QualType castTy) { bool isLocType = Loc::isLocType(castTy); @@ -86,6 +92,8 @@ SVal SimpleSValBuilder::evalCastFromNonLoc(NonLoc val, QualType castTy) { if (T->isIntegerType() && castTy->isIntegerType()) return val; + if (!isLocType) + return makeNonLoc(se, T, castTy); return UnknownVal(); } diff --git a/lib/StaticAnalyzer/Core/Store.cpp b/lib/StaticAnalyzer/Core/Store.cpp index 48a6f4f60f..5d152d4d53 100644 --- a/lib/StaticAnalyzer/Core/Store.cpp +++ b/lib/StaticAnalyzer/Core/Store.cpp @@ -212,7 +212,7 @@ const MemRegion *StoreManager::castRegion(const MemRegion *R, QualType CastToTy) SVal StoreManager::CastRetrievedVal(SVal V, const TypedValueRegion *R, QualType castTy, bool performTestOnly) { - if (castTy.isNull()) + if (castTy.isNull() || V.isUnknownOrUndef()) return V; ASTContext &Ctx = svalBuilder.getContext(); @@ -227,12 +227,8 @@ SVal StoreManager::CastRetrievedVal(SVal V, const TypedValueRegion *R, return V; } - if (const Loc *L = dyn_cast(&V)) - return svalBuilder.evalCastFromLoc(*L, castTy); - else if (const NonLoc *NL = dyn_cast(&V)) - return svalBuilder.evalCastFromNonLoc(*NL, castTy); - - return V; + assert(isa(&V) || isa(&V)); + return svalBuilder.dispatchCast(V, castTy); } SVal StoreManager::getLValueFieldOrIvar(const Decl *D, SVal Base) { diff --git a/lib/StaticAnalyzer/Core/SymbolManager.cpp b/lib/StaticAnalyzer/Core/SymbolManager.cpp index b843ab1a90..02c0a9e36d 100644 --- a/lib/StaticAnalyzer/Core/SymbolManager.cpp +++ b/lib/StaticAnalyzer/Core/SymbolManager.cpp @@ -66,6 +66,12 @@ void SymSymExpr::dumpToStream(raw_ostream &os) const { os << ')'; } +void SymbolCast::dumpToStream(raw_ostream &os) const { + os << '(' << ToTy.getAsString() << ") ("; + Operand->dumpToStream(os); + os << ')'; +} + void SymbolConjured::dumpToStream(raw_ostream &os) const { os << "conj_$" << getSymbolID() << '{' << T.getAsString() << '}'; } @@ -174,6 +180,22 @@ SymbolManager::getMetadataSymbol(const MemRegion* R, const Stmt *S, QualType T, return cast(SD); } +const SymbolCast* +SymbolManager::getCastSymbol(const SymExpr *Op, + QualType From, QualType To) { + llvm::FoldingSetNodeID ID; + SymbolCast::Profile(ID, Op, From, To); + void *InsertPos; + SymExpr *data = DataSet.FindNodeOrInsertPos(ID, InsertPos); + if (!data) { + data = (SymbolCast*) BPAlloc.Allocate(); + new (data) SymbolCast(Op, From, To); + DataSet.InsertNode(data, InsertPos); + } + + return cast(data); +} + const SymIntExpr *SymbolManager::getSymIntExpr(const SymExpr *lhs, BinaryOperator::Opcode op, const llvm::APSInt& v, diff --git a/test/Analysis/taint-tester.c b/test/Analysis/taint-tester.c index b424d5d5b0..eb05f577d1 100644 --- a/test/Analysis/taint-tester.c +++ b/test/Analysis/taint-tester.c @@ -12,4 +12,12 @@ void bufferScanfAssignment(int x) { scanf("%d", &n); addr += n;// expected-warning {{tainted}} *addr = n; // expected-warning 2 {{tainted}} + + double tdiv = n / 30; // expected-warning 3 {{tainted}} + char *loc_cast = (char *) n; // expected-warning {{tainted}} + char tinc = tdiv++; // expected-warning {{tainted}} + int tincdec = (char)tinc--; // expected-warning 2 {{tainted}} + int tprtarithmetic1 = *(addr+1); + + } -- 2.40.0