r347954 - [analyzer] Nullability: Don't detect post factum violation on concrete values.
Artem Dergachev via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 29 19:39:58 PST 2018
Author: dergachev
Date: Thu Nov 29 19:39:58 2018
New Revision: 347954
URL: http://llvm.org/viewvc/llvm-project?rev=347954&view=rev
Log:
[analyzer] Nullability: Don't detect post factum violation on concrete values.
The checker suppresses warnings on paths on which a nonnull value is assumed
to be nullable. This probably deserves a warning, but it's a separate story.
Now, because dead symbol collection fires in pretty random moments,
there sometimes was a situation when dead symbol collection fired after
computing a parameter but before actually evaluating call enter into the
function, which triggered the suppression when the argument was null
in the first place earlier than the obvious warning for null-to-nonnull
was emitted, causing false negatives.
Only trigger the suppression for symbols, not for concrete values.
It is impossible to constrain a concrete value post-factum because
it is impossible to constrain a concrete value at all.
This covers all the necessary cases because by the time we reach the call,
symbolic values should be either not constrained to null, or already collapsed
into concrete null values. Which in turn happens because they are passed through
the Store, and the respective collapse is implemented as part of getSVal(),
which is also weird.
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=347954&r1=347953&r2=347954&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp Thu Nov 29 19:39:58 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=347954&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/nullability-arc.mm (added)
+++ cfe/trunk/test/Analysis/nullability-arc.mm Thu Nov 29 19:39:58 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=347954&r1=347953&r2=347954&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/nullability.mm (original)
+++ cfe/trunk/test/Analysis/nullability.mm Thu Nov 29 19:39:58 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