Szelethus updated this revision to Diff 212801.
Szelethus added a comment.
Use `size() == 3` instead if `isLinear()`, add a related test case.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64232/new/
https://reviews.llvm.org/D64232
Files:
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
clang/test/Analysis/diagnostics/find_last_store.c
clang/test/Analysis/track-control-dependency-conditions.cpp
clang/test/Analysis/uninit-vals.c
Index: clang/test/Analysis/uninit-vals.c
===================================================================
--- clang/test/Analysis/uninit-vals.c
+++ clang/test/Analysis/uninit-vals.c
@@ -149,8 +149,6 @@
RetVoidFuncType f = foo_radar12278788_fp;
return ((RetIntFuncType)f)(); //expected-warning {{Undefined or garbage value returned to caller}}
//expected-note@-1 {{Undefined or garbage value returned to caller}}
- //expected-note@-2 {{Calling 'foo_radar12278788_fp'}}
- //expected-note@-3 {{Returning from 'foo_radar12278788_fp'}}
}
void rdar13665798() {
@@ -164,8 +162,6 @@
RetVoidFuncType f = foo_radar12278788_fp;
return ((RetIntFuncType)f)(); //expected-warning {{Undefined or garbage value returned to caller}}
//expected-note@-1 {{Undefined or garbage value returned to caller}}
- //expected-note@-2 {{Calling 'foo_radar12278788_fp'}}
- //expected-note@-3 {{Returning from 'foo_radar12278788_fp'}}
}();
}
@@ -182,18 +178,14 @@
void use(struct Point p);
void testUseHalfPoint() {
- struct Point p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}}
- // expected-note@-1{{Returning from 'getHalfPoint'}}
- // expected-note@-2{{'p' initialized here}}
+ struct Point p = getHalfPoint(); // expected-note{{'p' initialized here}}
use(p); // expected-warning{{uninitialized}}
// expected-note@-1{{uninitialized}}
}
void testUseHalfPoint2() {
struct Point p;
- p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}}
- // expected-note@-1{{Returning from 'getHalfPoint'}}
- // expected-note@-2{{Value assigned to 'p'}}
+ p = getHalfPoint(); // expected-note{{Value assigned to 'p'}}
use(p); // expected-warning{{uninitialized}}
// expected-note@-1{{uninitialized}}
}
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
@@ -119,7 +119,7 @@
bool coin();
bool foo() {
- return coin(); // tracking-note{{Returning value}}
+ return coin();
}
int bar;
@@ -127,12 +127,10 @@
void test() {
int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
- if (int flag = foo()) // tracking-note{{Calling 'foo'}}
- // tracking-note@-1{{Returning from 'foo'}}
- // tracking-note@-2{{'flag' initialized here}}
- // debug-note@-3{{Tracking condition 'flag'}}
- // expected-note@-4{{Assuming 'flag' is not equal to 0}}
- // expected-note@-5{{Taking true branch}}
+ if (int flag = foo()) // tracking-note{{'flag' initialized here}}
+ // debug-note@-1{{Tracking condition 'flag'}}
+ // expected-note@-2{{Assuming 'flag' is not equal to 0}}
+ // expected-note@-3{{Taking true branch}}
*x = 5; // expected-warning{{Dereference of null pointer}}
// expected-note@-1{{Dereference of null pointer}}
@@ -143,39 +141,114 @@
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@-1 {{Calling 'ConvertsToBool::operator bool'}}
- // tracking-note@-2{{Returning from 'ConvertsToBool::operator bool'}}
- // debug-note@-3{{Tracking condition 'ConvertsToBool()'}}
- // expected-note@-4{{Assuming the condition is true}}
- // expected-note@-5{{Taking true branch}}
+ // debug-note@-1{{Tracking condition 'ConvertsToBool()'}}
+ // expected-note@-2{{Assuming the condition is true}}
+ // expected-note@-3{{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 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@-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@-1{{Taking false branch}}
+ ;
+ if (!conjurePointer())
+ // tracking-note@-1{{Calling 'conjurePointer'}}
+ // tracking-note@-2{{Returning from 'conjurePointer'}}
+ // debug-note@-3{{Tracking condition '!conjurePointer()'}}
+ // expected-note@-4{{Assuming the condition is true}}
+ // expected-note@-5{{Taking true branch}}
+ *ptr = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note@-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@-1{{Taking false branch}}
+ ;
+ if (!conjurePointer())
+ // tracking-note@-1{{Calling 'conjurePointer'}}
+ // tracking-note@-2{{Returning from 'conjurePointer'}}
+ // debug-note@-3{{Tracking condition '!conjurePointer()'}}
+ // expected-note@-4{{Assuming the condition is true}}
+ // expected-note@-5{{Taking true branch}}
+ *ptr = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note@-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@-1{{Passing value via 1st parameter 'P'}}
+ // debug-note@-2{{Tracking condition 'cast(conjure())'}}
+ // expected-note@-3{{Assuming the condition is false}}
+ // expected-note@-4{{Taking false branch}}
+ return;
+ *x = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note@-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@-1{{Taking false branch}}
;
if (!flipCoin())
- // tracking-note@-1{{Calling 'flipCoin'}}
- // tracking-note@-2{{Returning from 'flipCoin'}}
- // debug-note@-3{{Tracking condition '!flipCoin()'}}
- // expected-note@-4{{Assuming the condition is true}}
- // expected-note@-5{{Taking true branch}}
+ // debug-note@-1{{Tracking condition '!flipCoin()'}}
+ // expected-note@-2{{Assuming the condition is true}}
+ // expected-note@-3{{Taking true branch}}
*ptr = 5; // expected-warning{{Dereference of null pointer}}
// expected-note@-1{{Dereference of null pointer}}
}
@@ -207,6 +280,36 @@
}
} // 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@-1{{'value' is false}}
+ // tracking-note@-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@-1{{Taking false branch}}
+ ;
+ if (!flipCoin())
+ // tracking-note@-1{{Calling 'flipCoin'}}
+ // tracking-note@-2{{Returning from 'flipCoin'}}
+ // debug-note@-3{{Tracking condition '!flipCoin()'}}
+ // expected-note@-4{{Assuming the condition is true}}
+ // expected-note@-5{{Taking true branch}}
+ *ptr = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note@-1{{Dereference of null pointer}}
+}
+} // end of namespace important_returning_value_note_in_linear_function
+
namespace tracked_condition_is_only_initialized {
int getInt();
Index: clang/test/Analysis/diagnostics/find_last_store.c
===================================================================
--- clang/test/Analysis/diagnostics/find_last_store.c
+++ clang/test/Analysis/diagnostics/find_last_store.c
@@ -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@-1{{Returning from 'd'}}
- // expected-note@-2{{'e' initialized here}}
+ c *e = d(); // expected-note{{'e' initialized here}}
(void)(e || e->b); // expected-note{{Assuming 'e' is null}}
// expected-note@-1{{Left side of '||' is false}}
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -841,7 +841,7 @@
/// 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,
@@ -856,7 +856,7 @@
ReturnVisitor(const StackFrameContext *Frame,
bool Suppressed,
AnalyzerOptions &Options)
- : StackFrame(Frame), EnableNullFPSuppression(Suppressed),
+ : CalleeSFC(Frame), EnableNullFPSuppression(Suppressed),
Options(Options) {}
static void *getTag() {
@@ -866,7 +866,7 @@
void Profile(llvm::FoldingSetNodeID &ID) const override {
ID.AddPointer(ReturnVisitor::getTag());
- ID.AddPointer(StackFrame);
+ ID.AddPointer(CalleeSFC);
ID.AddBoolean(EnableNullFPSuppression);
}
@@ -950,7 +950,6 @@
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 +959,7 @@
visitNodeInitial(const ExplodedNode *N,
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 +973,7 @@
// 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 +1007,8 @@
SmallString<64> Msg;
llvm::raw_svector_ostream Out(Msg);
+ bool WouldEventBeMeaningless = false;
+
if (State->isNull(V).isConstrainedTrue()) {
if (V.getAs<Loc>()) {
@@ -1030,10 +1031,19 @@
} 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 +1062,20 @@
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;
}
std::shared_ptr<PathDiagnosticPiece>
@@ -1071,7 +1090,7 @@
if (!CE)
return nullptr;
- if (CE->getCalleeContext() != StackFrame)
+ if (CE->getCalleeContext() != CalleeSFC)
return nullptr;
Mode = Satisfied;
@@ -1083,7 +1102,7 @@
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 +1145,7 @@
void finalizeVisitor(BugReporterContext &, const ExplodedNode *,
BugReport &BR) override {
if (EnableNullFPSuppression && ShouldInvalidate)
- BR.markInvalid(ReturnVisitor::getTag(), StackFrame);
+ BR.markInvalid(ReturnVisitor::getTag(), CalleeSFC);
}
};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
