Charusso updated this revision to Diff 203876.
Charusso retitled this revision from "[analyzer] ReturnVisitor: Handle non-null
ReturnStmts" to "[analyzer] ReturnVisitor: Handle unknown ReturnStmts better".
Charusso edited the summary of this revision.
Charusso added a comment.
- The report was too misleading.
- It turns out we have to keep non-nulls.
- Now we do not treat unknown values as known.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62978/new/
https://reviews.llvm.org/D62978
Files:
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
clang/test/Analysis/diagnostics/find_last_store.c
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/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
@@ -874,7 +874,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));
@@ -925,16 +924,13 @@
RetE = RetE->IgnoreParenCasts();
- // If we're returning 0, we should track where that 0 came from.
- bugreporter::trackExpressionValue(N, RetE, BR, EnableNullFPSuppression);
-
// Build an appropriate message based on the return value.
SmallString<64> Msg;
llvm::raw_svector_ostream Out(Msg);
+ // Known to be null.
if (State->isNull(V).isConstrainedTrue()) {
if (V.getAs<Loc>()) {
-
// If we have counter-suppression enabled, make sure we keep visiting
// future nodes. We want to emit a path note as well, in case
// the report is resurrected as valid later on.
@@ -950,8 +946,8 @@
} else {
Out << "Returning zero";
}
-
- } else {
+ // Known to be non-null.
+ } else if (State->isNonNull(V).isConstrainedTrue()) {
if (auto CI = V.getAs<nonloc::ConcreteInt>()) {
Out << "Returning the value " << CI->getValue();
} else if (V.getAs<Loc>()) {
@@ -959,8 +955,15 @@
} else {
Out << "Returning value";
}
+ // Unknown value. If a pointer and its LValue is available treat as known.
+ } else if (V.getAs<Loc>() && LValue) {
+ Out << "Returning pointer";
}
+ // If we do not return a known value do not report that.
+ if (Msg.empty())
+ return nullptr;
+
if (LValue) {
if (const MemRegion *MR = LValue->getAsRegion()) {
if (MR->canPrintPretty()) {
@@ -976,6 +979,9 @@
Out << " (loaded from '" << *DD << "')";
}
+ // If we return a value track where it came from.
+ bugreporter::trackExpressionValue(N, RetE, BR, EnableNullFPSuppression);
+
PathDiagnosticLocation L(Ret, BRC.getSourceManager(), StackFrame);
if (!L.isValid() || !L.asLocation().isValid())
return nullptr;
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
