[PATCH] D62978: [analyzer] ReturnVisitor: Handle non-null ReturnStmts

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Ah, that positive!

No, i don't think this is a valid way to suppress it. I'll tease you a bit more 
though, and only give a hint: the null value that we're null-dereferencing was 
in fact assumed to be null //in the top frame//, not within any inline 
function. Therefore the whole "Returning pointer..." note is completely 
misleading: it is irrelevant what value we return from `getSuperRegion()`, we 
might have as well evaluated it conservatively, the report would still have 
been emitted.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62978/new/

https://reviews.llvm.org/D62978



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62978: [analyzer] ReturnVisitor: Handle non-null ReturnStmts

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

Suppressed example: F9091784: return-non-null.html 



Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62978/new/

https://reviews.llvm.org/D62978



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62978: [analyzer] ReturnVisitor: Handle non-null ReturnStmts

[Csaba Dabis via Phabricator via cfe-commits]

Charusso created this revision.
Charusso added reviewers: NoQ, xazax.hun, ravikandhadai, baloghadamsoftware, 
Szelethus.
Charusso added a project: clang.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, rnkovacs, szepet.
Charusso added a comment.

Suppressed example: F9091784: return-non-null.html 



If we have a value at the return side do not write out it is being null.


Repository:
  rC Clang

https://reviews.llvm.org/D62978

Files:
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/test/Analysis/diagnostics/find_last_store.c
  clang/test/Analysis/inlining/false-positive-suppression.cpp


Index: clang/test/Analysis/inlining/false-positive-suppression.cpp
===
--- clang/test/Analysis/inlining/false-positive-suppression.cpp
+++ clang/test/Analysis/inlining/false-positive-suppression.cpp
@@ -1,5 +1,13 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-config 
suppress-null-return-paths=false -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify -DSUPPRESSED=1 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:  -analyzer-config suppress-null-return-paths=false \
+// RUN:  -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:  -DSUPPRESSED=1 \
+// RUN:  -verify %s
+
+#ifdef SUPPRESSED
+// expected-no-diagnostics
+#endif
 
 namespace rdar12676053 {
   // Delta-reduced from a preprocessed file.
@@ -85,7 +93,10 @@
 
 int * = m.getValue(i);
 box2 = 0;
-*box2 = 1; // expected-warning {{Dereference of null pointer}}
+*box2 = 1;
+#ifndef SUPPRESSED
+// expected-warning@-2 {{Dereference of null pointer}}
+#endif
   }
 
   SomeClass *() {
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
@@ -1,4 +1,9 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-output=text 
-verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:  -analyzer-config suppress-null-return-paths=false \
+// RUN:  -analyzer-output=text -verify %s
+
+// FIXME: Make it work with suppress-null-return-paths=true.
+
 typedef struct { float b; } c;
 void *a();
 void *d() {
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -827,8 +827,7 @@
   /// the statement is a call that was inlined, we add the visitor to the
   /// bug report, so it can print a note later.
   static void addVisitorIfNecessary(const ExplodedNode *Node, const Stmt *S,
-BugReport ,
-bool InEnableNullFPSuppression) {
+BugReport , bool IsNullFPSuppression) {
 if (!CallEvent::isCallStmt(S))
   return;
 
@@ -877,11 +876,11 @@
 // See if the return value is NULL. If so, suppress the report.
 AnalyzerOptions  = State->getAnalysisManager().options;
 
+// If we have a value at the return side do not write out it is being null.
 bool EnableNullFPSuppression = false;
-if (InEnableNullFPSuppression &&
-Options.ShouldSuppressNullReturnPaths)
+if (IsNullFPSuppression && Options.ShouldSuppressNullReturnPaths)
   if (Optional RetLoc = RetVal.getAs())
-EnableNullFPSuppression = State->isNull(*RetLoc).isConstrainedTrue();
+EnableNullFPSuppression = true;
 
 BR.markInteresting(CalleeContext);
 BR.addVisitor(llvm::make_unique(CalleeContext,


Index: clang/test/Analysis/inlining/false-positive-suppression.cpp
===
--- clang/test/Analysis/inlining/false-positive-suppression.cpp
+++ clang/test/Analysis/inlining/false-positive-suppression.cpp
@@ -1,5 +1,13 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-config suppress-null-return-paths=false -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify -DSUPPRESSED=1 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:  -analyzer-config suppress-null-return-paths=false \
+// RUN:  -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:  -DSUPPRESSED=1 \
+// RUN:  -verify %s
+
+#ifdef SUPPRESSED
+// expected-no-diagnostics
+#endif
 
 namespace rdar12676053 {
   // Delta-reduced from a preprocessed file.
@@ -85,7 +93,10 @@
 
 int * = m.getValue(i);
 box2 = 0;
-*box2 = 1; // expected-warning {{Dereference of null pointer}}
+*box2 = 1;
+#ifndef SUPPRESSED
+// expected-warning@-2 {{Dereference of null pointer}}
+#endif
   }
 
   SomeClass *() {
Index: clang/test/Analysis/diagnostics/find_last_store.c