Re-apply r347954 "[analyzer] Nullability: Don't detect post factum violation..."

Buildbot failures were caused by an unrelated UB that was introduced in r347943
and fixed in r347970.

Also the revision was incorrectly specified as r344580 during revert.

Differential Revision: https://reviews.llvm.org/D54017

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@348188 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
index eae0a974ce8f6bf6d9e5f4c6222724db5fcaa308..38c694d5a25e7b0e31f069c1df350a4c4fcb2c4f 100644 (file)
--- a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -329,8 +329,8 @@ NullabilityChecker::NullabilityBugVisitor::VisitNode(const ExplodedNode *N,
                                                     nullptr);
 }
 
-/// Returns true when the value stored at the given location is null
-/// and the passed in type is nonnnull.
+/// Returns true when the value stored at the given location has been
+/// constrained to null after being passed through an object of nonnnull type.
 static bool checkValueAtLValForInvariantViolation(ProgramStateRef State,
                                                   SVal LV, QualType T) {
   if (getNullabilityAnnotation(T) != Nullability::Nonnull)
@@ -340,9 +340,14 @@ static bool checkValueAtLValForInvariantViolation(ProgramStateRef State,
   if (!RegionVal)
     return false;
 
-  auto StoredVal =
-  State->getSVal(RegionVal->getRegion()).getAs<DefinedOrUnknownSVal>();
-  if (!StoredVal)
+  // If the value was constrained to null *after* it was passed through that
+  // location, it could not have been a concrete pointer *when* it was passed.
+  // In that case we would have handled the situation when the value was
+  // bound to that location, by emitting (or not emitting) a report.
+  // Therefore we are only interested in symbolic regions that can be either
+  // null or non-null depending on the value of their respective symbol.
+  auto StoredVal = State->getSVal(*RegionVal).getAs<loc::MemRegionVal>();
+  if (!StoredVal || !isa<SymbolicRegion>(StoredVal->getRegion()))
     return false;
 
   if (getNullConstraint(*StoredVal, State) == NullConstraint::IsNull)
@@ -1170,10 +1175,15 @@ void NullabilityChecker::printState(raw_ostream &Out, ProgramStateRef State,
 
   NullabilityMapTy B = State->get<NullabilityMap>();
 
+  if (State->get<InvariantViolated>())
+    Out << Sep << NL
+        << "Nullability invariant was violated, warnings suppressed." << NL;
+
   if (B.isEmpty())
     return;
 
-  Out << Sep << NL;
+  if (!State->get<InvariantViolated>())
+    Out << Sep << NL;
 
   for (NullabilityMapTy::iterator I = B.begin(), E = B.end(); I != E; ++I) {
     Out << I->first << " : ";

diff --git a/test/Analysis/nullability-arc.mm b/test/Analysis/nullability-arc.mm
new file mode 100644 (file)
index 0000000..5c68dda
--- /dev/null
+++ b/test/Analysis/nullability-arc.mm
@@ -0,0 +1,39 @@
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,nullability\
+// RUN:                       -analyzer-output=text -verify %s
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,nullability\
+// RUN:                       -analyzer-output=text -verify %s -fobjc-arc
+
+#if !__has_feature(objc_arc)
+// expected-no-diagnostics
+#endif
+
+
+#define nil ((id)0)
+
+@interface Param
+@end
+
+@interface Base
+- (void)foo:(Param *_Nonnull)param;
+@end
+
+@interface Derived : Base
+@end
+
+@implementation Derived
+- (void)foo:(Param *)param {
+  // FIXME: Why do we not emit the warning under ARC?
+  [super foo:param];
+#if __has_feature(objc_arc)
+  // expected-warning@-2{{nil passed to a callee that requires a non-null 1st parameter}}
+  // expected-note@-3   {{nil passed to a callee that requires a non-null 1st parameter}}
+#endif
+
+  [self foo:nil];
+#if __has_feature(objc_arc)
+  // expected-note@-2{{Calling 'foo:'}}
+  // expected-note@-3{{Passing nil object reference via 1st parameter 'param'}}
+#endif
+}
+@end
+

diff --git a/test/Analysis/nullability.mm b/test/Analysis/nullability.mm
index 2278efff3e992f05e63957972678f7404634356e..c3f27e4d22a4e8581f22cd2cfd38c0203e34bc20 100644 (file)
--- a/test/Analysis/nullability.mm
+++ b/test/Analysis/nullability.mm
@@ -1,5 +1,36 @@
-// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core,nullability.NullPassedToNonnull,nullability.NullReturnedFromNonnull,nullability.NullablePassedToNonnull,nullability.NullableReturnedFromNonnull,nullability.NullableDereferenced -DNOSYSTEMHEADERS=0 -verify %s
-// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core,nullability.NullPassedToNonnull,nullability.NullReturnedFromNonnull,nullability.NullablePassedToNonnull,nullability.NullableReturnedFromNonnull,nullability.NullableDereferenced -analyzer-config nullability:NoDiagnoseCallsToSystemHeaders=true -DNOSYSTEMHEADERS=1 -verify %s
+// RUN: %clang_analyze_cc1 -fblocks -verify %s -analyzer-checker=core \
+// RUN:   -analyzer-checker=nullability.NullPassedToNonnull \
+// RUN:   -analyzer-checker=nullability.NullReturnedFromNonnull \
+// RUN:   -analyzer-checker=nullability.NullablePassedToNonnull \
+// RUN:   -analyzer-checker=nullability.NullableReturnedFromNonnull \
+// RUN:   -analyzer-checker=nullability.NullableDereferenced \
+// RUN:   -DNOSYSTEMHEADERS=0
+
+// RUN: %clang_analyze_cc1 -fblocks -verify %s -analyzer-checker=core \
+// RUN:   -analyzer-checker=nullability.NullPassedToNonnull \
+// RUN:   -analyzer-checker=nullability.NullReturnedFromNonnull \
+// RUN:   -analyzer-checker=nullability.NullablePassedToNonnull \
+// RUN:   -analyzer-checker=nullability.NullableReturnedFromNonnull \
+// RUN:   -analyzer-checker=nullability.NullableDereferenced \
+// RUN:   -DNOSYSTEMHEADERS=1 \
+// RUN:   -analyzer-config nullability:NoDiagnoseCallsToSystemHeaders=true
+
+// RUN: %clang_analyze_cc1 -fblocks -verify %s -analyzer-checker=core\
+// RUN:   -analyzer-checker=nullability.NullPassedToNonnull\
+// RUN:   -analyzer-checker=nullability.NullReturnedFromNonnull\
+// RUN:   -analyzer-checker=nullability.NullablePassedToNonnull\
+// RUN:   -analyzer-checker=nullability.NullableReturnedFromNonnull\
+// RUN:   -analyzer-checker=nullability.NullableDereferenced\
+// RUN:   -DNOSYSTEMHEADERS=0 -fobjc-arc
+
+// RUN: %clang_analyze_cc1 -fblocks -verify %s -analyzer-checker=core\
+// RUN:   -analyzer-checker=nullability.NullPassedToNonnull\
+// RUN:   -analyzer-checker=nullability.NullReturnedFromNonnull\
+// RUN:   -analyzer-checker=nullability.NullablePassedToNonnull\
+// RUN:   -analyzer-checker=nullability.NullableReturnedFromNonnull\
+// RUN:   -analyzer-checker=nullability.NullableDereferenced\
+// RUN:   -DNOSYSTEMHEADERS=1 -fobjc-arc\
+// RUN:   -analyzer-config nullability:NoDiagnoseCallsToSystemHeaders=true
 
 #include "Inputs/system-header-simulator-for-nullability.h"