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

Artem Dergachev via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 3 13:04:30 PST 2018


Author: dergachev
Date: Mon Dec  3 13:04:30 2018
New Revision: 348188

URL: http://llvm.org/viewvc/llvm-project?rev=348188&view=rev
Log:
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

Added:
    cfe/trunk/test/Analysis/nullability-arc.mm
Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
    cfe/trunk/test/Analysis/nullability.mm

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp?rev=348188&r1=348187&r2=348188&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp Mon Dec  3 13:04:30 2018
@@ -329,8 +329,8 @@ NullabilityChecker::NullabilityBugVisito
                                                     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 checkValueAtLValForInvariant
   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_
 
   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 << " : ";

Added: cfe/trunk/test/Analysis/nullability-arc.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/nullability-arc.mm?rev=348188&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/nullability-arc.mm (added)
+++ cfe/trunk/test/Analysis/nullability-arc.mm Mon Dec  3 13:04:30 2018
@@ -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)
+
+ at interface Param
+ at end
+
+ at interface Base
+- (void)foo:(Param *_Nonnull)param;
+ at end
+
+ at interface Derived : Base
+ at end
+
+ at 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 at -2{{nil passed to a callee that requires a non-null 1st parameter}}
+  // expected-note at -3   {{nil passed to a callee that requires a non-null 1st parameter}}
+#endif
+
+  [self foo:nil];
+#if __has_feature(objc_arc)
+  // expected-note at -2{{Calling 'foo:'}}
+  // expected-note at -3{{Passing nil object reference via 1st parameter 'param'}}
+#endif
+}
+ at end
+

Modified: cfe/trunk/test/Analysis/nullability.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/nullability.mm?rev=348188&r1=348187&r2=348188&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/nullability.mm (original)
+++ cfe/trunk/test/Analysis/nullability.mm Mon Dec  3 13:04:30 2018
@@ -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"
 




More information about the cfe-commits mailing list