Szelethus updated this revision to Diff 208276.
Szelethus added a comment.
Rebase.
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,11 +127,9 @@
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'}}
- // debug-note@-2{{Tracking condition 'flag'}}
- // expected-note@-3{{Assuming 'flag' is not equal to 0}}
- // expected-note@-4{{Taking true branch}}
+ if (int flag = foo()) // debug-note{{Tracking condition 'flag'}}
+ // expected-note@-1{{Assuming 'flag' is not equal to 0}}
+ // expected-note@-2{{Taking true branch}}
*x = 5; // expected-warning{{Dereference of null pointer}}
// expected-note@-1{{Dereference of null pointer}}
@@ -142,18 +140,16 @@
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}}
}
@@ -163,18 +159,16 @@
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}}
}
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
@@ -855,7 +855,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,
@@ -870,7 +870,7 @@
ReturnVisitor(const StackFrameContext *Frame,
bool Suppressed,
AnalyzerOptions &Options)
- : StackFrame(Frame), EnableNullFPSuppression(Suppressed),
+ : CalleeSFC(Frame), EnableNullFPSuppression(Suppressed),
Options(Options) {}
static void *getTag() {
@@ -880,7 +880,7 @@
void Profile(llvm::FoldingSetNodeID &ID) const override {
ID.AddPointer(ReturnVisitor::getTag());
- ID.AddPointer(StackFrame);
+ ID.AddPointer(CalleeSFC);
ID.AddBoolean(EnableNullFPSuppression);
}
@@ -964,7 +964,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));
@@ -974,7 +973,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>();
@@ -988,7 +987,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;
@@ -1022,6 +1021,8 @@
SmallString<64> Msg;
llvm::raw_svector_ostream Out(Msg);
+ bool WouldEventBeMeaningless = false;
+
if (State->isNull(V).isConstrainedTrue()) {
if (V.getAs<Loc>()) {
@@ -1044,16 +1045,24 @@
} 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.
+ if (N->getCFG().isLinear())
+ WouldEventBeMeaningless = true;
+
+ if (V.getAs<Loc>())
+ Out << "Returning pointer";
+ else
+ Out << "Returning value";
}
}
if (LValue) {
if (const MemRegion *MR = LValue->getAsRegion()) {
if (MR->canPrintPretty()) {
+ WouldEventBeMeaningless = false;
Out << " (reference to ";
MR->printPretty(Out);
Out << ")";
@@ -1061,16 +1070,28 @@
}
} else {
// FIXME: We should have a more generalized location printing mechanism.
- if (const auto *DR = dyn_cast<DeclRefExpr>(RetE))
- if (const auto *DD = dyn_cast<DeclaratorDecl>(DR->getDecl()))
+ if (const auto *DR = dyn_cast<DeclRefExpr>(RetE)) {
+ if (const auto *DD = dyn_cast<DeclaratorDecl>(DR->getDecl())) {
+ WouldEventBeMeaningless = false;
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>
@@ -1085,7 +1106,7 @@
if (!CE)
return nullptr;
- if (CE->getCalleeContext() != StackFrame)
+ if (CE->getCalleeContext() != CalleeSFC)
return nullptr;
Mode = Satisfied;
@@ -1097,7 +1118,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)
@@ -1140,7 +1161,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
