Szelethus created this revision.
Szelethus added reviewers: NoQ, xazax.hun, baloghadamsoftware, Charusso,
rnkovacs, dcoughlin.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy,
mikhail.ramalho, a.sidorin, szepet, whisperity.
Szelethus added a parent revision: D65724: [analyzer] Don't make
ConditionBRVisitor events prunable when the condition is an interesting field.
In D65724 <https://reviews.llvm.org/D65724>, I do a pretty thorough explanation
about how I'm solving this problem, I think that summary nails whats happening
here ;)
The actual message is kinda dumb here, but I figured that we'd have a couple
rounds on word smithing anyways, so I uploaded this as is. Its probably good
enough to gather some analysis data with.
Also, with some luck, this should the final technical patch regarding the first
part of my GSoC ;)
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D65725
Files:
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
clang/test/Analysis/track-control-dependency-conditions.cpp
Index: clang/test/Analysis/track-control-dependency-conditions.cpp
===================================================================
--- clang/test/Analysis/track-control-dependency-conditions.cpp
+++ clang/test/Analysis/track-control-dependency-conditions.cpp
@@ -407,12 +407,37 @@
}
} // end of namespace condition_written_in_nested_stackframe_before_assignment
+namespace collapse_point_not_in_condition_bool {
+
+[[noreturn]] void halt();
+
+void check(bool b) {
+ if (!b) // tracking-note{{Assuming 'b' is true, which will be (a part of a) condition}}
+ // tracking-note@-1{{Taking false branch}}
+ halt();
+}
+
+void f(bool flag) {
+ int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
+
+ check(flag); // tracking-note{{Calling 'check'}}
+ // tracking-note@-1{{Returning from 'check'}}
+
+ if (flag) // expected-note{{'flag' is true}}
+ // expected-note@-1{{Taking true branch}}
+ // debug-note@-2{{Tracking condition 'flag'}}
+ *x = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note@-1{{Dereference of null pointer}}
+}
+
+} // end of namespace collapse_point_not_in_condition_bool
+
namespace collapse_point_not_in_condition {
[[noreturn]] void halt();
void assert(int b) {
- if (!b) // tracking-note{{Assuming 'b' is not equal to 0}}
+ if (!b) // tracking-note{{Assuming 'b' is not equal to 0, which will be (a part of a) condition}}
// tracking-note@-1{{Taking false branch}}
halt();
}
@@ -437,7 +462,7 @@
[[noreturn]] void halt();
void assert(int b) {
- if (!b) // tracking-note{{Assuming 'b' is not equal to 0}}
+ if (!b) // tracking-note{{Assuming 'b' is not equal to 0, which will be (a part of a) condition}}
// tracking-note@-1{{Taking false branch}}
halt();
}
@@ -459,6 +484,31 @@
} // end of namespace unimportant_write_before_collapse_point
+namespace collapse_point_not_in_condition_binary_op {
+
+[[noreturn]] void halt();
+
+void check(int b) {
+ if (b == 1) // tracking-note{{Assuming 'b' is not equal to 1, which will be (a part of a) condition}}
+ // tracking-note@-1{{Taking false branch}}
+ halt();
+}
+
+void f(int flag) {
+ int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
+
+ check(flag); // tracking-note{{Calling 'check'}}
+ // tracking-note@-1{{Returning from 'check'}}
+
+ if (flag) // expected-note{{'flag' is not equal to 0}}
+ // expected-note@-1{{Taking true branch}}
+ // debug-note@-2{{Tracking condition 'flag'}}
+ *x = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note@-1{{Dereference of null pointer}}
+}
+
+} // end of namespace collapse_point_not_in_condition_binary_op
+
namespace collapse_point_not_in_condition_as_field {
[[noreturn]] void halt();
@@ -467,7 +517,7 @@
IntWrapper();
void check() {
- if (!b) // tracking-note{{Assuming field 'b' is not equal to 0}}
+ if (!b) // tracking-note{{Assuming field 'b' is not equal to 0, which will be (a part of a) condition}}
// tracking-note@-1{{Taking false branch}}
halt();
return;
@@ -488,6 +538,65 @@
} // end of namespace collapse_point_not_in_condition_as_field
+namespace assignemnt_in_condition_in_nested_stackframe {
+int flag;
+
+bool coin();
+
+[[noreturn]] void halt();
+
+void foo() {
+ if ((flag = coin()))
+ // tracking-note@-1{{Value assigned to 'flag', which will be (a part of a) condition}}
+ // tracking-note@-2{{Assuming 'flag' is not equal to 0, which will be (a part of a) condition}}
+ // tracking-note@-3{{Taking true branch}}
+ return;
+ halt();
+ return;
+}
+
+void f() {
+ int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
+
+ foo(); // tracking-note{{Calling 'foo'}}
+ // tracking-note@-1{{Returning from 'foo'}}
+ if (flag) // expected-note{{'flag' is not equal to 0}}
+ // expected-note@-1{{Taking true branch}}
+ // debug-note@-2{{Tracking condition 'flag'}}
+ *x = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note@-1{{Dereference of null pointer}}
+}
+} // end of namespace assignemnt_in_condition_in_nested_stackframe
+
+namespace condition_variable_less {
+int flag;
+
+bool coin();
+
+[[noreturn]] void halt();
+
+void foo() {
+ if (flag > 0)
+ // tracking-note@-1{{Assuming 'flag' is > 0, which will be (a part of a) condition}}
+ // tracking-note@-2{{Taking true branch}}
+ return;
+ halt();
+ return;
+}
+
+void f() {
+ int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
+
+ foo(); // tracking-note{{Calling 'foo'}}
+ // tracking-note@-1{{Returning from 'foo'}}
+ if (flag) // expected-note{{'flag' is not equal to 0}}
+ // expected-note@-1{{Taking true branch}}
+ // debug-note@-2{{Tracking condition 'flag'}}
+ *x = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note@-1{{Dereference of null pointer}}
+}
+} // end of namespace condition_variable_less
+
namespace dont_track_assertlike_conditions {
extern void __assert_fail (__const char *__assertion, __const char *__file,
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -210,6 +210,22 @@
return None;
}
+static bool isVarAnInterestingCondition(const Expr *CondVarExpr,
+ const ExplodedNode *N,
+ const BugReport *B) {
+ // Even if this condition is marked as interesting, it isn't *that*
+ // interesting if it didn't happen in a nested stackframe, the user could just
+ // follow the arrows.
+ if (!B->getErrorNode()->getStackFrame()->isParentOf(N->getStackFrame()))
+ return false;
+
+ if (Optional<SVal> V = getSValForVar(CondVarExpr, N))
+ if (Optional<bugreporter::TrackingKind> K = B->getInterestingnessKind(*V))
+ return *K == bugreporter::TrackingKind::Condition;
+
+ return false;
+}
+
static bool isInterestingExpr(const Expr *E, const ExplodedNode *N,
const BugReport *B) {
if (Optional<SVal> V = getSValForVar(E, N))
@@ -2485,6 +2501,10 @@
const LocationContext *LCtx = N->getLocationContext();
PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LCtx);
+ if (isVarAnInterestingCondition(BExpr->getLHS(), N, &R) ||
+ isVarAnInterestingCondition(BExpr->getRHS(), N, &R))
+ Out << WillBeUsedForACondition;
+
// Convert 'field ...' to 'Field ...' if it is a MemberExpr.
std::string Message = Out.str();
Message[0] = toupper(Message[0]);
@@ -2515,6 +2535,9 @@
const LocationContext *LCtx = N->getLocationContext();
PathDiagnosticLocation Loc(CondVarExpr, BRC.getSourceManager(), LCtx);
+ if (isVarAnInterestingCondition(CondVarExpr, N, &report))
+ Out << WillBeUsedForACondition;
+
auto event = std::make_shared<PathDiagnosticEventPiece>(Loc, Out.str());
if (isInterestingExpr(CondVarExpr, N, &report))
@@ -2541,6 +2564,9 @@
const LocationContext *LCtx = N->getLocationContext();
PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LCtx);
+ if (isVarAnInterestingCondition(DRE, N, &report))
+ Out << WillBeUsedForACondition;
+
// If we know the value create a pop-up note.
if (!IsAssuming)
return std::make_shared<PathDiagnosticPopUpPiece>(Loc, Out.str());
@@ -2570,6 +2596,9 @@
if (!Loc.isValid() || !Loc.asLocation().isValid())
return nullptr;
+ if (isVarAnInterestingCondition(ME, N, &report))
+ Out << WillBeUsedForACondition;
+
// If we know the value create a pop-up note.
if (!IsAssuming)
return std::make_shared<PathDiagnosticPopUpPiece>(Loc, Out.str());
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
