r368771 - [analyzer] Prune calls to functions with linear CFGs that return a non-zero constrained value
Kristof Umann via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 13 16:22:33 PDT 2019
Author: szelethus
Date: Tue Aug 13 16:22:33 2019
New Revision: 368771
URL: http://llvm.org/viewvc/llvm-project?rev=368771&view=rev
Log:
[analyzer] Prune calls to functions with linear CFGs that return a non-zero constrained value
During the evaluation of D62883, I noticed a bunch of totally
meaningless notes with the pattern of "Calling 'A'" -> "Returning value"
-> "Returning from 'A'", which added no value to the report at all.
This patch (not only affecting tracked conditions mind you) prunes
diagnostic messages to functions that return a value not constrained to
be 0, and are also linear.
Differential Revision: https://reviews.llvm.org/D64232
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/test/Analysis/diagnostics/find_last_store.c
cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp
cfe/trunk/test/Analysis/uninit-vals.c
Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=368771&r1=368770&r2=368771&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Tue Aug 13 16:22:33 2019
@@ -841,7 +841,7 @@ namespace {
/// This visitor is intended to be used when another visitor discovers that an
/// interesting value comes from an inlined function call.
class ReturnVisitor : public BugReporterVisitor {
- const StackFrameContext *StackFrame;
+ const StackFrameContext *CalleeSFC;
enum {
Initial,
MaybeUnsuppress,
@@ -853,10 +853,9 @@ class ReturnVisitor : public BugReporter
AnalyzerOptions& Options;
public:
- ReturnVisitor(const StackFrameContext *Frame,
- bool Suppressed,
+ ReturnVisitor(const StackFrameContext *Frame, bool Suppressed,
AnalyzerOptions &Options)
- : StackFrame(Frame), EnableNullFPSuppression(Suppressed),
+ : CalleeSFC(Frame), EnableNullFPSuppression(Suppressed),
Options(Options) {}
static void *getTag() {
@@ -866,7 +865,7 @@ public:
void Profile(llvm::FoldingSetNodeID &ID) const override {
ID.AddPointer(ReturnVisitor::getTag());
- ID.AddPointer(StackFrame);
+ ID.AddPointer(CalleeSFC);
ID.AddBoolean(EnableNullFPSuppression);
}
@@ -950,7 +949,6 @@ public:
if (Optional<Loc> RetLoc = RetVal.getAs<Loc>())
EnableNullFPSuppression = State->isNull(*RetLoc).isConstrainedTrue();
- BR.markInteresting(CalleeContext);
BR.addVisitor(llvm::make_unique<ReturnVisitor>(CalleeContext,
EnableNullFPSuppression,
Options));
@@ -960,7 +958,7 @@ public:
BugReporterContext &BRC,
BugReport &BR) {
// Only print a message at the interesting return statement.
- if (N->getLocationContext() != StackFrame)
+ if (N->getLocationContext() != CalleeSFC)
return nullptr;
Optional<StmtPoint> SP = N->getLocationAs<StmtPoint>();
@@ -974,7 +972,7 @@ public:
// Okay, we're at the right return statement, but do we have the return
// value available?
ProgramStateRef State = N->getState();
- SVal V = State->getSVal(Ret, StackFrame);
+ SVal V = State->getSVal(Ret, CalleeSFC);
if (V.isUnknownOrUndef())
return nullptr;
@@ -1008,6 +1006,8 @@ public:
SmallString<64> Msg;
llvm::raw_svector_ostream Out(Msg);
+ bool WouldEventBeMeaningless = false;
+
if (State->isNull(V).isConstrainedTrue()) {
if (V.getAs<Loc>()) {
@@ -1030,10 +1030,19 @@ public:
} else {
if (auto CI = V.getAs<nonloc::ConcreteInt>()) {
Out << "Returning the value " << CI->getValue();
- } else if (V.getAs<Loc>()) {
- Out << "Returning pointer";
} else {
- Out << "Returning value";
+ // There is nothing interesting about returning a value, when it is
+ // plain value without any constraints, and the function is guaranteed
+ // to return that every time. We could use CFG::isLinear() here, but
+ // constexpr branches are obvious to the compiler, not necesserily to
+ // the programmer.
+ if (N->getCFG().size() == 3)
+ WouldEventBeMeaningless = true;
+
+ if (V.getAs<Loc>())
+ Out << "Returning pointer";
+ else
+ Out << "Returning value";
}
}
@@ -1052,11 +1061,20 @@ public:
Out << " (loaded from '" << *DD << "')";
}
- PathDiagnosticLocation L(Ret, BRC.getSourceManager(), StackFrame);
+ PathDiagnosticLocation L(Ret, BRC.getSourceManager(), CalleeSFC);
if (!L.isValid() || !L.asLocation().isValid())
return nullptr;
- return std::make_shared<PathDiagnosticEventPiece>(L, Out.str());
+ auto EventPiece = std::make_shared<PathDiagnosticEventPiece>(L, Out.str());
+
+ // If we determined that the note is meaningless, make it prunable, and
+ // don't mark the stackframe interesting.
+ if (WouldEventBeMeaningless)
+ EventPiece->setPrunable(true);
+ else
+ BR.markInteresting(CalleeSFC);
+
+ return EventPiece;
}
PathDiagnosticPieceRef visitNodeMaybeUnsuppress(const ExplodedNode *N,
@@ -1071,7 +1089,7 @@ public:
if (!CE)
return nullptr;
- if (CE->getCalleeContext() != StackFrame)
+ if (CE->getCalleeContext() != CalleeSFC)
return nullptr;
Mode = Satisfied;
@@ -1083,7 +1101,7 @@ public:
CallEventManager &CallMgr = StateMgr.getCallEventManager();
ProgramStateRef State = N->getState();
- CallEventRef<> Call = CallMgr.getCaller(StackFrame, State);
+ CallEventRef<> Call = CallMgr.getCaller(CalleeSFC, State);
for (unsigned I = 0, E = Call->getNumArgs(); I != E; ++I) {
Optional<Loc> ArgV = Call->getArgSVal(I).getAs<Loc>();
if (!ArgV)
@@ -1126,7 +1144,7 @@ public:
void finalizeVisitor(BugReporterContext &, const ExplodedNode *,
BugReport &BR) override {
if (EnableNullFPSuppression && ShouldInvalidate)
- BR.markInvalid(ReturnVisitor::getTag(), StackFrame);
+ BR.markInvalid(ReturnVisitor::getTag(), CalleeSFC);
}
};
Modified: cfe/trunk/test/Analysis/diagnostics/find_last_store.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/find_last_store.c?rev=368771&r1=368770&r2=368771&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/diagnostics/find_last_store.c (original)
+++ cfe/trunk/test/Analysis/diagnostics/find_last_store.c Tue Aug 13 16:22:33 2019
@@ -2,13 +2,11 @@
typedef struct { float b; } c;
void *a();
void *d() {
- return a(); // expected-note{{Returning pointer}}
+ return a();
}
void no_find_last_store() {
- c *e = d(); // expected-note{{Calling 'd'}}
- // expected-note at -1{{Returning from 'd'}}
- // expected-note at -2{{'e' initialized here}}
+ c *e = d(); // expected-note{{'e' initialized here}}
(void)(e || e->b); // expected-note{{Assuming 'e' is null}}
// expected-note at -1{{Left side of '||' is false}}
Modified: cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp?rev=368771&r1=368770&r2=368771&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp (original)
+++ cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp Tue Aug 13 16:22:33 2019
@@ -119,7 +119,7 @@ namespace variable_declaration_in_condit
bool coin();
bool foo() {
- return coin(); // tracking-note{{Returning value}}
+ return coin();
}
int bar;
@@ -127,12 +127,10 @@ int bar;
void test() {
int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
- if (int flag = foo()) // tracking-note{{Calling 'foo'}}
- // tracking-note at -1{{Returning from 'foo'}}
- // tracking-note at -2{{'flag' initialized here}}
- // debug-note at -3{{Tracking condition 'flag'}}
- // expected-note at -4{{Assuming 'flag' is not equal to 0}}
- // expected-note at -5{{Taking true branch}}
+ if (int flag = foo()) // tracking-note{{'flag' initialized here}}
+ // debug-note at -1{{Tracking condition 'flag'}}
+ // expected-note at -2{{Assuming 'flag' is not equal to 0}}
+ // expected-note at -3{{Taking true branch}}
*x = 5; // expected-warning{{Dereference of null pointer}}
// expected-note at -1{{Dereference of null pointer}}
@@ -143,39 +141,114 @@ namespace conversion_to_bool {
bool coin();
struct ConvertsToBool {
- operator bool() const { return coin(); } // tracking-note{{Returning value}}
+ operator bool() const { return coin(); }
};
void test() {
int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
if (ConvertsToBool())
- // tracking-note at -1 {{Calling 'ConvertsToBool::operator bool'}}
- // tracking-note at -2{{Returning from 'ConvertsToBool::operator bool'}}
- // debug-note at -3{{Tracking condition 'ConvertsToBool()'}}
- // expected-note at -4{{Assuming the condition is true}}
- // expected-note at -5{{Taking true branch}}
+ // debug-note at -1{{Tracking condition 'ConvertsToBool()'}}
+ // expected-note at -2{{Assuming the condition is true}}
+ // expected-note at -3{{Taking true branch}}
*x = 5; // expected-warning{{Dereference of null pointer}}
// expected-note at -1{{Dereference of null pointer}}
}
} // end of namespace variable_declaration_in_condition
+namespace important_returning_pointer_loaded_from {
+bool coin();
+
+int *getIntPtr();
+
+void storeValue(int **i) {
+ *i = getIntPtr(); // tracking-note{{Value assigned to 'i'}}
+}
+
+int *conjurePointer() {
+ int *i;
+ storeValue(&i); // tracking-note{{Calling 'storeValue'}}
+ // tracking-note at -1{{Returning from 'storeValue'}}
+ return i; // tracking-note{{Returning pointer (loaded from 'i')}}
+}
+
+void f(int *ptr) {
+ if (ptr) // expected-note{{Assuming 'ptr' is null}}
+ // expected-note at -1{{Taking false branch}}
+ ;
+ if (!conjurePointer())
+ // tracking-note at -1{{Calling 'conjurePointer'}}
+ // tracking-note at -2{{Returning from 'conjurePointer'}}
+ // debug-note at -3{{Tracking condition '!conjurePointer()'}}
+ // expected-note at -4{{Assuming the condition is true}}
+ // expected-note at -5{{Taking true branch}}
+ *ptr = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note at -1{{Dereference of null pointer}}
+}
+} // end of namespace important_returning_pointer_loaded_from
+
+namespace unimportant_returning_pointer_loaded_from {
+bool coin();
+
+int *getIntPtr();
+
+int *conjurePointer() {
+ int *i = getIntPtr(); // tracking-note{{'i' initialized here}}
+ return i; // tracking-note{{Returning pointer (loaded from 'i')}}
+}
+
+void f(int *ptr) {
+ if (ptr) // expected-note{{Assuming 'ptr' is null}}
+ // expected-note at -1{{Taking false branch}}
+ ;
+ if (!conjurePointer())
+ // tracking-note at -1{{Calling 'conjurePointer'}}
+ // tracking-note at -2{{Returning from 'conjurePointer'}}
+ // debug-note at -3{{Tracking condition '!conjurePointer()'}}
+ // expected-note at -4{{Assuming the condition is true}}
+ // expected-note at -5{{Taking true branch}}
+ *ptr = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note at -1{{Dereference of null pointer}}
+}
+} // end of namespace unimportant_returning_pointer_loaded_from
+
+namespace unimportant_returning_pointer_loaded_from_through_cast {
+
+void *conjure();
+
+int *cast(void *P) {
+ return static_cast<int *>(P);
+}
+
+void f() {
+ int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
+
+ if (cast(conjure()))
+ // tracking-note at -1{{Passing value via 1st parameter 'P'}}
+ // debug-note at -2{{Tracking condition 'cast(conjure())'}}
+ // expected-note at -3{{Assuming the condition is false}}
+ // expected-note at -4{{Taking false branch}}
+ return;
+ *x = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note at -1{{Dereference of null pointer}}
+}
+
+} // end of namespace unimportant_returning_pointer_loaded_from_through_cast
+
namespace unimportant_returning_value_note {
bool coin();
-bool flipCoin() { return coin(); } // tracking-note{{Returning value}}
+bool flipCoin() { return coin(); }
void i(int *ptr) {
if (ptr) // expected-note{{Assuming 'ptr' is null}}
// expected-note at -1{{Taking false branch}}
;
if (!flipCoin())
- // tracking-note at -1{{Calling 'flipCoin'}}
- // tracking-note at -2{{Returning from 'flipCoin'}}
- // debug-note at -3{{Tracking condition '!flipCoin()'}}
- // expected-note at -4{{Assuming the condition is true}}
- // expected-note at -5{{Taking true branch}}
+ // debug-note at -1{{Tracking condition '!flipCoin()'}}
+ // expected-note at -2{{Assuming the condition is true}}
+ // expected-note at -3{{Taking true branch}}
*ptr = 5; // expected-warning{{Dereference of null pointer}}
// expected-note at -1{{Dereference of null pointer}}
}
@@ -207,6 +280,36 @@ void i(int *ptr) {
}
} // end of namespace important_returning_value_note
+namespace important_returning_value_note_in_linear_function {
+bool coin();
+
+struct super_complicated_template_hackery {
+ static constexpr bool value = false;
+};
+
+bool flipCoin() {
+ if (super_complicated_template_hackery::value)
+ // tracking-note at -1{{'value' is false}}
+ // tracking-note at -2{{Taking false branch}}
+ return true;
+ return coin(); // tracking-note{{Returning value}}
+}
+
+void i(int *ptr) {
+ if (ptr) // expected-note{{Assuming 'ptr' is null}}
+ // expected-note at -1{{Taking false branch}}
+ ;
+ if (!flipCoin())
+ // tracking-note at -1{{Calling 'flipCoin'}}
+ // tracking-note at -2{{Returning from 'flipCoin'}}
+ // debug-note at -3{{Tracking condition '!flipCoin()'}}
+ // expected-note at -4{{Assuming the condition is true}}
+ // expected-note at -5{{Taking true branch}}
+ *ptr = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note at -1{{Dereference of null pointer}}
+}
+} // end of namespace important_returning_value_note_in_linear_function
+
namespace tracked_condition_is_only_initialized {
int getInt();
Modified: cfe/trunk/test/Analysis/uninit-vals.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/uninit-vals.c?rev=368771&r1=368770&r2=368771&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/uninit-vals.c (original)
+++ cfe/trunk/test/Analysis/uninit-vals.c Tue Aug 13 16:22:33 2019
@@ -149,8 +149,6 @@ int test_radar12278788_FP() {
RetVoidFuncType f = foo_radar12278788_fp;
return ((RetIntFuncType)f)(); //expected-warning {{Undefined or garbage value returned to caller}}
//expected-note at -1 {{Undefined or garbage value returned to caller}}
- //expected-note at -2 {{Calling 'foo_radar12278788_fp'}}
- //expected-note at -3 {{Returning from 'foo_radar12278788_fp'}}
}
void rdar13665798() {
@@ -164,8 +162,6 @@ void rdar13665798() {
RetVoidFuncType f = foo_radar12278788_fp;
return ((RetIntFuncType)f)(); //expected-warning {{Undefined or garbage value returned to caller}}
//expected-note at -1 {{Undefined or garbage value returned to caller}}
- //expected-note at -2 {{Calling 'foo_radar12278788_fp'}}
- //expected-note at -3 {{Returning from 'foo_radar12278788_fp'}}
}();
}
@@ -182,18 +178,14 @@ struct Point getHalfPoint() {
void use(struct Point p);
void testUseHalfPoint() {
- struct Point p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}}
- // expected-note at -1{{Returning from 'getHalfPoint'}}
- // expected-note at -2{{'p' initialized here}}
+ struct Point p = getHalfPoint(); // expected-note{{'p' initialized here}}
use(p); // expected-warning{{uninitialized}}
// expected-note at -1{{uninitialized}}
}
void testUseHalfPoint2() {
struct Point p;
- p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}}
- // expected-note at -1{{Returning from 'getHalfPoint'}}
- // expected-note at -2{{Value assigned to 'p'}}
+ p = getHalfPoint(); // expected-note{{Value assigned to 'p'}}
use(p); // expected-warning{{uninitialized}}
// expected-note at -1{{uninitialized}}
}
More information about the cfe-commits
mailing list