Szelethus updated this revision to Diff 205916.
Szelethus marked an inline comment as done.
Szelethus added a comment.
- Now using `CFGBlock::getTerminatorConditionExpr()`
- Uniqueing already tracked conditions as an (`Expr`, `ExplodedNode`) pair
instead of on `Expr`
- Add a `TODO:` about caching
- Add plenty of new test cases for good measure (examples from my latest mail
http://lists.llvm.org/pipermail/cfe-dev/2019-June/062613.html)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62883/new/
https://reviews.llvm.org/D62883
Files:
clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
clang/test/Analysis/analyzer-config.c
clang/test/Analysis/track-control-dependency-conditions.cpp
Index: clang/test/Analysis/track-control-dependency-conditions.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/track-control-dependency-conditions.cpp
@@ -0,0 +1,287 @@
+// RUN: %clang_analyze_cc1 %s -verify -DTRACKING_CONDITIONS \
+// RUN: -analyzer-config track-conditions=true \
+// RUN: -analyzer-output=text \
+// RUN: -analyzer-checker=core
+//
+// RUN: %clang_analyze_cc1 %s -verify \
+// RUN: -analyzer-output=text \
+// RUN: -analyzer-checker=core
+
+namespace example_1 {
+int flag;
+bool coin();
+
+void foo() {
+ flag = coin();
+#ifdef TRACKING_CONDITIONS
+ // expected-note@-2{{Value assigned to 'flag'}}
+#endif // TRACKING_CONDITIONS
+}
+
+void test() {
+ 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 {{Assuming 'flag' is 0}}
+ // expected-note@-1{{Taking false branch}}
+ x = new int;
+
+ foo();
+#ifdef TRACKING_CONDITIONS
+ // expected-note@-2{{Calling 'foo'}}
+ // expected-note@-3{{Returning from 'foo'}}
+#endif // TRACKING_CONDITIONS
+
+ if (flag) // expected-note {{Assuming 'flag' is not equal to 0}}
+ // expected-note@-1{{Taking true branch}}
+ *x = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note@-1{{Dereference of null pointer}}
+}
+} // end of namespace example_1
+
+namespace example_2 {
+int flag;
+bool coin();
+
+void foo() {
+ flag = coin();
+#ifdef TRACKING_CONDITIONS
+ // expected-note@-2{{Value assigned to 'flag'}}
+#endif // TRACKING_CONDITIONS
+}
+
+void test() {
+ int *x = 0;
+ flag = 1;
+
+ foo();
+ if (flag) // expected-note {{Assuming 'flag' is 0}}
+ // expected-note@-1{{Taking false branch}}
+ x = new int;
+
+ x = 0; // expected-note{{Null pointer value stored to 'x'}}
+
+ foo();
+#ifdef TRACKING_CONDITIONS
+ // expected-note@-2{{Calling 'foo'}}
+ // expected-note@-3{{Returning from 'foo'}}
+#endif // TRACKING_CONDITIONS
+
+ if (flag) // expected-note {{Assuming 'flag' is not equal to 0}}
+ // expected-note@-1{{Taking true branch}}
+ *x = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note@-1{{Dereference of null pointer}}
+}
+} // end of namespace example_2
+
+namespace global_variable_invalidation {
+int flag;
+bool coin();
+
+void foo() {
+ // coin() could write bar, do it's invalidated.
+ flag = coin();
+#ifdef TRACKING_CONDITIONS
+ // expected-note@-2{{Value assigned to 'flag'}}
+ // expected-note@-3{{Value assigned to 'bar'}}
+#endif // TRACKING_CONDITIONS
+}
+
+int bar;
+
+void test() {
+ int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
+ flag = 1;
+
+ foo();
+#ifdef TRACKING_CONDITIONS
+ // expected-note@-2{{Calling 'foo'}}
+ // expected-note@-3{{Returning from 'foo'}}
+#endif // TRACKING_CONDITIONS
+
+ if (bar) // expected-note {{Assuming 'bar' is not equal to 0}}
+ // expected-note@-1{{Taking true branch}}
+ if (flag) // expected-note {{Assuming 'flag' is not equal to 0}}
+ // expected-note@-1{{Taking true branch}}
+ *x = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note@-1{{Dereference of null pointer}}
+}
+} // end of namespace global_variable_invalidation
+
+namespace variable_declaration_in_condition {
+bool coin();
+
+bool foo() {
+ return coin();
+#ifdef TRACKING_CONDITIONS
+ // expected-note@-2{{Returning value}}
+#endif // TRACKING_CONDITIONS
+}
+
+int bar;
+
+void test() {
+ int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
+
+ if (int flag = foo())
+#ifdef TRACKING_CONDITIONS
+ // expected-note@-2{{Calling 'foo'}}
+ // expected-note@-3{{Returning from 'foo'}}
+ // expected-note@-4{{'flag' initialized here}}
+#endif // TRACKING_CONDITIONS
+ // expected-note@-6{{Assuming 'flag' is not equal to 0}}
+ // expected-note@-7{{Taking true branch}}
+ *x = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note@-1{{Dereference of null pointer}}
+}
+} // end of namespace variable_declaration_in_condition
+
+namespace conversion_to_bool {
+bool coin();
+
+struct ConvertsToBool {
+ operator bool() const { return coin(); }
+#ifdef TRACKING_CONDITIONS
+ // expected-note@-2{{Returning value}}
+#endif // TRACKING_CONDITIONS
+};
+
+void test() {
+ int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
+
+ if (ConvertsToBool())
+#ifdef TRACKING_CONDITIONS
+ // expected-note@-2 {{Calling 'ConvertsToBool::operator bool'}}
+ // expected-note@-3{{Returning from 'ConvertsToBool::operator bool'}}
+#endif // TRACKING_CONDITIONS
+ // expected-note@-5{{Assuming the condition is true}}
+ // expected-note@-6{{Taking true branch}}
+ *x = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note@-1{{Dereference of null pointer}}
+}
+
+} // end of namespace variable_declaration_in_condition
+
+namespace tracked_condition_is_only_initialized {
+int getInt();
+
+void f() {
+ int flag = getInt();
+#ifdef TRACKING_CONDITIONS
+ // expected-note@-2{{'flag' initialized here}}
+#endif // TRACKING_CONDITIONS
+ int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
+ if (flag) // expected-note{{Assuming 'flag' is not equal to 0}}
+ // expected-note@-1{{Taking true branch}}
+ *x = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note@-1{{Dereference of null pointer}}
+}
+} // end of namespace tracked_condition_is_only_initialized
+
+namespace tracked_condition_written_in_same_stackframe {
+int flag;
+int getInt();
+
+void f(int y) {
+ y = 1;
+ flag = y;
+#ifdef TRACKING_CONDITIONS
+ // expected-note@-3{{The value 1 is assigned to 'y'}}
+ // expected-note@-3{{The value 1 is assigned to 'flag'}}
+#endif // TRACKING_CONDITIONS
+ int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
+ if (flag) // expected-note{{'flag' is 1}}
+ // expected-note@-1{{Taking true branch}}
+ *x = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note@-1{{Dereference of null pointer}}
+}
+} // end of namespace tracked_condition_written_in_same_stackframe
+
+namespace tracked_condition_written_in_nested_stackframe {
+int flag;
+int getInt();
+
+void foo() {
+ int y;
+ y = 1;
+ flag = y;
+#ifdef TRACKING_CONDITIONS
+ // expected-note@-3{{The value 1 is assigned to 'y'}}
+ // expected-note@-3{{The value 1 is assigned to 'flag'}}
+#endif // TRACKING_CONDITIONS
+
+}
+
+void f(int y) {
+ int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
+ foo();
+#ifdef TRACKING_CONDITIONS
+ // expected-note@-2{{Calling 'foo'}}
+ // expected-note@-3{{Returning from 'foo'}}
+#endif // TRACKING_CONDITIONS
+ if (flag) // expected-note{{'flag' is 1}}
+ // expected-note@-1{{Taking true branch}}
+ *x = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note@-1{{Dereference of null pointer}}
+}
+} // end of namespace tracked_condition_written_in_nested_stackframe
+
+namespace collapse_point_not_in_condition {
+
+[[noreturn]] void halt();
+
+void assert(int b) {
+ if (!b)
+#ifdef TRACKING_CONDITIONS
+ // expected-note@-2{{Assuming 'b' is not equal to 0}}
+ // expected-note@-3{{Taking false branch}}
+#endif // TRACKING_CONDITIONS
+ halt();
+}
+
+void f(int flag) {
+ int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
+ assert(flag);
+#ifdef TRACKING_CONDITIONS
+ // expected-note@-2{{Calling 'assert'}}
+ // expected-note@-3{{Returning from 'assert'}}
+#endif // TRACKING_CONDITIONS
+ if (flag) // expected-note{{'flag' is not equal to 0}}
+ // expected-note@-1{{Taking true branch}}
+ *x = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note@-1{{Dereference of null pointer}}
+}
+
+} // end of namespace collapse_point_not_in_condition
+
+namespace unimportant_write_before_collapse_point {
+
+[[noreturn]] void halt();
+
+void assert(int b) {
+ if (!b)
+#ifdef TRACKING_CONDITIONS
+ // expected-note@-2{{Assuming 'b' is not equal to 0}}
+ // expected-note@-3{{Taking false branch}}
+#endif // TRACKING_CONDITIONS
+ halt();
+}
+int getInt();
+
+void f(int flag) {
+ int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
+ flag = getInt();
+ assert(flag);
+#ifdef TRACKING_CONDITIONS
+ // expected-note@-3{{Value assigned to 'flag'}}
+ // expected-note@-3{{Calling 'assert'}}
+ // expected-note@-4{{Returning from 'assert'}}
+#endif // TRACKING_CONDITIONS
+ if (flag) // expected-note{{'flag' is not equal to 0}}
+ // expected-note@-1{{Taking true branch}}
+ *x = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note@-1{{Dereference of null pointer}}
+}
+
+} // end of namespace unimportant_write_before_collapse_point
Index: clang/test/Analysis/analyzer-config.c
===================================================================
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -84,8 +84,9 @@
// CHECK-NEXT: suppress-c++-stdlib = true
// CHECK-NEXT: suppress-inlined-defensive-checks = true
// CHECK-NEXT: suppress-null-return-paths = true
+// CHECK-NEXT: track-conditions = false
// CHECK-NEXT: unix.DynamicMemoryModeling:Optimistic = false
// CHECK-NEXT: unroll-loops = false
// CHECK-NEXT: widen-loops = false
// CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 85
+// CHECK-NEXT: num-entries = 86
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -22,6 +22,7 @@
#include "clang/AST/Stmt.h"
#include "clang/AST/Type.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Analysis/Analyses/Dominators.h"
#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Analysis/CFG.h"
#include "clang/Analysis/CFGStmtMap.h"
@@ -1554,6 +1555,83 @@
return nullptr;
}
+//===----------------------------------------------------------------------===//
+// TrackControlDependencyCondBRVisitor.
+//===----------------------------------------------------------------------===//
+
+namespace {
+/// 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;
+ ControlDependencyCalculator ControlDeps;
+ llvm::SmallSet<const CFGBlock *, 32> VisitedBlocks;
+
+public:
+ TrackControlDependencyCondBRVisitor(const ExplodedNode *O)
+ : Origin(O), ControlDeps(&O->getCFG()) {}
+
+ void Profile(llvm::FoldingSetNodeID &ID) const override {
+ static int x = 0;
+ ID.AddPointer(&x);
+ }
+
+ std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N,
+ BugReporterContext &BRC,
+ BugReport &BR) override;
+};
+} // end of anonymous namespace
+
+static CFGBlock *GetRelevantBlock(const ExplodedNode *Node) {
+ if (auto SP = Node->getLocationAs<StmtPoint>()) {
+ const Stmt *S = SP->getStmt();
+ assert(S);
+
+ return const_cast<CFGBlock *>(Node->getLocationContext()
+ ->getAnalysisDeclContext()->getCFGStmtMap()->getBlock(S));
+ }
+
+ return nullptr;
+}
+
+std::shared_ptr<PathDiagnosticPiece>
+TrackControlDependencyCondBRVisitor::VisitNode(const ExplodedNode *N,
+ BugReporterContext &BRC,
+ BugReport &BR) {
+ // We can only reason about control dependencies within the same stack frame.
+ if (Origin->getStackFrame() != N->getStackFrame())
+ return nullptr;
+
+ CFGBlock *NB = GetRelevantBlock(N);
+
+ // Skip if we already inspected this block.
+ if (!VisitedBlocks.insert(NB).second)
+ return nullptr;
+
+ CFGBlock *OriginB = GetRelevantBlock(Origin);
+
+ // TODO: Cache CFGBlocks for each ExplodedNode.
+ if (!OriginB || !NB)
+ return nullptr;
+
+ if (ControlDeps.isControlDependent(OriginB, NB))
+ if (const Expr *Condition = NB->getTerminatorConditionExpr())
+ if (BR.addTrackedCondition({Condition, N}))
+ bugreporter::trackExpressionValue(
+ N, Condition, BR, /*EnableNullFPSuppression=*/false);
+
+ return nullptr;
+}
+
//===----------------------------------------------------------------------===//
// Implementation of trackExpressionValue.
//===----------------------------------------------------------------------===//
@@ -1670,6 +1748,10 @@
ProgramStateRef LVState = LVNode->getState();
+ if (LVState->getAnalysisManager().getAnalyzerOptions().ShouldTrackConditions)
+ report.addVisitor(llvm::make_unique<TrackControlDependencyCondBRVisitor>(
+ InputNode));
+
// The message send could be nil due to the receiver being nil.
// At this point in the path, the receiver should be live since we are at the
// message send expr. If it is nil, start tracking it.
Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
+++ clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
@@ -153,6 +153,10 @@
/// \sa removeInvalidation
llvm::SmallSet<InvalidationRecord, 4> Invalidations;
+ using ConditionValue = std::pair<const Expr *, const ExplodedNode*>;
+ /// Conditions we're already tracking.
+ llvm::SmallSet<ConditionValue, 4> TrackedConditions;
+
private:
// Used internally by BugReporter.
Symbols &getInterestingSymbols();
@@ -349,6 +353,10 @@
visitor_iterator visitor_begin() { return Callbacks.begin(); }
visitor_iterator visitor_end() { return Callbacks.end(); }
+ bool addTrackedCondition(const ConditionValue &Cond) {
+ return TrackedConditions.insert(Cond).second;
+ }
+
/// Profile to identify equivalent bug reports for error report coalescing.
/// Reports are uniqued to ensure that we do not emit multiple diagnostics
/// for each bug.
Index: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
+++ clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
@@ -291,6 +291,11 @@
"the analyzer's progress related to ctu.",
false)
+ANALYZER_OPTION(bool, ShouldTrackConditions, "track-conditions",
+ "Whether to track conditions that are a control dependency of "
+ "an already tracked variable.",
+ false)
+
//===----------------------------------------------------------------------===//
// Unsinged analyzer options.
//===----------------------------------------------------------------------===//
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
