From: Artem Dergachev Date: Mon, 3 Dec 2018 21:04:30 +0000 (+0000) Subject: Re-apply r347954 "[analyzer] Nullability: Don't detect post factum violation..." X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=674329f694109671fdb32e25005b8f0a066fbddf;p=clang 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 eae0a974ce..38c694d5a2 100644 --- 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(); - 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(); + if (!StoredVal || !isa(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(); + if (State->get()) + Out << Sep << NL + << "Nullability invariant was violated, warnings suppressed." << NL; + if (B.isEmpty()) return; - Out << Sep << NL; + if (!State->get()) + 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 index 0000000000..5c68dda42e --- /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 2278efff3e..c3f27e4d22 100644 --- 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"