diff options
| author | Dimitry Andric <dim@FreeBSD.org> | 2021-11-19 20:06:13 +0000 |
|---|---|---|
| committer | Dimitry Andric <dim@FreeBSD.org> | 2021-11-19 20:06:13 +0000 |
| commit | c0981da47d5696fe36474fcf86b4ce03ae3ff818 (patch) | |
| tree | f42add1021b9f2ac6a69ac7cf6c4499962739a45 /clang/lib/StaticAnalyzer/Checkers | |
| parent | 344a3780b2e33f6ca763666c380202b18aab72a3 (diff) | |
Diffstat (limited to 'clang/lib/StaticAnalyzer/Checkers')
46 files changed, 877 insertions, 238 deletions
diff --git a/clang/lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp index c06604b6cffe..6154eeb3419c 100644 --- a/clang/lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp @@ -93,11 +93,10 @@ void AnalyzerStatsChecker::checkEndAnalysis(ExplodedGraph &G, if (!Loc.isValid()) return; - if (isa<FunctionDecl>(D) || isa<ObjCMethodDecl>(D)) { + if (isa<FunctionDecl, ObjCMethodDecl>(D)) { const NamedDecl *ND = cast<NamedDecl>(D); output << *ND; - } - else if (isa<BlockDecl>(D)) { + } else if (isa<BlockDecl>(D)) { output << "block(line:" << Loc.getLine() << ":col:" << Loc.getColumn(); } diff --git a/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp b/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp index a86a410ebcbc..2c210fb6cdb9 100644 --- a/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp @@ -12,7 +12,6 @@ // //===----------------------------------------------------------------------===// -#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/AST/ASTContext.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/Expr.h" @@ -20,9 +19,11 @@ #include "clang/AST/StmtObjC.h" #include "clang/Analysis/DomainSpecific/CocoaConventions.h" #include "clang/Analysis/SelectorExtras.h" +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h" @@ -533,10 +534,12 @@ void CFNumberChecker::checkPreStmt(const CallExpr *CE, namespace { class CFRetainReleaseChecker : public Checker<check::PreCall> { mutable APIMisuse BT{this, "null passed to CF memory management function"}; - CallDescription CFRetain{"CFRetain", 1}, - CFRelease{"CFRelease", 1}, - CFMakeCollectable{"CFMakeCollectable", 1}, - CFAutorelease{"CFAutorelease", 1}; + const CallDescriptionSet ModelledCalls = { + {"CFRetain", 1}, + {"CFRelease", 1}, + {"CFMakeCollectable", 1}, + {"CFAutorelease", 1}, + }; public: void checkPreCall(const CallEvent &Call, CheckerContext &C) const; @@ -550,8 +553,7 @@ void CFRetainReleaseChecker::checkPreCall(const CallEvent &Call, return; // Check if we called CFRetain/CFRelease/CFMakeCollectable/CFAutorelease. - if (!(Call.isCalled(CFRetain) || Call.isCalled(CFRelease) || - Call.isCalled(CFMakeCollectable) || Call.isCalled(CFAutorelease))) + if (!ModelledCalls.contains(Call)) return; // Get the argument's value. diff --git a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp index 2752b37f9b3f..8416ab39e194 100644 --- a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp @@ -17,6 +17,7 @@ #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" @@ -96,14 +97,7 @@ void BlockInCriticalSectionChecker::initIdentifierInfo(ASTContext &Ctx) const { } bool BlockInCriticalSectionChecker::isBlockingFunction(const CallEvent &Call) const { - if (Call.isCalled(SleepFn) - || Call.isCalled(GetcFn) - || Call.isCalled(FgetsFn) - || Call.isCalled(ReadFn) - || Call.isCalled(RecvFn)) { - return true; - } - return false; + return matchesAny(Call, SleepFn, GetcFn, FgetsFn, ReadFn, RecvFn); } bool BlockInCriticalSectionChecker::isLockFunction(const CallEvent &Call) const { @@ -113,15 +107,8 @@ bool BlockInCriticalSectionChecker::isLockFunction(const CallEvent &Call) const return true; } - if (Call.isCalled(LockFn) - || Call.isCalled(PthreadLockFn) - || Call.isCalled(PthreadTryLockFn) - || Call.isCalled(MtxLock) - || Call.isCalled(MtxTimedLock) - || Call.isCalled(MtxTryLock)) { - return true; - } - return false; + return matchesAny(Call, LockFn, PthreadLockFn, PthreadTryLockFn, MtxLock, + MtxTimedLock, MtxTryLock); } bool BlockInCriticalSectionChecker::isUnlockFunction(const CallEvent &Call) const { @@ -132,12 +119,7 @@ bool BlockInCriticalSectionChecker::isUnlockFunction(const CallEvent &Call) cons return true; } - if (Call.isCalled(UnlockFn) - || Call.isCalled(PthreadUnlockFn) - || Call.isCalled(MtxUnlock)) { - return true; - } - return false; + return matchesAny(Call, UnlockFn, PthreadUnlockFn, MtxUnlock); } void BlockInCriticalSectionChecker::checkPostCall(const CallEvent &Call, diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index 69b90be9aa7e..475cee9ce04b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/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/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h" @@ -2271,11 +2272,10 @@ CStringChecker::FnCheck CStringChecker::identifyCall(const CallEvent &Call, if (!FD) return nullptr; - if (Call.isCalled(StdCopy)) { + if (StdCopy.matches(Call)) return &CStringChecker::evalStdCopy; - } else if (Call.isCalled(StdCopyBackward)) { + if (StdCopyBackward.matches(Call)) return &CStringChecker::evalStdCopyBackward; - } // Pro-actively check that argument types are safe to do arithmetic upon. // We do not want to crash if someone accidentally passes a structure diff --git a/clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp index 131c1345af99..4235c0c13821 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp @@ -20,6 +20,7 @@ #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicType.h" diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckObjCInstMethSignature.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckObjCInstMethSignature.cpp index 175dfcef0df4..a13de306eac8 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CheckObjCInstMethSignature.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CheckObjCInstMethSignature.cpp @@ -6,7 +6,7 @@ // //===----------------------------------------------------------------------===// // -// This file defines a CheckObjCInstMethSignature, a flow-insenstive check +// This file defines a CheckObjCInstMethSignature, a flow-insensitive check // that determines if an Objective-C class interface incorrectly redefines // the method signature in a subclass. // diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp index d06c87631bfb..61ff5e59f06d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp @@ -785,9 +785,8 @@ void WalkAST::checkDeprecatedOrUnsafeBufferHandling(const CallExpr *CE, // real flow analysis. auto FormatString = dyn_cast<StringLiteral>(CE->getArg(ArgIndex)->IgnoreParenImpCasts()); - if (FormatString && - FormatString->getString().find("%s") == StringRef::npos && - FormatString->getString().find("%[") == StringRef::npos) + if (FormatString && !FormatString->getString().contains("%s") && + !FormatString->getString().contains("%[")) BoundsProvided = true; } diff --git a/clang/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp index fd53c04f4bbf..ce8d6c879870 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp +++ b/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/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h" @@ -63,11 +64,11 @@ private: } // end anonymous namespace bool ChrootChecker::evalCall(const CallEvent &Call, CheckerContext &C) const { - if (Call.isCalled(Chroot)) { + if (Chroot.matches(Call)) { evalChroot(Call, C); return true; } - if (Call.isCalled(Chdir)) { + if (Chdir.matches(Call)) { evalChdir(Call, C); return true; } @@ -115,7 +116,7 @@ void ChrootChecker::evalChdir(const CallEvent &Call, CheckerContext &C) const { void ChrootChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { // Ignore chroot and chdir. - if (Call.isCalled(Chroot) || Call.isCalled(Chdir)) + if (matchesAny(Call, Chroot, Chdir)) return; // If jail state is ROOT_CHANGED, generate BugReport. diff --git a/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp index 1a7f0d5ab74c..77a3218f55fb 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp @@ -10,11 +10,12 @@ // //===----------------------------------------------------------------------===// -#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/AST/DeclTemplate.h" #include "clang/Driver/DriverDiagnostic.h" +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicType.h" @@ -71,42 +72,27 @@ public: SVal) const; CallDescriptionMap<NoItParamFn> NoIterParamFunctions = { - {{0, "clear", 0}, - &ContainerModeling::handleClear}, - {{0, "assign", 2}, - &ContainerModeling::handleAssign}, - {{0, "push_back", 1}, - &ContainerModeling::handlePushBack}, - {{0, "emplace_back", 1}, - &ContainerModeling::handlePushBack}, - {{0, "pop_back", 0}, - &ContainerModeling::handlePopBack}, - {{0, "push_front", 1}, - &ContainerModeling::handlePushFront}, - {{0, "emplace_front", 1}, - &ContainerModeling::handlePushFront}, - {{0, "pop_front", 0}, - &ContainerModeling::handlePopFront}, + {{"clear", 0}, &ContainerModeling::handleClear}, + {{"assign", 2}, &ContainerModeling::handleAssign}, + {{"push_back", 1}, &ContainerModeling::handlePushBack}, + {{"emplace_back", 1}, &ContainerModeling::handlePushBack}, + {{"pop_back", 0}, &ContainerModeling::handlePopBack}, + {{"push_front", 1}, &ContainerModeling::handlePushFront}, + {{"emplace_front", 1}, &ContainerModeling::handlePushFront}, + {{"pop_front", 0}, &ContainerModeling::handlePopFront}, }; - + CallDescriptionMap<OneItParamFn> OneIterParamFunctions = { - {{0, "insert", 2}, - &ContainerModeling::handleInsert}, - {{0, "emplace", 2}, - &ContainerModeling::handleInsert}, - {{0, "erase", 1}, - &ContainerModeling::handleErase}, - {{0, "erase_after", 1}, - &ContainerModeling::handleEraseAfter}, + {{"insert", 2}, &ContainerModeling::handleInsert}, + {{"emplace", 2}, &ContainerModeling::handleInsert}, + {{"erase", 1}, &ContainerModeling::handleErase}, + {{"erase_after", 1}, &ContainerModeling::handleEraseAfter}, }; - + CallDescriptionMap<TwoItParamFn> TwoIterParamFunctions = { - {{0, "erase", 2}, - &ContainerModeling::handleErase}, - {{0, "erase_after", 2}, - &ContainerModeling::handleEraseAfter}, + {{"erase", 2}, &ContainerModeling::handleErase}, + {{"erase_after", 2}, &ContainerModeling::handleEraseAfter}, }; - }; bool isBeginCall(const FunctionDecl *Func); diff --git a/clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp index 4216a6883119..8da482a2aec9 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp @@ -49,7 +49,8 @@ private: bool isLossOfSign(const ImplicitCastExpr *Cast, CheckerContext &C) const; - void reportBug(ExplodedNode *N, CheckerContext &C, const char Msg[]) const; + void reportBug(ExplodedNode *N, const Expr *E, CheckerContext &C, + const char Msg[]) const; }; } @@ -108,20 +109,21 @@ void ConversionChecker::checkPreStmt(const ImplicitCastExpr *Cast, if (!N) return; if (LossOfSign) - reportBug(N, C, "Loss of sign in implicit conversion"); + reportBug(N, Cast, C, "Loss of sign in implicit conversion"); if (LossOfPrecision) - reportBug(N, C, "Loss of precision in implicit conversion"); + reportBug(N, Cast, C, "Loss of precision in implicit conversion"); } } -void ConversionChecker::reportBug(ExplodedNode *N, CheckerContext &C, - const char Msg[]) const { +void ConversionChecker::reportBug(ExplodedNode *N, const Expr *E, + CheckerContext &C, const char Msg[]) const { if (!BT) BT.reset( new BuiltinBug(this, "Conversion", "Possible loss of sign/precision.")); // Generate a report for this bug. auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N); + bugreporter::trackExpressionValue(N, E, *R); C.emitReport(std::move(R)); } diff --git a/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp index 6fed999ffc80..47fd57c7db9b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp @@ -13,6 +13,7 @@ #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" @@ -40,10 +41,10 @@ class DebugContainerModeling CheckerContext &) const; CallDescriptionMap<FnCheck> Callbacks = { - {{0, "clang_analyzer_container_begin", 1}, - &DebugContainerModeling::analyzerContainerBegin}, - {{0, "clang_analyzer_container_end", 1}, - &DebugContainerModeling::analyzerContainerEnd}, + {{"clang_analyzer_container_begin", 1}, + &DebugContainerModeling::analyzerContainerBegin}, + {{"clang_analyzer_container_end", 1}, + &DebugContainerModeling::analyzerContainerEnd}, }; public: diff --git a/clang/lib/StaticAnalyzer/Checkers/DebugIteratorModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/DebugIteratorModeling.cpp index 5833eea56da8..6add9a007a87 100644 --- a/clang/lib/StaticAnalyzer/Checkers/DebugIteratorModeling.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/DebugIteratorModeling.cpp @@ -13,6 +13,7 @@ #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" @@ -41,12 +42,12 @@ class DebugIteratorModeling CheckerContext &) const; CallDescriptionMap<FnCheck> Callbacks = { - {{0, "clang_analyzer_iterator_position", 1}, - &DebugIteratorModeling::analyzerIteratorPosition}, - {{0, "clang_analyzer_iterator_container", 1}, - &DebugIteratorModeling::analyzerIteratorContainer}, - {{0, "clang_analyzer_iterator_validity", 1}, - &DebugIteratorModeling::analyzerIteratorValidity}, + {{"clang_analyzer_iterator_position", 1}, + &DebugIteratorModeling::analyzerIteratorPosition}, + {{"clang_analyzer_iterator_container", 1}, + &DebugIteratorModeling::analyzerIteratorContainer}, + {{"clang_analyzer_iterator_validity", 1}, + &DebugIteratorModeling::analyzerIteratorValidity}, }; public: diff --git a/clang/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp b/clang/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp index df88b71ff063..49486ea796c2 100644 --- a/clang/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp @@ -44,8 +44,8 @@ static bool DefaultMethodFilter(const ObjCMethodDecl *M) { M->getMethodFamily() == OMF_dealloc || M->getMethodFamily() == OMF_copy || M->getMethodFamily() == OMF_mutableCopy || - M->getSelector().getNameForSlot(0).find("init") != StringRef::npos || - M->getSelector().getNameForSlot(0).find("Init") != StringRef::npos; + M->getSelector().getNameForSlot(0).contains("init") || + M->getSelector().getNameForSlot(0).contains("Init"); } class DirectIvarAssignment : diff --git a/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp b/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp index 14ba5d769969..b07f59125a82 100644 --- a/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp @@ -384,7 +384,7 @@ void DynamicTypePropagation::checkPostCall(const CallEvent &Call, // FIXME: Instead of relying on the ParentMap, we should have the // trigger-statement (InitListExpr in this case) available in this // callback, ideally as part of CallEvent. - if (dyn_cast_or_null<InitListExpr>( + if (isa_and_nonnull<InitListExpr>( LCtx->getParentMap().getParent(Ctor->getOriginExpr()))) return; diff --git a/clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp index 0e94b915a468..e5088fb266bc 100644 --- a/clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp @@ -94,10 +94,10 @@ void EnumCastOutOfRangeChecker::checkPreStmt(const CastExpr *CE, // Only perform enum range check on casts where such checks are valid. For // all other cast kinds (where enum range checks are unnecessary or invalid), - // just return immediately. TODO: The set of casts whitelisted for enum - // range checking may be incomplete. Better to add a missing cast kind to - // enable a missing check than to generate false negatives and have to remove - // those later. + // just return immediately. TODO: The set of casts allowed for enum range + // checking may be incomplete. Better to add a missing cast kind to enable a + // missing check than to generate false negatives and have to remove those + // later. switch (CE->getCastKind()) { case CK_IntegralCast: break; diff --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp index 42c777eb2c52..66ef781871ec 100644 --- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp @@ -260,7 +260,7 @@ private: } bool isDestinationArgument(unsigned ArgNum) const { - return (llvm::find(DstArgs, ArgNum) != DstArgs.end()); + return llvm::is_contained(DstArgs, ArgNum); } static bool isTaintedOrPointsToTainted(const Expr *E, @@ -435,7 +435,6 @@ GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule( .Case("getch", {{}, {ReturnValueIndex}}) .Case("getchar", {{}, {ReturnValueIndex}}) .Case("getchar_unlocked", {{}, {ReturnValueIndex}}) - .Case("getenv", {{}, {ReturnValueIndex}}) .Case("gets", {{}, {0, ReturnValueIndex}}) .Case("scanf", {{}, {}, VariadicType::Dst, 1}) .Case("socket", {{}, @@ -468,6 +467,16 @@ GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule( if (!Rule.isNull()) return Rule; + + // `getenv` returns taint only in untrusted environments. + if (FData.FullName == "getenv") { + if (C.getAnalysisManager() + .getAnalyzerOptions() + .ShouldAssumeControlledEnvironment) + return {}; + return {{}, {ReturnValueIndex}}; + } + assert(FData.FDecl); // Check if it's one of the memory setting/copying functions. @@ -505,7 +514,7 @@ GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule( if (OneOf("snprintf")) return {{1}, {0, ReturnValueIndex}, VariadicType::Src, 3}; if (OneOf("sprintf")) - return {{}, {0, ReturnValueIndex}, VariadicType::Src, 2}; + return {{1}, {0, ReturnValueIndex}, VariadicType::Src, 2}; if (OneOf("strcpy", "stpcpy", "strcat")) return {{1}, {0, ReturnValueIndex}}; if (OneOf("bcopy")) @@ -780,7 +789,7 @@ bool GenericTaintChecker::isStdin(const Expr *E, CheckerContext &C) { // variable named stdin with the proper type. if (const auto *D = dyn_cast_or_null<VarDecl>(DeclReg->getDecl())) { D = D->getCanonicalDecl(); - if ((D->getName().find("stdin") != StringRef::npos) && D->isExternC()) { + if (D->getName().contains("stdin") && D->isExternC()) { const auto *PtrTy = dyn_cast<PointerType>(D->getType().getTypePtr()); if (PtrTy && PtrTy->getPointeeType().getCanonicalType() == C.getASTContext().getFILEType().getCanonicalType()) @@ -807,7 +816,7 @@ static bool getPrintfFormatArgumentNum(const CallEvent &Call, } // Or if a function is named setproctitle (this is a heuristic). - if (C.getCalleeName(FDecl).find("setproctitle") != StringRef::npos) { + if (C.getCalleeName(FDecl).contains("setproctitle")) { ArgNum = 0; return true; } diff --git a/clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp index bcae73378028..6f9867b9607d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp @@ -13,11 +13,12 @@ //===----------------------------------------------------------------------===// #include "AllocationState.h" -#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "InterCheckerAPI.h" +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h" #include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" @@ -125,19 +126,15 @@ bool InnerPointerChecker::isInvalidatingMemberFunction( return true; return false; } - return (isa<CXXDestructorCall>(Call) || Call.isCalled(AppendFn) || - Call.isCalled(AssignFn) || Call.isCalled(ClearFn) || - Call.isCalled(EraseFn) || Call.isCalled(InsertFn) || - Call.isCalled(PopBackFn) || Call.isCalled(PushBackFn) || - Call.isCalled(ReplaceFn) || Call.isCalled(ReserveFn) || - Call.isCalled(ResizeFn) || Call.isCalled(ShrinkToFitFn) || - Call.isCalled(SwapFn)); + return isa<CXXDestructorCall>(Call) || + matchesAny(Call, AppendFn, AssignFn, ClearFn, EraseFn, InsertFn, + PopBackFn, PushBackFn, ReplaceFn, ReserveFn, ResizeFn, + ShrinkToFitFn, SwapFn); } bool InnerPointerChecker::isInnerPointerAccessFunction( const CallEvent &Call) const { - return (Call.isCalled(CStrFn) || Call.isCalled(DataFn) || - Call.isCalled(DataMemberFn)); + return matchesAny(Call, CStrFn, DataFn, DataMemberFn); } void InnerPointerChecker::markPtrSymbolsReleased(const CallEvent &Call, @@ -184,7 +181,7 @@ void InnerPointerChecker::checkFunctionArguments(const CallEvent &Call, // std::addressof function accepts a non-const reference as an argument, // but doesn't modify it. - if (Call.isCalled(AddressofFn)) + if (AddressofFn.matches(Call)) continue; markPtrSymbolsReleased(Call, State, ArgRegion, C); diff --git a/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp index ab5e6a1c9991..235c9010412a 100644 --- a/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp @@ -64,10 +64,11 @@ // making an assumption e.g. `S1 + n == S2 + m` we store `S1 - S2 == m - n` as // a constraint which we later retrieve when doing an actual comparison. -#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/AST/DeclTemplate.h" +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicType.h" diff --git a/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp index a47484497771..c682449921ac 100644 --- a/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp @@ -14,10 +14,10 @@ #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" - #include "Iterator.h" using namespace clang; diff --git a/clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp index 28d3e058fee2..b57c5dc6de56 100644 --- a/clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp @@ -949,7 +949,7 @@ void NonLocalizedStringChecker::checkPostCall(const CallEvent &Call, const IdentifierInfo *Identifier = Call.getCalleeIdentifier(); SVal sv = Call.getReturnValue(); - if (isAnnotatedAsReturningLocalized(D) || LSF.count(Identifier) != 0) { + if (isAnnotatedAsReturningLocalized(D) || LSF.contains(Identifier)) { setLocalizedState(sv, C); } else if (isNSStringType(RT, C.getASTContext()) && !hasLocalizedState(sv, C)) { @@ -1339,7 +1339,10 @@ bool PluralMisuseChecker::MethodCrawler::EndVisitIfStmt(IfStmt *I) { } bool PluralMisuseChecker::MethodCrawler::VisitIfStmt(const IfStmt *I) { - const Expr *Condition = I->getCond()->IgnoreParenImpCasts(); + const Expr *Condition = I->getCond(); + if (!Condition) + return true; + Condition = Condition->IgnoreParenImpCasts(); if (isCheckingPlurality(Condition)) { MatchingStatements.push_back(I); InMatchingStatement = true; diff --git a/clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp index b72d72580c28..5bf96acc0462 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp @@ -27,6 +27,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/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" @@ -180,7 +181,7 @@ static bool isInMIGCall(CheckerContext &C) { } void MIGChecker::checkPostCall(const CallEvent &Call, CheckerContext &C) const { - if (Call.isCalled(OsRefRetain)) { + if (OsRefRetain.matches(Call)) { // If the code is doing reference counting over the parameter, // it opens up an opportunity for safely calling a destructor function. // TODO: We should still check for over-releases. @@ -198,7 +199,7 @@ void MIGChecker::checkPostCall(const CallEvent &Call, CheckerContext &C) const { auto I = llvm::find_if(Deallocators, [&](const std::pair<CallDescription, unsigned> &Item) { - return Call.isCalled(Item.first); + return Item.first.matches(Call); }); if (I == Deallocators.end()) return; diff --git a/clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp index a157ee2da5df..635eb00e4ca9 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp @@ -160,7 +160,7 @@ static bool isEnclosingFunctionParam(const Expr *E) { E = E->IgnoreParenCasts(); if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E)) { const ValueDecl *VD = DRE->getDecl(); - if (isa<ImplicitParamDecl>(VD) || isa<ParmVarDecl>(VD)) + if (isa<ImplicitParamDecl, ParmVarDecl>(VD)) return true; } return false; @@ -199,8 +199,7 @@ unsigned MacOSKeychainAPIChecker::getTrackedFunctionIndex(StringRef Name, static bool isBadDeallocationArgument(const MemRegion *Arg) { if (!Arg) return false; - return isa<AllocaRegion>(Arg) || isa<BlockDataRegion>(Arg) || - isa<TypedRegion>(Arg); + return isa<AllocaRegion, BlockDataRegion, TypedRegion>(Arg); } /// Given the address expression, retrieve the value it's pointing to. Assume diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index a6470da09c45..10ed6149528c 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -48,9 +48,13 @@ #include "InterCheckerAPI.h" #include "clang/AST/Attr.h" #include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" #include "clang/AST/Expr.h" #include "clang/AST/ExprCXX.h" #include "clang/AST/ParentMap.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Analysis/ProgramPoint.h" #include "clang/Basic/LLVM.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TargetInfo.h" @@ -60,20 +64,26 @@ #include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h" #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h" #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/StoreRef.h" #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h" #include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SetOperations.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringExtras.h" +#include "llvm/Support/Casting.h" #include "llvm/Support/Compiler.h" #include "llvm/Support/ErrorHandling.h" +#include "llvm/Support/raw_ostream.h" #include <climits> #include <functional> #include <utility> @@ -298,6 +308,8 @@ public: /// which might free a pointer are annotated. DefaultBool ShouldIncludeOwnershipAnnotatedFunctions; + DefaultBool ShouldRegisterNoOwnershipChangeVisitor; + /// Many checkers are essentially built into this one, so enabling them will /// make MallocChecker perform additional modeling and reporting. enum CheckKind { @@ -722,11 +734,169 @@ private: bool isArgZERO_SIZE_PTR(ProgramStateRef State, CheckerContext &C, SVal ArgVal) const; }; +} // end anonymous namespace + +//===----------------------------------------------------------------------===// +// Definition of NoOwnershipChangeVisitor. +//===----------------------------------------------------------------------===// + +namespace { +class NoOwnershipChangeVisitor final : public NoStateChangeFuncVisitor { + SymbolRef Sym; + using OwnerSet = llvm::SmallPtrSet<const MemRegion *, 8>; + + // Collect which entities point to the allocated memory, and could be + // responsible for deallocating it. + class OwnershipBindingsHandler : public StoreManager::BindingsHandler { + SymbolRef Sym; + OwnerSet &Owners; + + public: + OwnershipBindingsHandler(SymbolRef Sym, OwnerSet &Owners) + : Sym(Sym), Owners(Owners) {} + + bool HandleBinding(StoreManager &SMgr, Store Store, const MemRegion *Region, + SVal Val) override { + if (Val.getAsSymbol() == Sym) + Owners.insert(Region); + return true; + } + + LLVM_DUMP_METHOD void dump() const { dumpToStream(llvm::errs()); } + LLVM_DUMP_METHOD void dumpToStream(llvm::raw_ostream &out) const { + out << "Owners: {\n"; + for (const MemRegion *Owner : Owners) { + out << " "; + Owner->dumpToStream(out); + out << ",\n"; + } + out << "}\n"; + } + }; + +protected: + OwnerSet getOwnersAtNode(const ExplodedNode *N) { + OwnerSet Ret; + + ProgramStateRef State = N->getState(); + OwnershipBindingsHandler Handler{Sym, Ret}; + State->getStateManager().getStoreManager().iterBindings(State->getStore(), + Handler); + return Ret; + } + + LLVM_DUMP_METHOD static std::string + getFunctionName(const ExplodedNode *CallEnterN) { + if (const CallExpr *CE = llvm::dyn_cast_or_null<CallExpr>( + CallEnterN->getLocationAs<CallEnter>()->getCallExpr())) + if (const FunctionDecl *FD = CE->getDirectCallee()) + return FD->getQualifiedNameAsString(); + return ""; + } + + bool doesFnIntendToHandleOwnership(const Decl *Callee, ASTContext &ACtx) { + using namespace clang::ast_matchers; + const FunctionDecl *FD = dyn_cast<FunctionDecl>(Callee); + if (!FD) + return false; + // TODO: Operator delete is hardly the only deallocator -- Can we reuse + // isFreeingCall() or something thats already here? + auto Deallocations = match( + stmt(hasDescendant(cxxDeleteExpr().bind("delete")) + ), *FD->getBody(), ACtx); + // TODO: Ownership my change with an attempt to store the allocated memory. + return !Deallocations.empty(); + } + + virtual bool + wasModifiedInFunction(const ExplodedNode *CallEnterN, + const ExplodedNode *CallExitEndN) override { + if (!doesFnIntendToHandleOwnership( + CallExitEndN->getFirstPred()->getLocationContext()->getDecl(), + CallExitEndN->getState()->getAnalysisManager().getASTContext())) + return true; + + if (CallEnterN->getState()->get<RegionState>(Sym) != + CallExitEndN->getState()->get<RegionState>(Sym)) + return true; + + OwnerSet CurrOwners = getOwnersAtNode(CallEnterN); + OwnerSet ExitOwners = getOwnersAtNode(CallExitEndN); + + // Owners in the current set may be purged from the analyzer later on. + // If a variable is dead (is not referenced directly or indirectly after + // some point), it will be removed from the Store before the end of its + // actual lifetime. + // This means that that if the ownership status didn't change, CurrOwners + // must be a superset of, but not necessarily equal to ExitOwners. + return !llvm::set_is_subset(ExitOwners, CurrOwners); + } + + static PathDiagnosticPieceRef emitNote(const ExplodedNode *N) { + PathDiagnosticLocation L = PathDiagnosticLocation::create( + N->getLocation(), + N->getState()->getStateManager().getContext().getSourceManager()); + return std::make_shared<PathDiagnosticEventPiece>( + L, "Returning without deallocating memory or storing the pointer for " + "later deallocation"); + } + + virtual PathDiagnosticPieceRef + maybeEmitNoteForObjCSelf(PathSensitiveBugReport &R, + const ObjCMethodCall &Call, + const ExplodedNode *N) override { + // TODO: Implement. + return nullptr; + } + + virtual PathDiagnosticPieceRef + maybeEmitNoteForCXXThis(PathSensitiveBugReport &R, + const CXXConstructorCall &Call, + const ExplodedNode *N) override { + // TODO: Implement. + return nullptr; + } + + virtual PathDiagnosticPieceRef + maybeEmitNoteForParameters(PathSensitiveBugReport &R, const CallEvent &Call, + const ExplodedNode *N) override { + // TODO: Factor the logic of "what constitutes as an entity being passed + // into a function call" out by reusing the code in + // NoStoreFuncVisitor::maybeEmitNoteForParameters, maybe by incorporating + // the printing technology in UninitializedObject's FieldChainInfo. + ArrayRef<ParmVarDecl *> Parameters = Call.parameters(); + for (unsigned I = 0; I < Call.getNumArgs() && I < Parameters.size(); ++I) { + SVal V = Call.getArgSVal(I); + if (V.getAsSymbol() == Sym) + return emitNote(N); + } + return nullptr; + } + +public: + NoOwnershipChangeVisitor(SymbolRef Sym) + : NoStateChangeFuncVisitor(bugreporter::TrackingKind::Thorough), + Sym(Sym) {} + + void Profile(llvm::FoldingSetNodeID &ID) const override { + static int Tag = 0; + ID.AddPointer(&Tag); + ID.AddPointer(Sym); + } + + void *getTag() const { + static int Tag = 0; + return static_cast<void *>(&Tag); + } +}; + +} // end anonymous namespace //===----------------------------------------------------------------------===// // Definition of MallocBugVisitor. //===----------------------------------------------------------------------===// +namespace { /// The bug visitor which allows us to print extra diagnostics along the /// BugReport path. For example, showing the allocation site of the leaked /// region. @@ -767,7 +937,7 @@ public: /// Did not track -> allocated. Other state (released) -> allocated. static inline bool isAllocated(const RefState *RSCurr, const RefState *RSPrev, const Stmt *Stmt) { - return (Stmt && (isa<CallExpr>(Stmt) || isa<CXXNewExpr>(Stmt)) && + return (isa_and_nonnull<CallExpr, CXXNewExpr>(Stmt) && (RSCurr && (RSCurr->isAllocated() || RSCurr->isAllocatedOfSizeZero())) && (!RSPrev || @@ -780,8 +950,7 @@ public: const Stmt *Stmt) { bool IsReleased = (RSCurr && RSCurr->isReleased()) && (!RSPrev || !RSPrev->isReleased()); - assert(!IsReleased || - (Stmt && (isa<CallExpr>(Stmt) || isa<CXXDeleteExpr>(Stmt))) || + assert(!IsReleased || (isa_and_nonnull<CallExpr, CXXDeleteExpr>(Stmt)) || (!Stmt && RSCurr->getAllocationFamily() == AF_InnerBuffer)); return IsReleased; } @@ -789,11 +958,10 @@ public: /// Did not track -> relinquished. Other state (allocated) -> relinquished. static inline bool isRelinquished(const RefState *RSCurr, const RefState *RSPrev, const Stmt *Stmt) { - return (Stmt && - (isa<CallExpr>(Stmt) || isa<ObjCMessageExpr>(Stmt) || - isa<ObjCPropertyRefExpr>(Stmt)) && - (RSCurr && RSCurr->isRelinquished()) && - (!RSPrev || !RSPrev->isRelinquished())); + return ( + isa_and_nonnull<CallExpr, ObjCMessageExpr, ObjCPropertyRefExpr>(Stmt) && + (RSCurr && RSCurr->isRelinquished()) && + (!RSPrev || !RSPrev->isRelinquished())); } /// If the expression is not a call, and the state change is @@ -803,7 +971,7 @@ public: static inline bool hasReallocFailed(const RefState *RSCurr, const RefState *RSPrev, const Stmt *Stmt) { - return ((!Stmt || !isa<CallExpr>(Stmt)) && + return ((!isa_and_nonnull<CallExpr>(Stmt)) && (RSCurr && (RSCurr->isAllocated() || RSCurr->isAllocatedOfSizeZero())) && (RSPrev && @@ -851,7 +1019,6 @@ private: } }; }; - } // end anonymous namespace // A map from the freed symbol to the symbol representing the return value of @@ -1753,7 +1920,7 @@ ProgramStateRef MallocChecker::FreeMemAux( // Parameters, locals, statics, globals, and memory returned by // __builtin_alloca() shouldn't be freed. - if (!(isa<UnknownSpaceRegion>(MS) || isa<HeapSpaceRegion>(MS))) { + if (!isa<UnknownSpaceRegion, HeapSpaceRegion>(MS)) { // FIXME: at the time this code was written, malloc() regions were // represented by conjured symbols, which are all in UnknownSpaceRegion. // This means that there isn't actually anything from HeapSpaceRegion @@ -2303,7 +2470,8 @@ void MallocChecker::HandleUseZeroAlloc(CheckerContext &C, SourceRange Range, categories::MemoryError)); auto R = std::make_unique<PathSensitiveBugReport>( - *BT_UseZerroAllocated[*CheckKind], "Use of zero-allocated memory", N); + *BT_UseZerroAllocated[*CheckKind], + "Use of memory allocated with size zero", N); R->addRange(Range); if (Sym) { @@ -2579,6 +2747,8 @@ void MallocChecker::HandleLeak(SymbolRef Sym, ExplodedNode *N, AllocNode->getLocationContext()->getDecl()); R->markInteresting(Sym); R->addVisitor<MallocBugVisitor>(Sym, true); + if (ShouldRegisterNoOwnershipChangeVisitor) + R->addVisitor<NoOwnershipChangeVisitor>(Sym); C.emitReport(std::move(R)); } @@ -2733,7 +2903,7 @@ void MallocChecker::checkEscapeOnReturn(const ReturnStmt *S, // the callee could still free the memory. // TODO: This logic should be a part of generic symbol escape callback. if (const MemRegion *MR = RetVal.getAsRegion()) - if (isa<FieldRegion>(MR) || isa<ElementRegion>(MR)) + if (isa<FieldRegion, ElementRegion>(MR)) if (const SymbolicRegion *BMR = dyn_cast<SymbolicRegion>(MR->getBaseRegion())) Sym = BMR->getSymbol(); @@ -2916,7 +3086,7 @@ bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly( // TODO: If we want to be more optimistic here, we'll need to make sure that // regions escape to C++ containers. They seem to do that even now, but for // mysterious reasons. - if (!(isa<SimpleFunctionCall>(Call) || isa<ObjCMethodCall>(Call))) + if (!isa<SimpleFunctionCall, ObjCMethodCall>(Call)) return true; // Check Objective-C messages by selector name. @@ -3024,7 +3194,7 @@ bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly( const Expr *ArgE = Call->getArgExpr(0)->IgnoreParenCasts(); if (const DeclRefExpr *ArgDRE = dyn_cast<DeclRefExpr>(ArgE)) if (const VarDecl *D = dyn_cast<VarDecl>(ArgDRE->getDecl())) - if (D->getCanonicalDecl()->getName().find("std") != StringRef::npos) + if (D->getCanonicalDecl()->getName().contains("std")) return true; } } @@ -3395,6 +3565,9 @@ void ento::registerDynamicMemoryModeling(CheckerManager &mgr) { auto *checker = mgr.registerChecker<MallocChecker>(); checker->ShouldIncludeOwnershipAnnotatedFunctions = mgr.getAnalyzerOptions().getCheckerBooleanOption(checker, "Optimistic"); + checker->ShouldRegisterNoOwnershipChangeVisitor = + mgr.getAnalyzerOptions().getCheckerBooleanOption( + checker, "AddNoOwnershipChangeNotes"); } bool ento::shouldRegisterDynamicMemoryModeling(const CheckerManager &mgr) { diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp index e31630f63b5a..a6e8fcd425d5 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp @@ -32,12 +32,14 @@ using llvm::APSInt; namespace { struct MallocOverflowCheck { + const CallExpr *call; const BinaryOperator *mulop; const Expr *variable; APSInt maxVal; - MallocOverflowCheck(const BinaryOperator *m, const Expr *v, APSInt val) - : mulop(m), variable(v), maxVal(std::move(val)) {} + MallocOverflowCheck(const CallExpr *call, const BinaryOperator *m, + const Expr *v, APSInt val) + : call(call), mulop(m), variable(v), maxVal(std::move(val)) {} }; class MallocOverflowSecurityChecker : public Checker<check::ASTCodeBody> { @@ -46,8 +48,8 @@ public: BugReporter &BR) const; void CheckMallocArgument( - SmallVectorImpl<MallocOverflowCheck> &PossibleMallocOverflows, - const Expr *TheArgument, ASTContext &Context) const; + SmallVectorImpl<MallocOverflowCheck> &PossibleMallocOverflows, + const CallExpr *TheCall, ASTContext &Context) const; void OutputPossibleOverflows( SmallVectorImpl<MallocOverflowCheck> &PossibleMallocOverflows, @@ -62,16 +64,15 @@ static inline bool EvaluatesToZero(APSInt &Val, BinaryOperatorKind op) { } void MallocOverflowSecurityChecker::CheckMallocArgument( - SmallVectorImpl<MallocOverflowCheck> &PossibleMallocOverflows, - const Expr *TheArgument, - ASTContext &Context) const { + SmallVectorImpl<MallocOverflowCheck> &PossibleMallocOverflows, + const CallExpr *TheCall, ASTContext &Context) const { /* Look for a linear combination with a single variable, and at least one multiplication. Reject anything that applies to the variable: an explicit cast, conditional expression, an operation that could reduce the range of the result, or anything too complicated :-). */ - const Expr *e = TheArgument; + const Expr *e = TheCall->getArg(0); const BinaryOperator * mulop = nullptr; APSInt maxVal; @@ -101,8 +102,7 @@ void MallocOverflowSecurityChecker::CheckMallocArgument( e = rhs; } else return; - } - else if (isa<DeclRefExpr>(e) || isa<MemberExpr>(e)) + } else if (isa<DeclRefExpr, MemberExpr>(e)) break; else return; @@ -115,9 +115,8 @@ void MallocOverflowSecurityChecker::CheckMallocArgument( // the data so when the body of the function is completely available // we can check for comparisons. - // TODO: Could push this into the innermost scope where 'e' is - // defined, rather than the whole function. - PossibleMallocOverflows.push_back(MallocOverflowCheck(mulop, e, maxVal)); + PossibleMallocOverflows.push_back( + MallocOverflowCheck(TheCall, mulop, e, maxVal)); } namespace { @@ -153,17 +152,19 @@ private: return getDecl(CheckDR) == getDecl(DR) && Pred(Check); return false; }; - toScanFor.erase(std::remove_if(toScanFor.begin(), toScanFor.end(), P), - toScanFor.end()); + llvm::erase_if(toScanFor, P); } void CheckExpr(const Expr *E_p) { - auto PredTrue = [](const MallocOverflowCheck &) { return true; }; const Expr *E = E_p->IgnoreParenImpCasts(); + const auto PrecedesMalloc = [E, this](const MallocOverflowCheck &c) { + return Context.getSourceManager().isBeforeInTranslationUnit( + E->getExprLoc(), c.call->getExprLoc()); + }; if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(E)) - Erase<DeclRefExpr>(DR, PredTrue); + Erase<DeclRefExpr>(DR, PrecedesMalloc); else if (const auto *ME = dyn_cast<MemberExpr>(E)) { - Erase<MemberExpr>(ME, PredTrue); + Erase<MemberExpr>(ME, PrecedesMalloc); } } @@ -322,7 +323,7 @@ void MallocOverflowSecurityChecker::checkASTCodeBody(const Decl *D, if (FnInfo->isStr ("malloc") || FnInfo->isStr ("_MALLOC")) { if (TheCall->getNumArgs() == 1) - CheckMallocArgument(PossibleMallocOverflows, TheCall->getArg(0), + CheckMallocArgument(PossibleMallocOverflows, TheCall, mgr.getASTContext()); } } diff --git a/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp index 5d63d6efd234..517a5d78271b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.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/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" @@ -46,7 +47,7 @@ int MmapWriteExecChecker::ProtRead = 0x01; void MmapWriteExecChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { - if (Call.isCalled(MmapFn) || Call.isCalled(MprotectFn)) { + if (matchesAny(Call, MmapFn, MprotectFn)) { SVal ProtVal = Call.getArgSVal(2); Optional<nonloc::ConcreteInt> ProtLoc = ProtVal.getAs<nonloc::ConcreteInt>(); int64_t Prot = ProtLoc->getValue().getSExtValue(); diff --git a/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp index cbe938982000..4a232c6f4b3f 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp @@ -553,8 +553,8 @@ MoveChecker::classifyObject(const MemRegion *MR, // For the purposes of this checker, we classify move-safe STL types // as not-"STL" types, because that's how the checker treats them. MR = unwrapRValueReferenceIndirection(MR); - bool IsLocal = - MR && isa<VarRegion>(MR) && isa<StackSpaceRegion>(MR->getMemorySpace()); + bool IsLocal = isa_and_nonnull<VarRegion>(MR) && + isa<StackSpaceRegion>(MR->getMemorySpace()); if (!RD || !RD->getDeclContext()->isStdNamespace()) return { IsLocal, SK_NonStd }; @@ -712,12 +712,9 @@ ProgramStateRef MoveChecker::checkRegionChanges( // directly, but not all of them end up being invalidated. // But when they do, they appear in the InvalidatedRegions array as well. for (const auto *Region : RequestedRegions) { - if (ThisRegion != Region) { - if (llvm::find(InvalidatedRegions, Region) != - std::end(InvalidatedRegions)) { - State = removeFromState(State, Region); - } - } + if (ThisRegion != Region && + llvm::is_contained(InvalidatedRegions, Region)) + State = removeFromState(State, Region); } } else { // For invalidations that aren't caused by calls, assume nothing. In diff --git a/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp index 90c5583d8969..dcca8be55e33 100644 --- a/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp @@ -6,7 +6,7 @@ // //===----------------------------------------------------------------------===// // -// This file defines a CheckNSError, a flow-insenstive check +// This file defines a CheckNSError, a flow-insensitive check // that determines if an Objective-C class interface correctly returns // a non-void return type. // diff --git a/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp index ee71b55a39e6..f4e9a67438e7 100644 --- a/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp @@ -21,6 +21,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/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" @@ -681,9 +682,7 @@ ProgramStateRef PthreadLockChecker::checkRegionChanges( // We assume that system library function wouldn't touch the mutex unless // it takes the mutex explicitly as an argument. // FIXME: This is a bit quadratic. - if (IsLibraryFunction && - std::find(ExplicitRegions.begin(), ExplicitRegions.end(), R) == - ExplicitRegions.end()) + if (IsLibraryFunction && !llvm::is_contained(ExplicitRegions, R)) continue; State = State->remove<LockMap>(R); diff --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp index 3f3267ff9391..0bde088d0e85 100644 --- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp @@ -290,7 +290,7 @@ void RetainCountChecker::checkPostStmt(const ObjCIvarRefExpr *IRE, ProgramStateRef State = C.getState(); SymbolRef Sym = State->getSVal(*IVarLoc).getAsSymbol(); - if (!Sym || !dyn_cast_or_null<ObjCIvarRegion>(Sym->getOriginRegion())) + if (!Sym || !isa_and_nonnull<ObjCIvarRegion>(Sym->getOriginRegion())) return; // Accessing an ivar directly is unusual. If we've done that, be more @@ -1188,14 +1188,14 @@ ProgramStateRef RetainCountChecker::checkRegionChanges( if (!invalidated) return state; - llvm::SmallPtrSet<SymbolRef, 8> WhitelistedSymbols; + llvm::SmallPtrSet<SymbolRef, 8> AllowedSymbols; for (const MemRegion *I : ExplicitRegions) if (const SymbolicRegion *SR = I->StripCasts()->getAs<SymbolicRegion>()) - WhitelistedSymbols.insert(SR->getSymbol()); + AllowedSymbols.insert(SR->getSymbol()); for (SymbolRef sym : *invalidated) { - if (WhitelistedSymbols.count(sym)) + if (AllowedSymbols.count(sym)) continue; // Remove any existing reference-count binding. state = removeRefBinding(state, sym); diff --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp index 64ac6bc4c06b..41ef45d317cd 100644 --- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp @@ -73,11 +73,8 @@ RefCountBug::RefCountBug(CheckerNameRef Checker, RefCountBugKind BT) static bool isNumericLiteralExpression(const Expr *E) { // FIXME: This set of cases was copied from SemaExprObjC. - return isa<IntegerLiteral>(E) || - isa<CharacterLiteral>(E) || - isa<FloatingLiteral>(E) || - isa<ObjCBoolLiteralExpr>(E) || - isa<CXXBoolLiteralExpr>(E); + return isa<IntegerLiteral, CharacterLiteral, FloatingLiteral, + ObjCBoolLiteralExpr, CXXBoolLiteralExpr>(E); } /// If type represents a pointer to CXXRecordDecl, diff --git a/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp index 885750218b9e..c4dc06d4a077 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp @@ -12,6 +12,7 @@ //===----------------------------------------------------------------------===// #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" @@ -79,16 +80,44 @@ void ReturnPointerRangeChecker::checkPreStmt(const ReturnStmt *RS, "Returned pointer value points outside the original object " "(potential buffer overflow)")); - // FIXME: It would be nice to eventually make this diagnostic more clear, - // e.g., by referencing the original declaration or by saying *why* this - // reference is outside the range. - // Generate a report for this bug. - auto report = + auto Report = std::make_unique<PathSensitiveBugReport>(*BT, BT->getDescription(), N); + Report->addRange(RetE->getSourceRange()); + + const auto ConcreteElementCount = ElementCount.getAs<nonloc::ConcreteInt>(); + const auto ConcreteIdx = Idx.getAs<nonloc::ConcreteInt>(); + + const auto *DeclR = ER->getSuperRegion()->getAs<DeclRegion>(); + + if (DeclR) + Report->addNote("Original object declared here", + {DeclR->getDecl(), C.getSourceManager()}); + + if (ConcreteElementCount) { + SmallString<128> SBuf; + llvm::raw_svector_ostream OS(SBuf); + OS << "Original object "; + if (DeclR) { + OS << "'"; + DeclR->getDecl()->printName(OS); + OS << "' "; + } + OS << "is an array of " << ConcreteElementCount->getValue() << " '"; + ER->getValueType().print(OS, + PrintingPolicy(C.getASTContext().getLangOpts())); + OS << "' objects"; + if (ConcreteIdx) { + OS << ", returned pointer points at index " << ConcreteIdx->getValue(); + } + + Report->addNote(SBuf, + {RetE, C.getSourceManager(), C.getLocationContext()}); + } + + bugreporter::trackExpressionValue(N, RetE, *Report); - report->addRange(RetE->getSourceRange()); - C.emitReport(std::move(report)); + C.emitReport(std::move(Report)); } } diff --git a/clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp index 14ecede17083..cd502241ef61 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp @@ -14,6 +14,7 @@ #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "llvm/ADT/Optional.h" diff --git a/clang/lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp index 933e0146ff59..ea72ebe3ed57 100644 --- a/clang/lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp @@ -12,6 +12,7 @@ #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" diff --git a/clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp index 8d380ed1b93d..1de5d7285f65 100644 --- a/clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp @@ -17,6 +17,7 @@ #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include <utility> @@ -119,7 +120,7 @@ void SimpleStreamChecker::checkPostCall(const CallEvent &Call, if (!Call.isGlobalCFunction()) return; - if (!Call.isCalled(OpenFn)) + if (!OpenFn.matches(Call)) return; // Get the symbolic value corresponding to the file handle. @@ -138,7 +139,7 @@ void SimpleStreamChecker::checkPreCall(const CallEvent &Call, if (!Call.isGlobalCFunction()) return; - if (!Call.isCalled(CloseFn)) + if (!CloseFn.matches(Call)) return; // Get the symbolic value corresponding to the file handle. diff --git a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp index 09e885e8133f..c789a8dbcca1 100644 --- a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp @@ -23,6 +23,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/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h" @@ -102,12 +103,8 @@ static bool hasStdClassWithName(const CXXRecordDecl *RD, ArrayRef<llvm::StringLiteral> Names) { if (!RD || !RD->getDeclContext()->isStdNamespace()) return false; - if (RD->getDeclName().isIdentifier()) { - StringRef Name = RD->getName(); - return llvm::any_of(Names, [&Name](StringRef GivenName) -> bool { - return Name == GivenName; - }); - } + if (RD->getDeclName().isIdentifier()) + return llvm::is_contained(Names, RD->getName()); return false; } @@ -289,7 +286,7 @@ bool SmartPtrModeling::evalCall(const CallEvent &Call, if (ModelSmartPtrDereference && isStdOstreamOperatorCall(Call)) return handleOstreamOperator(Call, C); - if (Call.isCalled(StdSwapCall)) { + if (StdSwapCall.matches(Call)) { // Check the first arg, if it is of std::unique_ptr type. assert(Call.getNumArgs() == 2 && "std::swap should have two arguments"); const Expr *FirstArg = Call.getArgExpr(0); @@ -298,8 +295,7 @@ bool SmartPtrModeling::evalCall(const CallEvent &Call, return handleSwap(State, Call.getArgSVal(0), Call.getArgSVal(1), C); } - if (Call.isCalled(StdMakeUniqueCall) || - Call.isCalled(StdMakeUniqueForOverwriteCall)) { + if (matchesAny(Call, StdMakeUniqueCall, StdMakeUniqueForOverwriteCall)) { if (!ModelSmartPtrDereference) return false; diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp index b5c9356322fc..d5e86e86424d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp @@ -11,9 +11,9 @@ // //===----------------------------------------------------------------------===// -#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/AST/ExprCXX.h" #include "clang/Basic/SourceManager.h" +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" @@ -303,21 +303,53 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS, class CallBack : public StoreManager::BindingsHandler { private: CheckerContext &Ctx; - const StackFrameContext *CurSFC; + const StackFrameContext *PoppedFrame; + + /// Look for stack variables referring to popped stack variables. + /// Returns true only if it found some dangling stack variables + /// referred by an other stack variable from different stack frame. + bool checkForDanglingStackVariable(const MemRegion *Referrer, + const MemRegion *Referred) { + const auto *ReferrerMemSpace = + Referrer->getMemorySpace()->getAs<StackSpaceRegion>(); + const auto *ReferredMemSpace = + Referred->getMemorySpace()->getAs<StackSpaceRegion>(); + + if (!ReferrerMemSpace || !ReferredMemSpace) + return false; + + const auto *ReferrerFrame = ReferrerMemSpace->getStackFrame(); + const auto *ReferredFrame = ReferredMemSpace->getStackFrame(); + + if (ReferrerMemSpace && ReferredMemSpace) { + if (ReferredFrame == PoppedFrame && + ReferrerFrame->isParentOf(PoppedFrame)) { + V.emplace_back(Referrer, Referred); + return true; + } + } + return false; + } public: SmallVector<std::pair<const MemRegion *, const MemRegion *>, 10> V; - CallBack(CheckerContext &CC) : Ctx(CC), CurSFC(CC.getStackFrame()) {} + CallBack(CheckerContext &CC) : Ctx(CC), PoppedFrame(CC.getStackFrame()) {} bool HandleBinding(StoreManager &SMgr, Store S, const MemRegion *Region, SVal Val) override { + const MemRegion *VR = Val.getAsRegion(); + if (!VR) + return true; + if (checkForDanglingStackVariable(Region, VR)) + return true; + + // Check the globals for the same. if (!isa<GlobalsSpaceRegion>(Region->getMemorySpace())) return true; - const MemRegion *VR = Val.getAsRegion(); - if (VR && isa<StackSpaceRegion>(VR->getMemorySpace()) && - !isArcManagedBlock(VR, Ctx) && !isNotInCurrentFrame(VR, Ctx)) + if (VR && VR->hasStackStorage() && !isArcManagedBlock(VR, Ctx) && + !isNotInCurrentFrame(VR, Ctx)) V.emplace_back(Region, VR); return true; } @@ -344,19 +376,41 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS, "invalid after returning from the function"); for (const auto &P : Cb.V) { + const MemRegion *Referrer = P.first; + const MemRegion *Referred = P.second; + // Generate a report for this bug. + const StringRef CommonSuffix = + "upon returning to the caller. This will be a dangling reference"; SmallString<128> Buf; llvm::raw_svector_ostream Out(Buf); - SourceRange Range = genName(Out, P.second, Ctx.getASTContext()); - Out << " is still referred to by the "; - if (isa<StaticGlobalSpaceRegion>(P.first->getMemorySpace())) - Out << "static"; - else - Out << "global"; - Out << " variable '"; - const VarRegion *VR = cast<VarRegion>(P.first->getBaseRegion()); - Out << *VR->getDecl() - << "' upon returning to the caller. This will be a dangling reference"; + const SourceRange Range = genName(Out, Referred, Ctx.getASTContext()); + + if (isa<CXXTempObjectRegion>(Referrer)) { + Out << " is still referred to by a temporary object on the stack " + << CommonSuffix; + auto Report = + std::make_unique<PathSensitiveBugReport>(*BT_stackleak, Out.str(), N); + Ctx.emitReport(std::move(Report)); + return; + } + + const StringRef ReferrerMemorySpace = [](const MemSpaceRegion *Space) { + if (isa<StaticGlobalSpaceRegion>(Space)) + return "static"; + if (isa<GlobalsSpaceRegion>(Space)) + return "global"; + assert(isa<StackSpaceRegion>(Space)); + return "stack"; + }(Referrer->getMemorySpace()); + + // This cast supposed to succeed. + const VarRegion *ReferrerVar = cast<VarRegion>(Referrer->getBaseRegion()); + const std::string ReferrerVarName = + ReferrerVar->getDecl()->getDeclName().getAsString(); + + Out << " is still referred to by the " << ReferrerMemorySpace + << " variable '" << ReferrerVarName << "' " << CommonSuffix; auto Report = std::make_unique<PathSensitiveBugReport>(*BT_stackleak, Out.str(), N); if (Range.isValid()) diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp index e758b465af1b..e8b963a535d8 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp @@ -568,6 +568,7 @@ public: bool DisplayLoadedSummaries = false; bool ModelPOSIX = false; + bool ShouldAssumeControlledEnvironment = false; private: Optional<Summary> findFunctionSummary(const FunctionDecl *FD, @@ -1433,6 +1434,20 @@ void StdLibraryFunctionsChecker::initFunctionSummaries( RetType{Ssize_tTy}), GetLineSummary); + { + Summary GetenvSummary = Summary(NoEvalCall) + .ArgConstraint(NotNull(ArgNo(0))) + .Case({NotNull(Ret)}); + // In untrusted environments the envvar might not exist. + if (!ShouldAssumeControlledEnvironment) + GetenvSummary.Case({NotNull(Ret)->negate()}); + + // char *getenv(const char *name); + addToFunctionSummaryMap( + "getenv", Signature(ArgTypes{ConstCharPtrTy}, RetType{CharPtrTy}), + std::move(GetenvSummary)); + } + if (ModelPOSIX) { // long a64l(const char *str64); @@ -2645,11 +2660,12 @@ void StdLibraryFunctionsChecker::initFunctionSummaries( void ento::registerStdCLibraryFunctionsChecker(CheckerManager &mgr) { auto *Checker = mgr.registerChecker<StdLibraryFunctionsChecker>(); + const AnalyzerOptions &Opts = mgr.getAnalyzerOptions(); Checker->DisplayLoadedSummaries = - mgr.getAnalyzerOptions().getCheckerBooleanOption( - Checker, "DisplayLoadedSummaries"); - Checker->ModelPOSIX = - mgr.getAnalyzerOptions().getCheckerBooleanOption(Checker, "ModelPOSIX"); + Opts.getCheckerBooleanOption(Checker, "DisplayLoadedSummaries"); + Checker->ModelPOSIX = Opts.getCheckerBooleanOption(Checker, "ModelPOSIX"); + Checker->ShouldAssumeControlledEnvironment = + Opts.ShouldAssumeControlledEnvironment; } bool ento::shouldRegisterStdCLibraryFunctionsChecker( diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp index dd65f8c035aa..26218b8e0454 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/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/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h" @@ -1118,4 +1119,4 @@ void ento::registerStreamTesterChecker(CheckerManager &Mgr) { bool ento::shouldRegisterStreamTesterChecker(const CheckerManager &Mgr) { return true; -}
\ No newline at end of file +} diff --git a/clang/lib/StaticAnalyzer/Checkers/StringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StringChecker.cpp new file mode 100644 index 000000000000..0d745f5d8d6f --- /dev/null +++ b/clang/lib/StaticAnalyzer/Checkers/StringChecker.cpp @@ -0,0 +1,105 @@ +//=== StringChecker.cpp -------------------------------------------*- C++ -*--// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This file implements the modeling of the std::basic_string type. +// This involves checking preconditions of the operations and applying the +// effects of the operations, e.g. their post-conditions. +// +//===----------------------------------------------------------------------===// + +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" + +using namespace clang; +using namespace ento; + +namespace { +class StringChecker : public Checker<check::PreCall> { + BugType BT_Null{this, "Dereference of null pointer", categories::LogicError}; + mutable const FunctionDecl *StringConstCharPtrCtor = nullptr; + mutable CanQualType SizeTypeTy; + const CallDescription TwoParamStdStringCtor = { + {"std", "basic_string", "basic_string"}, 2, 2}; + + bool isCharToStringCtor(const CallEvent &Call, const ASTContext &ACtx) const; + +public: + void checkPreCall(const CallEvent &Call, CheckerContext &C) const; +}; + +bool StringChecker::isCharToStringCtor(const CallEvent &Call, + const ASTContext &ACtx) const { + if (!TwoParamStdStringCtor.matches(Call)) + return false; + const auto *FD = dyn_cast<FunctionDecl>(Call.getDecl()); + assert(FD); + + // See if we already cached it. + if (StringConstCharPtrCtor && StringConstCharPtrCtor == FD) + return true; + + // Verify that the parameters have the expected types: + // - arg 1: `const CharT *` + // - arg 2: some allocator - which is definately not `size_t`. + const QualType Arg1Ty = Call.getArgExpr(0)->getType().getCanonicalType(); + const QualType Arg2Ty = Call.getArgExpr(1)->getType().getCanonicalType(); + + if (!Arg1Ty->isPointerType()) + return false; + + // It makes sure that we don't select the `string(const char* p, size_t len)` + // overload accidentally. + if (Arg2Ty.getCanonicalType() == ACtx.getSizeType()) + return false; + + StringConstCharPtrCtor = FD; // Cache the decl of the right overload. + return true; +} + +void StringChecker::checkPreCall(const CallEvent &Call, + CheckerContext &C) const { + if (!isCharToStringCtor(Call, C.getASTContext())) + return; + const auto Param = Call.getArgSVal(0).getAs<Loc>(); + if (!Param.hasValue()) + return; + + // We managed to constrain the parameter to non-null. + ProgramStateRef NotNull, Null; + std::tie(NotNull, Null) = C.getState()->assume(*Param); + + if (NotNull) { + const auto Callback = [Param](PathSensitiveBugReport &BR) -> std::string { + return BR.isInteresting(*Param) ? "Assuming the pointer is not null." + : ""; + }; + + // Emit note only if this operation constrained the pointer to be null. + C.addTransition(NotNull, Null ? C.getNoteTag(Callback) : nullptr); + return; + } + + // We found a path on which the parameter is NULL. + if (ExplodedNode *N = C.generateErrorNode(C.getState())) { + auto R = std::make_unique<PathSensitiveBugReport>( + BT_Null, "The parameter must not be null", N); + bugreporter::trackExpressionValue(N, Call.getArgExpr(0), *R); + C.emitReport(std::move(R)); + } +} + +} // end anonymous namespace + +void ento::registerStringChecker(CheckerManager &Mgr) { + Mgr.registerChecker<StringChecker>(); +} + +bool ento::shouldRegisterStringChecker(const CheckerManager &) { return true; } diff --git a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp index 381334de068e..2244cdb96d4f 100644 --- a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp @@ -110,7 +110,7 @@ void UnixAPIMisuseChecker::checkPreStmt(const CallExpr *CE, // Don't treat functions in namespaces with the same name a Unix function // as a call to the Unix function. const DeclContext *NamespaceCtx = FD->getEnclosingNamespaceContext(); - if (NamespaceCtx && isa<NamespaceDecl>(NamespaceCtx)) + if (isa_and_nonnull<NamespaceDecl>(NamespaceCtx)) return; StringRef FName = C.getCalleeName(FD); @@ -466,7 +466,7 @@ void UnixAPIPortabilityChecker::checkPreStmt(const CallExpr *CE, // Don't treat functions in namespaces with the same name a Unix function // as a call to the Unix function. const DeclContext *NamespaceCtx = FD->getEnclosingNamespaceContext(); - if (NamespaceCtx && isa<NamespaceDecl>(NamespaceCtx)) + if (isa_and_nonnull<NamespaceDecl>(NamespaceCtx)) return; StringRef FName = C.getCalleeName(FD); diff --git a/clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp index dde5912b6d6e..60da4fca12e6 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp @@ -15,6 +15,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/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" @@ -126,15 +127,15 @@ void ValistChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { if (!Call.isGlobalCFunction()) return; - if (Call.isCalled(VaStart)) + if (VaStart.matches(Call)) checkVAListStartCall(Call, C, false); - else if (Call.isCalled(VaCopy)) + else if (VaCopy.matches(Call)) checkVAListStartCall(Call, C, true); - else if (Call.isCalled(VaEnd)) + else if (VaEnd.matches(Call)) checkVAListEndCall(Call, C); else { for (auto FuncInfo : VAListAccepters) { - if (!Call.isCalled(FuncInfo.Func)) + if (!FuncInfo.Func.matches(Call)) continue; bool Symbolic; const MemRegion *VAList = diff --git a/clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp index 8f147026ae19..04e6603b4cbe 100644 --- a/clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp @@ -9,7 +9,7 @@ // This file defines vfork checker which checks for dangerous uses of vfork. // Vforked process shares memory (including stack) with parent so it's // range of actions is significantly limited: can't write variables, -// can't call functions not in whitelist, etc. For more details, see +// can't call functions not in the allowed list, etc. For more details, see // http://man7.org/linux/man-pages/man2/vfork.2.html // // This checker checks for prohibited constructs in vforked process. @@ -44,13 +44,14 @@ namespace { class VforkChecker : public Checker<check::PreCall, check::PostCall, check::Bind, check::PreStmt<ReturnStmt>> { mutable std::unique_ptr<BuiltinBug> BT; - mutable llvm::SmallSet<const IdentifierInfo *, 10> VforkWhitelist; + mutable llvm::SmallSet<const IdentifierInfo *, 10> VforkAllowlist; mutable const IdentifierInfo *II_vfork; static bool isChildProcess(const ProgramStateRef State); bool isVforkCall(const Decl *D, CheckerContext &C) const; - bool isCallWhitelisted(const IdentifierInfo *II, CheckerContext &C) const; + bool isCallExplicitelyAllowed(const IdentifierInfo *II, + CheckerContext &C) const; void reportBug(const char *What, CheckerContext &C, const char *Details = nullptr) const; @@ -93,9 +94,9 @@ bool VforkChecker::isVforkCall(const Decl *D, CheckerContext &C) const { } // Returns true iff ok to call function after successful vfork. -bool VforkChecker::isCallWhitelisted(const IdentifierInfo *II, - CheckerContext &C) const { - if (VforkWhitelist.empty()) { +bool VforkChecker::isCallExplicitelyAllowed(const IdentifierInfo *II, + CheckerContext &C) const { + if (VforkAllowlist.empty()) { // According to manpage. const char *ids[] = { "_Exit", @@ -112,10 +113,10 @@ bool VforkChecker::isCallWhitelisted(const IdentifierInfo *II, ASTContext &AC = C.getASTContext(); for (const char **id = ids; *id; ++id) - VforkWhitelist.insert(&AC.Idents.get(*id)); + VforkAllowlist.insert(&AC.Idents.get(*id)); } - return VforkWhitelist.count(II); + return VforkAllowlist.count(II); } void VforkChecker::reportBug(const char *What, CheckerContext &C, @@ -179,12 +180,13 @@ void VforkChecker::checkPostCall(const CallEvent &Call, C.addTransition(ChildState); } -// Prohibit calls to non-whitelist functions in child process. +// Prohibit calls to functions in child process which are not explicitly +// allowed. void VforkChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { ProgramStateRef State = C.getState(); - if (isChildProcess(State) - && !isCallWhitelisted(Call.getCalleeIdentifier(), C)) + if (isChildProcess(State) && + !isCallExplicitelyAllowed(Call.getCalleeIdentifier(), C)) reportBug("This function call", C); } diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h index ed4577755457..ec6a7144fa45 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h @@ -29,7 +29,7 @@ class Expr; /// values). /// /// For more context see Static Analyzer checkers documentation - specifically -/// webkit.UncountedCallArgsChecker checker. Whitelist of transformations: +/// webkit.UncountedCallArgsChecker checker. Allowed list of transformations: /// - constructors of ref-counted types (including factory methods) /// - getters of ref-counted types /// - member overloaded operators diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp index d70bd9489d2c..e6d0948f71bb 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp @@ -68,8 +68,7 @@ public: if (auto *F = CE->getDirectCallee()) { // Skip the first argument for overloaded member operators (e. g. lambda // or std::function call operator). - unsigned ArgIdx = - isa<CXXOperatorCallExpr>(CE) && dyn_cast_or_null<CXXMethodDecl>(F); + unsigned ArgIdx = isa<CXXOperatorCallExpr>(CE) && isa_and_nonnull<CXXMethodDecl>(F); for (auto P = F->param_begin(); // FIXME: Also check variadic function parameters. diff --git a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp new file mode 100644 index 000000000000..d7d573cd2d3b --- /dev/null +++ b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp @@ -0,0 +1,280 @@ +//== InvalidPtrChecker.cpp ------------------------------------- -*- C++ -*--=// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This file defines InvalidPtrChecker which finds usages of possibly +// invalidated pointer. +// CERT SEI Rules ENV31-C and ENV34-C +// For more information see: +// https://wiki.sei.cmu.edu/confluence/x/8tYxBQ +// https://wiki.sei.cmu.edu/confluence/x/5NUxBQ +//===----------------------------------------------------------------------===// + +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" + +using namespace clang; +using namespace ento; + +namespace { + +class InvalidPtrChecker + : public Checker<check::Location, check::BeginFunction, check::PostCall> { +private: + BugType BT{this, "Use of invalidated pointer", categories::MemoryError}; + + void EnvpInvalidatingCall(const CallEvent &Call, CheckerContext &C) const; + + using HandlerFn = void (InvalidPtrChecker::*)(const CallEvent &Call, + CheckerContext &C) const; + + // SEI CERT ENV31-C + const CallDescriptionMap<HandlerFn> EnvpInvalidatingFunctions = { + {{"setenv", 3}, &InvalidPtrChecker::EnvpInvalidatingCall}, + {{"unsetenv", 1}, &InvalidPtrChecker::EnvpInvalidatingCall}, + {{"putenv", 1}, &InvalidPtrChecker::EnvpInvalidatingCall}, + {{"_putenv_s", 2}, &InvalidPtrChecker::EnvpInvalidatingCall}, + {{"_wputenv_s", 2}, &InvalidPtrChecker::EnvpInvalidatingCall}, + }; + + void postPreviousReturnInvalidatingCall(const CallEvent &Call, + CheckerContext &C) const; + + // SEI CERT ENV34-C + const CallDescriptionMap<HandlerFn> PreviousCallInvalidatingFunctions = { + {{"getenv", 1}, &InvalidPtrChecker::postPreviousReturnInvalidatingCall}, + {{"setlocale", 2}, + &InvalidPtrChecker::postPreviousReturnInvalidatingCall}, + {{"strerror", 1}, &InvalidPtrChecker::postPreviousReturnInvalidatingCall}, + {{"localeconv", 0}, + &InvalidPtrChecker::postPreviousReturnInvalidatingCall}, + {{"asctime", 1}, &InvalidPtrChecker::postPreviousReturnInvalidatingCall}, + }; + +public: + // Obtain the environment pointer from 'main()' (if present). + void checkBeginFunction(CheckerContext &C) const; + + // Handle functions in EnvpInvalidatingFunctions, that invalidate environment + // pointer from 'main()' + // Handle functions in PreviousCallInvalidatingFunctions. + // Also, check if invalidated region is passed to a + // conservatively evaluated function call as an argument. + void checkPostCall(const CallEvent &Call, CheckerContext &C) const; + + // Check if invalidated region is being dereferenced. + void checkLocation(SVal l, bool isLoad, const Stmt *S, + CheckerContext &C) const; +}; + +} // namespace + +// Set of memory regions that were invalidated +REGISTER_SET_WITH_PROGRAMSTATE(InvalidMemoryRegions, const MemRegion *) + +// Stores the region of the environment pointer of 'main' (if present). +// Note: This pointer has type 'const MemRegion *', however the trait is only +// specialized to 'const void*' and 'void*' +REGISTER_TRAIT_WITH_PROGRAMSTATE(EnvPtrRegion, const void *) + +// Stores key-value pairs, where key is function declaration and value is +// pointer to memory region returned by previous call of this function +REGISTER_MAP_WITH_PROGRAMSTATE(PreviousCallResultMap, const FunctionDecl *, + const MemRegion *) + +void InvalidPtrChecker::EnvpInvalidatingCall(const CallEvent &Call, + CheckerContext &C) const { + StringRef FunctionName = Call.getCalleeIdentifier()->getName(); + ProgramStateRef State = C.getState(); + const auto *Reg = State->get<EnvPtrRegion>(); + if (!Reg) + return; + const auto *SymbolicEnvPtrRegion = + reinterpret_cast<const MemRegion *>(const_cast<const void *>(Reg)); + + State = State->add<InvalidMemoryRegions>(SymbolicEnvPtrRegion); + + const NoteTag *Note = + C.getNoteTag([SymbolicEnvPtrRegion, FunctionName]( + PathSensitiveBugReport &BR, llvm::raw_ostream &Out) { + if (!BR.isInteresting(SymbolicEnvPtrRegion)) + return; + Out << '\'' << FunctionName + << "' call may invalidate the environment parameter of 'main'"; + }); + + C.addTransition(State, Note); +} + +void InvalidPtrChecker::postPreviousReturnInvalidatingCall( + const CallEvent &Call, CheckerContext &C) const { + ProgramStateRef State = C.getState(); + + const NoteTag *Note = nullptr; + const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(Call.getDecl()); + // Invalidate the region of the previously returned pointer - if there was + // one. + if (const MemRegion *const *Reg = State->get<PreviousCallResultMap>(FD)) { + const MemRegion *PrevReg = *Reg; + State = State->add<InvalidMemoryRegions>(PrevReg); + Note = C.getNoteTag([PrevReg, FD](PathSensitiveBugReport &BR, + llvm::raw_ostream &Out) { + if (!BR.isInteresting(PrevReg)) + return; + Out << '\''; + FD->getNameForDiagnostic(Out, FD->getASTContext().getLangOpts(), true); + Out << "' call may invalidate the the result of the previous " << '\''; + FD->getNameForDiagnostic(Out, FD->getASTContext().getLangOpts(), true); + Out << '\''; + }); + } + + const LocationContext *LCtx = C.getLocationContext(); + const auto *CE = cast<CallExpr>(Call.getOriginExpr()); + + // Function call will return a pointer to the new symbolic region. + DefinedOrUnknownSVal RetVal = C.getSValBuilder().conjureSymbolVal( + CE, LCtx, CE->getType(), C.blockCount()); + State = State->BindExpr(CE, LCtx, RetVal); + + // Remember to this region. + const auto *SymRegOfRetVal = cast<SymbolicRegion>(RetVal.getAsRegion()); + const MemRegion *MR = + const_cast<MemRegion *>(SymRegOfRetVal->getBaseRegion()); + State = State->set<PreviousCallResultMap>(FD, MR); + + ExplodedNode *Node = C.addTransition(State, Note); + const NoteTag *PreviousCallNote = + C.getNoteTag([MR](PathSensitiveBugReport &BR, llvm::raw_ostream &Out) { + if (!BR.isInteresting(MR)) + return; + Out << '\'' << "'previous function call was here" << '\''; + }); + + C.addTransition(State, Node, PreviousCallNote); +} + +// TODO: This seems really ugly. Simplify this. +static const MemRegion *findInvalidatedSymbolicBase(ProgramStateRef State, + const MemRegion *Reg) { + while (Reg) { + if (State->contains<InvalidMemoryRegions>(Reg)) + return Reg; + const auto *SymBase = Reg->getSymbolicBase(); + if (!SymBase) + break; + const auto *SRV = dyn_cast<SymbolRegionValue>(SymBase->getSymbol()); + if (!SRV) + break; + Reg = SRV->getRegion(); + if (const auto *VarReg = dyn_cast<VarRegion>(SRV->getRegion())) + Reg = VarReg; + } + return nullptr; +} + +// Handle functions in EnvpInvalidatingFunctions, that invalidate environment +// pointer from 'main()' Also, check if invalidated region is passed to a +// function call as an argument. +void InvalidPtrChecker::checkPostCall(const CallEvent &Call, + CheckerContext &C) const { + // Check if function invalidates 'envp' argument of 'main' + if (const auto *Handler = EnvpInvalidatingFunctions.lookup(Call)) + (this->**Handler)(Call, C); + + // Check if function invalidates the result of previous call + if (const auto *Handler = PreviousCallInvalidatingFunctions.lookup(Call)) + (this->**Handler)(Call, C); + + // Check if one of the arguments of the function call is invalidated + + // If call was inlined, don't report invalidated argument + if (C.wasInlined) + return; + + ProgramStateRef State = C.getState(); + + for (unsigned I = 0, NumArgs = Call.getNumArgs(); I < NumArgs; ++I) { + + if (const auto *SR = dyn_cast_or_null<SymbolicRegion>( + Call.getArgSVal(I).getAsRegion())) { + if (const MemRegion *InvalidatedSymbolicBase = + findInvalidatedSymbolicBase(State, SR)) { + ExplodedNode *ErrorNode = C.generateNonFatalErrorNode(); + if (!ErrorNode) + return; + + SmallString<256> Msg; + llvm::raw_svector_ostream Out(Msg); + Out << "use of invalidated pointer '"; + Call.getArgExpr(I)->printPretty(Out, /*Helper=*/nullptr, + C.getASTContext().getPrintingPolicy()); + Out << "' in a function call"; + + auto Report = + std::make_unique<PathSensitiveBugReport>(BT, Out.str(), ErrorNode); + Report->markInteresting(InvalidatedSymbolicBase); + Report->addRange(Call.getArgSourceRange(I)); + C.emitReport(std::move(Report)); + } + } + } +} + +// Obtain the environment pointer from 'main()', if present. +void InvalidPtrChecker::checkBeginFunction(CheckerContext &C) const { + if (!C.inTopFrame()) + return; + + const auto *FD = dyn_cast<FunctionDecl>(C.getLocationContext()->getDecl()); + if (!FD || FD->param_size() != 3 || !FD->isMain()) + return; + + ProgramStateRef State = C.getState(); + const MemRegion *EnvpReg = + State->getRegion(FD->parameters()[2], C.getLocationContext()); + + // Save the memory region pointed by the environment pointer parameter of + // 'main'. + State = State->set<EnvPtrRegion>( + reinterpret_cast<void *>(const_cast<MemRegion *>(EnvpReg))); + C.addTransition(State); +} + +// Check if invalidated region is being dereferenced. +void InvalidPtrChecker::checkLocation(SVal Loc, bool isLoad, const Stmt *S, + CheckerContext &C) const { + ProgramStateRef State = C.getState(); + + // Ignore memory operations involving 'non-invalidated' locations. + const MemRegion *InvalidatedSymbolicBase = + findInvalidatedSymbolicBase(State, Loc.getAsRegion()); + if (!InvalidatedSymbolicBase) + return; + + ExplodedNode *ErrorNode = C.generateNonFatalErrorNode(); + if (!ErrorNode) + return; + + auto Report = std::make_unique<PathSensitiveBugReport>( + BT, "dereferencing an invalid pointer", ErrorNode); + Report->markInteresting(InvalidatedSymbolicBase); + C.emitReport(std::move(Report)); +} + +void ento::registerInvalidPtrChecker(CheckerManager &Mgr) { + Mgr.registerChecker<InvalidPtrChecker>(); +} + +bool ento::shouldRegisterInvalidPtrChecker(const CheckerManager &) { + return true; +} diff --git a/clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp index 1c67bbd77ec8..ed3bdafad084 100644 --- a/clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.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/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h" @@ -38,7 +39,7 @@ public: void PutenvWithAutoChecker::checkPostCall(const CallEvent &Call, CheckerContext &C) const { - if (!Call.isCalled(Putenv)) + if (!Putenv.matches(Call)) return; SVal ArgV = Call.getArgSVal(0); |