NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, mikhail.ramalho, Szelethus, baloghadamsoftware, Charusso. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, a.sidorin, szepet. Herald added a project: clang.
This is mostly motivated by me being annoyed that in C many functions are implemented as builtins and everybody keeps forgetting about it while writing checkers, leading to checkers not working at all except on LIT tests, which is fairly hard to notice. This is also a necessary step if we want to `evalCall()` calls that don't correspond to any `CallExpr`, such as automatic destructor calls. Repository: rC Clang https://reviews.llvm.org/D62440 Files: clang/include/clang/StaticAnalyzer/Core/Checker.h clang/include/clang/StaticAnalyzer/Core/CheckerManager.h clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp clang/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
Index: clang/lib/StaticAnalyzer/Core/CheckerManager.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/CheckerManager.cpp +++ clang/lib/StaticAnalyzer/Core/CheckerManager.cpp @@ -650,7 +650,6 @@ const ExplodedNodeSet &Src, const CallEvent &Call, ExprEngine &Eng) { - const CallExpr *CE = cast<CallExpr>(Call.getOriginExpr()); for (const auto Pred : Src) { bool anyEvaluated = false; @@ -659,16 +658,19 @@ // Check if any of the EvalCall callbacks can evaluate the call. for (const auto EvalCallChecker : EvalCallCheckers) { - ProgramPoint::Kind K = ProgramPoint::PostStmtKind; - const ProgramPoint &L = - ProgramPoint::getProgramPoint(CE, K, Pred->getLocationContext(), - EvalCallChecker.Checker); + // TODO: Support the situation when the call doesn't correspond + // to any Expr. + ProgramPoint L = ProgramPoint::getProgramPoint( + cast<CallExpr>(Call.getOriginExpr()), + ProgramPoint::PostStmtKind, + Pred->getLocationContext(), + EvalCallChecker.Checker); bool evaluated = false; { // CheckerContext generates transitions(populates checkDest) on // destruction, so introduce the scope to make sure it gets properly // populated. CheckerContext C(B, Eng, Pred, L); - evaluated = EvalCallChecker(CE, C); + evaluated = EvalCallChecker(Call, C); } assert(!(evaluated && anyEvaluated) && "There are more than one checkers evaluating the call"); Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -14,6 +14,7 @@ #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" @@ -71,7 +72,7 @@ II_fsetpos(nullptr), II_clearerr(nullptr), II_feof(nullptr), II_ferror(nullptr), II_fileno(nullptr) {} - bool evalCall(const CallExpr *CE, CheckerContext &C) const; + bool evalCall(const CallEvent &Call, CheckerContext &C) const; void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const; private: @@ -103,11 +104,15 @@ REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState) -bool StreamChecker::evalCall(const CallExpr *CE, CheckerContext &C) const { - const FunctionDecl *FD = C.getCalleeDecl(CE); +bool StreamChecker::evalCall(const CallEvent &Call, CheckerContext &C) const { + const auto *FD = dyn_cast_or_null<FunctionDecl>(Call.getDecl()); if (!FD || FD->getKind() != Decl::Function) return false; + const auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr()); + if (!CE) + return false; + ASTContext &Ctx = C.getASTContext(); if (!II_fopen) II_fopen = &Ctx.Idents.get("fopen"); Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp @@ -224,7 +224,7 @@ public: void checkPostCall(const CallEvent &Call, CheckerContext &C) const; - bool evalCall(const CallExpr *CE, CheckerContext &C) const; + bool evalCall(const CallEvent &Call, CheckerContext &C) const; private: Optional<FunctionSummaryTy> findFunctionSummary(const FunctionDecl *FD, @@ -367,12 +367,16 @@ } } -bool StdLibraryFunctionsChecker::evalCall(const CallExpr *CE, +bool StdLibraryFunctionsChecker::evalCall(const CallEvent &Call, CheckerContext &C) const { - const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(CE->getCalleeDecl()); + const auto *FD = dyn_cast_or_null<FunctionDecl>(Call.getDecl()); if (!FD) return false; + const auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr()); + if (!CE) + return false; + Optional<FunctionSummaryTy> FoundSummary = findFunctionSummary(FD, CE, C); if (!FoundSummary) return false; Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp +++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp @@ -26,32 +26,30 @@ namespace { class SmartPtrModeling : public Checker<eval::Call> { - bool isNullAfterMoveMethod(const CXXInstanceCall *Call) const; + bool isNullAfterMoveMethod(const CallEvent &Call) const; public: - bool evalCall(const CallExpr *CE, CheckerContext &C) const; + bool evalCall(const CallEvent &Call, CheckerContext &C) const; }; } // end of anonymous namespace -bool SmartPtrModeling::isNullAfterMoveMethod( - const CXXInstanceCall *Call) const { +bool SmartPtrModeling::isNullAfterMoveMethod(const CallEvent &Call) const { // TODO: Update CallDescription to support anonymous calls? // TODO: Handle other methods, such as .get() or .release(). // But once we do, we'd need a visitor to explain null dereferences // that are found via such modeling. - const auto *CD = dyn_cast_or_null<CXXConversionDecl>(Call->getDecl()); + const auto *CD = dyn_cast_or_null<CXXConversionDecl>(Call.getDecl()); return CD && CD->getConversionType()->isBooleanType(); } -bool SmartPtrModeling::evalCall(const CallExpr *CE, CheckerContext &C) const { - CallEventRef<> CallRef = C.getStateManager().getCallEventManager().getCall( - CE, C.getState(), C.getLocationContext()); - const auto *Call = dyn_cast_or_null<CXXInstanceCall>(CallRef); - if (!Call || !isNullAfterMoveMethod(Call)) +bool SmartPtrModeling::evalCall(const CallEvent &Call, + CheckerContext &C) const { + if (!isNullAfterMoveMethod(Call)) return false; ProgramStateRef State = C.getState(); - const MemRegion *ThisR = Call->getCXXThisVal().getAsRegion(); + const MemRegion *ThisR = + cast<CXXInstanceCall>(&Call)->getCXXThisVal().getAsRegion(); if (!move::isMovedFrom(State, ThisR)) { // TODO: Model this case as well. At least, avoid invalidation of globals. @@ -60,8 +58,8 @@ // TODO: Add a note to bug reports describing this decision. C.addTransition( - State->BindExpr(CE, C.getLocationContext(), - C.getSValBuilder().makeZeroVal(CE->getType()))); + State->BindExpr(Call.getOriginExpr(), C.getLocationContext(), + C.getSValBuilder().makeZeroVal(Call.getResultType()))); return true; } Index: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h =================================================================== --- clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h +++ clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h @@ -310,7 +310,7 @@ const CallEvent &Call, CheckerContext &C) const; - bool evalCall(const CallExpr *CE, CheckerContext &C) const; + bool evalCall(const CallEvent &Call, CheckerContext &C) const; ProgramStateRef evalAssume(ProgramStateRef state, SVal Cond, bool Assumption) const; Index: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp @@ -886,14 +886,19 @@ // Handle the return values of retain-count-related functions. //===----------------------------------------------------------------------===// -bool RetainCountChecker::evalCall(const CallExpr *CE, CheckerContext &C) const { +bool RetainCountChecker::evalCall(const CallEvent &Call, + CheckerContext &C) const { ProgramStateRef state = C.getState(); - const FunctionDecl *FD = C.getCalleeDecl(CE); + const auto *FD = dyn_cast_or_null<FunctionDecl>(Call.getDecl()); if (!FD) return false; + const auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr()); + if (!CE) + return false; + RetainSummaryManager &SmrMgr = getSummaryManager(C); - QualType ResultTy = CE->getCallReturnType(C.getASTContext()); + QualType ResultTy = Call.getResultType(); // See if the function has 'rc_ownership_trusted_implementation' // annotate attribute. If it does, we will not inline it. Index: clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp @@ -11,6 +11,7 @@ #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/IssueHash.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "llvm/ADT/StringSwitch.h" #include "llvm/Support/ScopedPrinter.h" @@ -53,7 +54,7 @@ ExplodedNode *N) const; public: - bool evalCall(const CallExpr *CE, CheckerContext &C) const; + bool evalCall(const CallEvent &Call, CheckerContext &C) const; void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const; void checkEndAnalysis(ExplodedGraph &G, BugReporter &BR, ExprEngine &Eng) const; @@ -63,8 +64,12 @@ REGISTER_SET_WITH_PROGRAMSTATE(MarkedSymbols, SymbolRef) REGISTER_MAP_WITH_PROGRAMSTATE(DenotedSymbols, SymbolRef, const StringLiteral *) -bool ExprInspectionChecker::evalCall(const CallExpr *CE, +bool ExprInspectionChecker::evalCall(const CallEvent &Call, CheckerContext &C) const { + const auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr()); + if (!CE) + return false; + // These checks should have no effect on the surrounding environment // (globals should not be invalidated, etc), hence the use of evalCall. FnCheck Handler = llvm::StringSwitch<FnCheck>(C.getCalleeName(CE)) Index: clang/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp @@ -14,6 +14,7 @@ #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" @@ -37,53 +38,44 @@ // ROOT_CHANGED<--chdir(..)-- JAIL_ENTERED<--chdir(..)-- // | | // bug<--foo()-- JAIL_ENTERED<--foo()-- -class ChrootChecker : public Checker<eval::Call, check::PreStmt<CallExpr> > { - mutable IdentifierInfo *II_chroot, *II_chdir; +class ChrootChecker : public Checker<eval::Call, check::PreCall> { // This bug refers to possibly break out of a chroot() jail. mutable std::unique_ptr<BuiltinBug> BT_BreakJail; + const CallDescription Chroot{"chroot", 1}, Chdir{"chdir", 1}; + public: - ChrootChecker() : II_chroot(nullptr), II_chdir(nullptr) {} + ChrootChecker() {} static void *getTag() { static int x; return &x; } - bool evalCall(const CallExpr *CE, CheckerContext &C) const; - void checkPreStmt(const CallExpr *CE, CheckerContext &C) const; + bool evalCall(const CallEvent &Call, CheckerContext &C) const; + void checkPreCall(const CallEvent &Call, CheckerContext &C) const; private: - void Chroot(CheckerContext &C, const CallExpr *CE) const; - void Chdir(CheckerContext &C, const CallExpr *CE) const; + void evalChroot(const CallEvent &Call, CheckerContext &C) const; + void evalChdir(const CallEvent &Call, CheckerContext &C) const; }; } // end anonymous namespace -bool ChrootChecker::evalCall(const CallExpr *CE, CheckerContext &C) const { - const FunctionDecl *FD = C.getCalleeDecl(CE); - if (!FD) - return false; - - ASTContext &Ctx = C.getASTContext(); - if (!II_chroot) - II_chroot = &Ctx.Idents.get("chroot"); - if (!II_chdir) - II_chdir = &Ctx.Idents.get("chdir"); - - if (FD->getIdentifier() == II_chroot) { - Chroot(C, CE); +bool ChrootChecker::evalCall(const CallEvent &Call, CheckerContext &C) const { + if (Call.isCalled(Chroot)) { + evalChroot(Call, C); return true; } - if (FD->getIdentifier() == II_chdir) { - Chdir(C, CE); + if (Call.isCalled(Chdir)) { + evalChdir(Call, C); return true; } return false; } -void ChrootChecker::Chroot(CheckerContext &C, const CallExpr *CE) const { +void ChrootChecker::evalChroot(const CallEvent &Call, CheckerContext &C) const { ProgramStateRef state = C.getState(); ProgramStateManager &Mgr = state->getStateManager(); @@ -93,7 +85,7 @@ C.addTransition(state); } -void ChrootChecker::Chdir(CheckerContext &C, const CallExpr *CE) const { +void ChrootChecker::evalChdir(const CallEvent &Call, CheckerContext &C) const { ProgramStateRef state = C.getState(); ProgramStateManager &Mgr = state->getStateManager(); @@ -103,7 +95,7 @@ return; // After chdir("/"), enter the jail, set the enum value JAIL_ENTERED. - const Expr *ArgExpr = CE->getArg(0); + const Expr *ArgExpr = Call.getArgExpr(0); SVal ArgVal = C.getSVal(ArgExpr); if (const MemRegion *R = ArgVal.getAsRegion()) { @@ -120,19 +112,10 @@ } // Check the jail state before any function call except chroot and chdir(). -void ChrootChecker::checkPreStmt(const CallExpr *CE, CheckerContext &C) const { - const FunctionDecl *FD = C.getCalleeDecl(CE); - if (!FD) - return; - - ASTContext &Ctx = C.getASTContext(); - if (!II_chroot) - II_chroot = &Ctx.Idents.get("chroot"); - if (!II_chdir) - II_chdir = &Ctx.Idents.get("chdir"); - +void ChrootChecker::checkPreCall(const CallEvent &Call, + CheckerContext &C) const { // Ignore chroot and chdir. - if (FD->getIdentifier() == II_chroot || FD->getIdentifier() == II_chdir) + if (Call.isCalled(Chroot) || Call.isCalled(Chdir)) return; // If jail state is ROOT_CHANGED, generate BugReport. Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -17,6 +17,7 @@ #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" #include "llvm/ADT/STLExtras.h" @@ -57,7 +58,7 @@ static void *getTag() { static int tag; return &tag; } - bool evalCall(const CallExpr *CE, CheckerContext &C) const; + bool evalCall(const CallEvent &Call, CheckerContext &C) const; void checkPreStmt(const DeclStmt *DS, CheckerContext &C) const; void checkLiveSymbols(ProgramStateRef state, SymbolReaper &SR) const; void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const; @@ -2334,8 +2335,8 @@ return nullptr; } -bool CStringChecker::evalCall(const CallExpr *CE, CheckerContext &C) const { - +bool CStringChecker::evalCall(const CallEvent &Call, CheckerContext &C) const { + const auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr()); FnCheck evalFunction = identifyCall(CE, C); // If the callee isn't a string function, let another checker handle it. Index: clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp @@ -14,6 +14,7 @@ #include "clang/Basic/Builtins.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" using namespace clang; @@ -23,30 +24,32 @@ class BuiltinFunctionChecker : public Checker<eval::Call> { public: - bool evalCall(const CallExpr *CE, CheckerContext &C) const; + bool evalCall(const CallEvent &Call, CheckerContext &C) const; }; } -bool BuiltinFunctionChecker::evalCall(const CallExpr *CE, +bool BuiltinFunctionChecker::evalCall(const CallEvent &Call, CheckerContext &C) const { ProgramStateRef state = C.getState(); - const FunctionDecl *FD = C.getCalleeDecl(CE); - const LocationContext *LCtx = C.getLocationContext(); + const auto *FD = dyn_cast_or_null<FunctionDecl>(Call.getDecl()); if (!FD) return false; + const LocationContext *LCtx = C.getLocationContext(); + const Expr *CE = Call.getOriginExpr(); + switch (FD->getBuiltinID()) { default: return false; case Builtin::BI__builtin_assume: { - assert (CE->arg_begin() != CE->arg_end()); - SVal ArgSVal = C.getSVal(CE->getArg(0)); - if (ArgSVal.isUndef()) + assert (Call.getNumArgs() > 0); + SVal Arg = Call.getArgSVal(0); + if (Arg.isUndef()) return true; // Return true to model purity. - state = state->assume(ArgSVal.castAs<DefinedOrUnknownSVal>(), true); + state = state->assume(Arg.castAs<DefinedOrUnknownSVal>(), true); // FIXME: do we want to warn here? Not right now. The most reports might // come from infeasible paths, thus being false positives. if (!state) { @@ -66,9 +69,9 @@ // __builtin_assume_aligned, just return the value of the subexpression. // __builtin_addressof is going from a reference to a pointer, but those // are represented the same way in the analyzer. - assert (CE->arg_begin() != CE->arg_end()); - SVal X = C.getSVal(*(CE->arg_begin())); - C.addTransition(state->BindExpr(CE, LCtx, X)); + assert (Call.getNumArgs() > 0); + SVal Arg = Call.getArgSVal(0); + C.addTransition(state->BindExpr(CE, LCtx, Arg)); return true; } @@ -82,12 +85,14 @@ // Set the extent of the region in bytes. This enables us to use the // SVal of the argument directly. If we save the extent in bits, we // cannot represent values like symbol*8. - auto Size = C.getSVal(*(CE->arg_begin())).castAs<DefinedOrUnknownSVal>(); + auto Size = Call.getArgSVal(0); + if (Size.isUndef()) + return true; // Return true to model purity. SValBuilder& svalBuilder = C.getSValBuilder(); DefinedOrUnknownSVal Extent = R->getExtent(svalBuilder); DefinedOrUnknownSVal extentMatchesSizeArg = - svalBuilder.evalEQ(state, Extent, Size); + svalBuilder.evalEQ(state, Extent, Size.castAs<DefinedOrUnknownSVal>()); state = state->assume(extentMatchesSizeArg, true); assert(state && "The region should not have any previous constraints"); Index: clang/include/clang/StaticAnalyzer/Core/CheckerManager.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/CheckerManager.h +++ clang/include/clang/StaticAnalyzer/Core/CheckerManager.h @@ -486,7 +486,7 @@ CheckerFn<ProgramStateRef (ProgramStateRef, const SVal &cond, bool assumption)>; - using EvalCallFunc = CheckerFn<bool (const CallExpr *, CheckerContext &)>; + using EvalCallFunc = CheckerFn<bool (const CallEvent &, CheckerContext &)>; using CheckEndOfTranslationUnit = CheckerFn<void (const TranslationUnitDecl *, AnalysisManager &, Index: clang/include/clang/StaticAnalyzer/Core/Checker.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/Checker.h +++ clang/include/clang/StaticAnalyzer/Core/Checker.h @@ -474,8 +474,9 @@ class Call { template <typename CHECKER> - static bool _evalCall(void *checker, const CallExpr *CE, CheckerContext &C) { - return ((const CHECKER *)checker)->evalCall(CE, C); + static bool _evalCall(void *checker, const CallEvent &Call, + CheckerContext &C) { + return ((const CHECKER *)checker)->evalCall(Call, C); } public:
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits