Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h +++ clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h @@ -14,6 +14,7 @@ #ifndef LLVM_CLANG_STATICANALYZER_CORE_BUGREPORTER_BUGREPORTERVISITORS_H #define LLVM_CLANG_STATICANALYZER_CORE_BUGREPORTER_BUGREPORTERVISITORS_H +#include "clang/Analysis/Analyses/Dominators.h" #include "clang/Analysis/ProgramPoint.h" #include "clang/Basic/LLVM.h" #include "clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h" @@ -159,6 +160,35 @@ static const Expr *getNilReceiver(const Stmt *S, const ExplodedNode *N); }; +/// Tracks the expressions that are a control dependency of the node that was +/// supplied to the constructor. +/// For example: +/// +/// cond = 1; +/// if (cond) +/// 10 / 0; +/// +/// An error is emitted at line 3. This visitor realizes that the branch +/// on line 2 is a control dependency of line 3, and tracks it's condition via +/// trackExpressionValue(). +class TrackControlDependencyCondBRVisitor final : public BugReporterVisitor { + const ExplodedNode *Origin; + CFGControlDependencyTree ControlDepTree; + llvm::SmallSet VisitedBlocks; + +public: + TrackControlDependencyCondBRVisitor(const ExplodedNode *O); + + void Profile(llvm::FoldingSetNodeID &ID) const override { + static int x = 0; + ID.AddPointer(&x); + } + + std::shared_ptr VisitNode(const ExplodedNode *N, + BugReporterContext &BRC, + BugReport &BR) override; +}; + /// Visitor that tries to report interesting diagnostics from conditions. class ConditionBRVisitor final : public BugReporterVisitor { // FIXME: constexpr initialization isn't supported by MSVC2013. Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -180,7 +180,7 @@ } //===----------------------------------------------------------------------===// -// Definitions for bug reporter visitors. +// Implementation of BugReporterVisitor. //===----------------------------------------------------------------------===// std::shared_ptr @@ -677,6 +677,10 @@ } }; +} // end of anonymous namespace + +namespace { + /// Suppress null-pointer-dereference bugs where dereferenced null was returned /// the macro. class MacroNullReturnSuppressionVisitor final : public BugReporterVisitor { @@ -765,6 +769,10 @@ } }; +} // end of anonymous namespace + +namespace { + /// Emits an extra note at the return statement of an interesting stack frame. /// /// The returned value is marked as an interesting value, and if it's null, @@ -1040,7 +1048,11 @@ } }; -} // namespace +} // end of anonymous namespace + +//===----------------------------------------------------------------------===// +// Implementation of FindLastStoreBRVisitor. +//===----------------------------------------------------------------------===// void FindLastStoreBRVisitor::Profile(llvm::FoldingSetNodeID &ID) const { static int tag = 0; @@ -1350,6 +1362,10 @@ return std::make_shared(L, os.str()); } +//===----------------------------------------------------------------------===// +// Implementation of TrackConstraintBRVisitor. +//===----------------------------------------------------------------------===// + void TrackConstraintBRVisitor::Profile(llvm::FoldingSetNodeID &ID) const { static int tag = 0; ID.AddPointer(&tag); @@ -1422,6 +1438,10 @@ return nullptr; } +//===----------------------------------------------------------------------===// +// Implementation of SuppressInlineDefensiveChecksVisitor. +//===----------------------------------------------------------------------===// + SuppressInlineDefensiveChecksVisitor:: SuppressInlineDefensiveChecksVisitor(DefinedSVal Value, const ExplodedNode *N) : V(Value) { @@ -1517,6 +1537,10 @@ return nullptr; } +//===----------------------------------------------------------------------===// +// Implementation of trackExpressionValue. +//===----------------------------------------------------------------------===// + static const MemRegion *getLocationRegionIfReference(const Expr *E, const ExplodedNode *N) { if (const auto *DR = dyn_cast(E)) { @@ -1627,6 +1651,9 @@ if (!LVNode) return false; + report.addVisitor(llvm::make_unique( + InputNode)); + ProgramStateRef LVState = LVNode->getState(); // The message send could be nil due to the receiver being nil. @@ -1736,6 +1763,10 @@ return true; } +//===----------------------------------------------------------------------===// +// Implementation of NulReceiverBRVisitor. +//===----------------------------------------------------------------------===// + const Expr *NilReceiverBRVisitor::getNilReceiver(const Stmt *S, const ExplodedNode *N) { const auto *ME = dyn_cast(S); @@ -1786,6 +1817,105 @@ return std::make_shared(L, OS.str()); } +//===----------------------------------------------------------------------===// +// Implementation of TrackControlDependencyCondBRVisitor. +//===----------------------------------------------------------------------===// + +TrackControlDependencyCondBRVisitor::TrackControlDependencyCondBRVisitor( + const ExplodedNode *O) + : Origin(O), ControlDepTree(&O->getCFG()) {} + +static CFGBlock *GetRelevantBlock(const ExplodedNode *Node) { + if (auto SP = Node->getLocationAs()) { + const Stmt *S = SP->getStmt(); + assert(S); + + return const_cast(Node->getLocationContext() + ->getAnalysisDeclContext()->getCFGStmtMap()->getBlock(S)); + } + + return nullptr; +} + +static const Expr *getTerminatorCondition(CFGBlock *B) { + const Stmt *S = B->getTerminatorStmt(); + assert(S); + + switch (S->getStmtClass()) { + case Stmt::IfStmtClass: + return cast(S)->getCond(); + + case Stmt::SwitchStmtClass: + return cast(S)->getCond(); + + case Stmt::ForStmtClass: + return cast(S)->getCond(); + + case Stmt::DoStmtClass: + return cast(S)->getCond(); + + case Stmt::WhileStmtClass: + return cast(S)->getCond(); + + case Stmt::CXXForRangeStmtClass: + return cast(S)->getCond(); + + case Stmt::ObjCForCollectionStmtClass: + return cast(S)->getCollection(); + + case Stmt::ConditionalOperatorClass: + return cast(S)->getCond(); + + case Stmt::BinaryOperatorClass: + // FIXME: Retrive the parent statement and call the function recursively. + return cast(S); + + case Stmt::CXXCatchStmtClass: + // FIXME: Track the thrown object. + break; + + case Stmt::GotoStmtClass: + case Stmt::BreakStmtClass: + case Stmt::ContinueStmtClass: + case Stmt::ReturnStmtClass: + // There are no conditions for these terminator statements. + break; + + default: + llvm_unreachable("Unknown terminator statement!"); + } + return nullptr; +} + +std::shared_ptr +TrackControlDependencyCondBRVisitor::VisitNode(const ExplodedNode *N, + BugReporterContext &BRC, + BugReport &BR) { + // If N is originaring from a different function than the node of interest, + // we can't reason about control dependencies. + if (&Origin->getCFG() != &N->getCFG()) + return nullptr; + + CFGBlock *NB = GetRelevantBlock(N); + if (!VisitedBlocks.insert(NB).second) + return nullptr; + + CFGBlock *OriginB = GetRelevantBlock(Origin); + if (!OriginB || !NB) + return nullptr; + + if (ControlDepTree.isControlDependency(OriginB, NB)) + if (const Expr *Condition = getTerminatorCondition(NB)) + bugreporter::trackExpressionValue( + N, Condition, BR, /*EnableNullFPSuppression=*/false); + + return nullptr; +} + +//===----------------------------------------------------------------------===// +// Implementation of FindLastStoreBRVisitor. +//===----------------------------------------------------------------------===// + // Registers every VarDecl inside a Stmt with a last store visitor. void FindLastStoreBRVisitor::registerStatementVarDecls(BugReport &BR, const Stmt *S, @@ -2239,6 +2369,10 @@ Piece->getString() == GenericFalseMessage; } +//===----------------------------------------------------------------------===// +// Implementation of LikelyFalsePositiveSuppressionBRVisitor. +//===----------------------------------------------------------------------===// + void LikelyFalsePositiveSuppressionBRVisitor::finalizeVisitor( BugReporterContext &BRC, const ExplodedNode *N, BugReport &BR) { // Here we suppress false positives coming from system headers. This list is @@ -2321,6 +2455,10 @@ } } +//===----------------------------------------------------------------------===// +// Implementation of UndefOrNullArgVisitor. +//===----------------------------------------------------------------------===// + std::shared_ptr UndefOrNullArgVisitor::VisitNode(const ExplodedNode *N, BugReporterContext &BRC, BugReport &BR) { @@ -2371,6 +2509,10 @@ return nullptr; } +//===----------------------------------------------------------------------===// +// Implementation of CXXSelfAssignmentBRVisitor. +//===----------------------------------------------------------------------===// + std::shared_ptr CXXSelfAssignmentBRVisitor::VisitNode(const ExplodedNode *Succ, BugReporterContext &BRC, BugReport &) { @@ -2423,6 +2565,9 @@ return std::move(Piece); } +//===----------------------------------------------------------------------===// +// Implementation of FalsePositiveRefutationBRVisitor. +//===----------------------------------------------------------------------===// FalsePositiveRefutationBRVisitor::FalsePositiveRefutationBRVisitor() : Constraints(ConstraintRangeTy::Factory().getEmptyMap()) {} @@ -2483,6 +2628,16 @@ return nullptr; } +void FalsePositiveRefutationBRVisitor::Profile( + llvm::FoldingSetNodeID &ID) const { + static int Tag = 0; + ID.AddPointer(&Tag); +} + +//===----------------------------------------------------------------------===// +// Implementation of TagVisitor. +//===----------------------------------------------------------------------===// + int NoteTag::Kind = 0; void TagVisitor::Profile(llvm::FoldingSetNodeID &ID) const { @@ -2508,9 +2663,3 @@ return nullptr; } - -void FalsePositiveRefutationBRVisitor::Profile( - llvm::FoldingSetNodeID &ID) const { - static int Tag = 0; - ID.AddPointer(&Tag); -} Index: clang/test/Analysis/Inputs/expected-plists/cxx-for-range.cpp.plist =================================================================== --- clang/test/Analysis/Inputs/expected-plists/cxx-for-range.cpp.plist +++ clang/test/Analysis/Inputs/expected-plists/cxx-for-range.cpp.plist @@ -3,7 +3,7 @@ clang_version -clang version 8.0.0 +clang version 9.0.0 diagnostics @@ -554,6 +554,69 @@ file0 + end + + + line88 + col20 + file0 + + + line88 + col23 + file0 + + + + + + + kindevent + location + + line88 + col20 + file0 + + ranges + + + + line88 + col20 + file0 + + + line88 + col23 + file0 + + + + depth0 + extended_message + Passing the value 1 via 1st parameter 'fail' + message + Passing the value 1 via 1st parameter 'fail' + + + kindcontrol + edges + + + start + + + line88 + col20 + file0 + + + line88 + col23 + file0 + + end @@ -1954,7 +2017,6 @@ files - /clang/test/Analysis/cxx-for-range.cpp Index: clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist =================================================================== --- clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist +++ clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist @@ -11601,6 +11601,35 @@ + + kindevent + location + + line459 + col5 + file0 + + ranges + + + + line459 + col5 + file0 + + + line459 + col13 + file0 + + + + depth0 + extended_message + The value 0 is assigned to 'first' + message + The value 0 is assigned to 'first' + kindcontrol edges Index: clang/test/Analysis/Inputs/expected-plists/retain-release.m.objc.plist =================================================================== --- clang/test/Analysis/Inputs/expected-plists/retain-release.m.objc.plist +++ clang/test/Analysis/Inputs/expected-plists/retain-release.m.objc.plist @@ -1884,6 +1884,35 @@ path + + kindevent + location + + line439 + col3 + file0 + + ranges + + + + line439 + col3 + file0 + + + line439 + col16 + file0 + + + + depth0 + extended_message + 'date' initialized here + message + 'date' initialized here + kindcontrol edges @@ -9277,6 +9306,69 @@ file0 + end + + + line731 + col3 + file0 + + + line731 + col10 + file0 + + + + + + + kindevent + location + + line731 + col3 + file0 + + ranges + + + + line731 + col3 + file0 + + + line731 + col16 + file0 + + + + depth0 + extended_message + 'name' initialized here + message + 'name' initialized here + + + kindcontrol + edges + + + start + + + line731 + col3 + file0 + + + line731 + col10 + file0 + + end @@ -26107,7 +26199,6 @@ files - /Volumes/Transcend/code/monorepo/llvm-project/clang/test/Analysis/retain-release.m Index: clang/test/Analysis/Inputs/expected-plists/retain-release.m.objcpp.plist =================================================================== --- clang/test/Analysis/Inputs/expected-plists/retain-release.m.objcpp.plist +++ clang/test/Analysis/Inputs/expected-plists/retain-release.m.objcpp.plist @@ -1884,6 +1884,35 @@ path + + kindevent + location + + line439 + col3 + file0 + + ranges + + + + line439 + col3 + file0 + + + line439 + col16 + file0 + + + + depth0 + extended_message + 'date' initialized here + message + 'date' initialized here + kindcontrol edges @@ -9277,6 +9306,69 @@ file0 + end + + + line731 + col3 + file0 + + + line731 + col10 + file0 + + + + + + + kindevent + location + + line731 + col3 + file0 + + ranges + + + + line731 + col3 + file0 + + + line731 + col16 + file0 + + + + depth0 + extended_message + 'name' initialized here + message + 'name' initialized here + + + kindcontrol + edges + + + start + + + line731 + col3 + file0 + + + line731 + col10 + file0 + + end @@ -26176,7 +26268,6 @@ files - /Volumes/Transcend/code/monorepo/llvm-project/clang/test/Analysis/retain-release.m Index: clang/test/Analysis/Inputs/expected-plists/unix-fns.c.plist =================================================================== --- clang/test/Analysis/Inputs/expected-plists/unix-fns.c.plist +++ clang/test/Analysis/Inputs/expected-plists/unix-fns.c.plist @@ -1594,6 +1594,35 @@ + + kindevent + location + + line223 + col24 + file0 + + ranges + + + + line223 + col24 + file0 + + + line227 + col3 + file0 + + + + depth0 + extended_message + 'q' + message + 'q' + kindevent location @@ -3015,7 +3044,6 @@ files - /home/szelethus/Documents/analyzer_opts/clang/test/Analysis/unix-fns.c Index: clang/test/Analysis/diagnostics/Inputs/expected-plists/undef-value-param.m.plist =================================================================== --- clang/test/Analysis/diagnostics/Inputs/expected-plists/undef-value-param.m.plist +++ clang/test/Analysis/diagnostics/Inputs/expected-plists/undef-value-param.m.plist @@ -3,7 +3,7 @@ clang_version -clang version 8.0.0 +clang version 9.0.0 diagnostics @@ -138,6 +138,69 @@ file0 + end + + + line55 + col5 + file0 + + + line55 + col21 + file0 + + + + + + + kindevent + location + + line55 + col29 + file0 + + ranges + + + + line55 + col29 + file0 + + + line55 + col53 + file0 + + + + depth1 + extended_message + Value assigned to 'err' + message + Value assigned to 'err' + + + kindcontrol + edges + + + start + + + line55 + col5 + file0 + + + line55 + col21 + file0 + + end @@ -1021,7 +1084,6 @@ files - /clang/test/Analysis/diagnostics/undef-value-param.m Index: clang/test/Analysis/diagnostics/no-store-func-path-notes.m =================================================================== --- clang/test/Analysis/diagnostics/no-store-func-path-notes.m +++ clang/test/Analysis/diagnostics/no-store-func-path-notes.m @@ -15,16 +15,19 @@ return 0; } return 1; // expected-note{{Returning without writing to '*var'}} + // expected-note@-1{{Returning the value 1}} } @end int foo(I *i) { - int x; //expected-note{{'x' declared without an initial value}} - int out = [i initVar:&x param:0]; //expected-note{{Calling 'initVar:param:'}} - //expected-note@-1{{Returning from 'initVar:param:'}} + int x; // expected-note{{'x' declared without an initial value}} + int out = [i initVar:&x param:0]; // expected-note{{Calling 'initVar:param:'}} + // expected-note@-1{{Returning from 'initVar:param:'}} + // expected-note@-2{{Passing the value 0 via 2nd parameter 'param'}} + // expected-note@-3{{'out' initialized to 1}} if (out) // expected-note{{Taking true branch}} - return x; //expected-warning{{Undefined or garbage value returned to caller}} - //expected-note@-1{{Undefined or garbage value returned to caller}} + return x; // expected-warning{{Undefined or garbage value returned to caller}} + // expected-note@-1{{Undefined or garbage value returned to caller}} return 0; } Index: clang/test/Analysis/diagnostics/undef-value-param.m =================================================================== --- clang/test/Analysis/diagnostics/undef-value-param.m +++ clang/test/Analysis/diagnostics/undef-value-param.m @@ -52,7 +52,7 @@ static void CreateRef(SCDynamicStoreRef *storeRef, unsigned x) { unsigned err = 0; - SCDynamicStoreRef ref = anotherCreateRef(&err, x); + SCDynamicStoreRef ref = anotherCreateRef(&err, x); // expected-note{{Value assigned to 'err'}} if (err) { //expected-note@-1{{Assuming 'err' is not equal to 0}} //expected-note@-2{{Taking true branch}} Index: clang/test/Analysis/track-control-dependency-conditions.cpp =================================================================== --- /dev/null +++ clang/test/Analysis/track-control-dependency-conditions.cpp @@ -0,0 +1,48 @@ +// RUN: %clang_analyze_cc1 %s -verify \ +// RUN: -analyzer-output=text \ +// RUN: -analyzer-checker=core + +int flag; +bool coin(); + +void foo() { + flag = coin(); // expected-note 2{{Value assigned to 'flag'}} +} + +void example_1() { + int *x = 0; // expected-note{{'x' initialized to a null pointer value}} + flag = 1; + + foo(); // TODO: Add nodes here about flag's value being invalidated. + if (flag) // expected-note {{Taking false branch}} + // expected-note@-1{{Assuming 'flag' is 0}} + x = new int; + + foo(); // expected-note {{Calling 'foo'}} + // expected-note@-1{{Returning from 'foo'}} + + if (flag) // expected-note {{Taking true branch}} + // expected-note@-1{{Assuming 'flag' is not equal to 0}} + *x = 5; // expected-warning{{Dereference of null pointer}} + // expected-note@-1{{Dereference of null pointer}} +} + +void example_2() { + int *x = 0; + flag = 1; + + foo(); // TODO: Add nodes here about flag's value being invalidated. + if (flag) // expected-note {{Taking false branch}} + // expected-note@-1{{Assuming 'flag' is 0}} + x = new int; + + x = 0; // expected-note{{Null pointer value stored to 'x'}} + + foo(); // expected-note {{Calling 'foo'}} + // expected-note@-1{{Returning from 'foo'}} + + if (flag) // expected-note {{Taking true branch}} + // expected-note@-1{{Assuming 'flag' is not equal to 0}} + *x = 5; // expected-warning{{Dereference of null pointer}} + // expected-note@-1{{Dereference of null pointer}} +}