[Static Analyzer] Moving nullability checkers to a top level package.

[xazax.hun]

Summary

This patch is a preparation to enabling some of the nullability checks by default.

1. Moved nullability checkers to a top level package. The core package is not a great fit, because some of the checks should be turned on by default and some of them should be opt-in checks.

2. When the only the default checks turned on, tracking nullability of the symbols is not necessary. This minimizes the impact of the default checks on memory consumption.

3. Added a separate test file to the configuration when only those checks are turned on that will be turned on by default later on.

[xazax.hun]

Removed an unintended comment formatting change.

[zaks.anna]

Please, explain in code why you are doing this. Otherwise, LGTM.

lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp

139 ↗

(On Diff #34705)

Explain what this is.

959 ↗

(On Diff #34705)

Explain why tracking is not needed for null passed/returned checks. And the reason to disable it - optimization.

cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td

 //===----------------------------------------------------------------------===//
 
 def Alpha : Package<"alpha">;
 
 def Core : Package<"core">;
 def CoreBuiltin : Package<"builtin">, InPackage<Core>;
 def CoreUninitialized  : Package<"uninitialized">, InPackage<Core>;
 def CoreAlpha : Package<"core">, InPackage<Alpha>, Hidden;
-def Nullability : Package<"nullability">, InPackage<CoreAlpha>, Hidden;
+def Nullability : Package<"nullability">, InPackage<Alpha>, Hidden;
 
 def Cplusplus : Package<"cplusplus">;
 def CplusplusAlpha : Package<"cplusplus">, InPackage<Alpha>, Hidden;
 
 def DeadCode : Package<"deadcode">;
 def DeadCodeAlpha : Package<"deadcode">, InPackage<Alpha>, Hidden;
 
 def Security : Package <"security">;

 def NullablePassedToNonnullChecker : Checker<"NullablePassedToNonnull">,
   HelpText<"Warns when a nullable pointer is passed to a pointer which has a _Nonnull type.">,
   DescFile<"NullabilityChecker.cpp">;
 
 def NullableReturnedFromNonnullChecker : Checker<"NullablePassedToNonnull">,
   HelpText<"Warns when a nullable pointer is returned from a function that has _Nonnull return type.">,
   DescFile<"NullabilityChecker.cpp">;
 
-} // end "alpha.core.nullability"
+} // end "alpha.nullability"
 
 //===----------------------------------------------------------------------===//
 // Evaluate "builtin" functions.
 //===----------------------------------------------------------------------===//
 
 let ParentPackage = CoreBuiltin in {
 
 def NoReturnFunctionChecker : Checker<"NoReturnFunctions">,

cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp

struct NullabilityChecksFilter {
     CheckName CheckNameNullPassedToNonnull;
     CheckName CheckNameNullReturnedFromNonnull;
     CheckName CheckNameNullableDereferenced;
     CheckName CheckNameNullablePassedToNonnull;
     CheckName CheckNameNullableReturnedFromNonnull;
   };
 
   NullabilityChecksFilter Filter;
+  // When set to false no nullability information will be tracked in
+  // NullabilityMap. It is possible to catch errors like passing a null pointer
+  // to a callee that expects nonnull argument without the information that is
+  // stroed in the NullabilityMap. This is an optimization.
+  DefaultBool NeedTracking;
 
 private:
   class NullabilityBugVisitor
       : public BugReporterVisitorImpl<NullabilityBugVisitor> {
   public:
     NullabilityBugVisitor(const MemRegion *M) : Region(M) {}
 
     void Profile(llvm::FoldingSetNodeID &ID) const override {
if (ValueExpr) {
       R->addRange(ValueExpr->getSourceRange());
       if (Error == ErrorKind::NilAssignedToNonnull ||
           Error == ErrorKind::NilPassedToNonnull ||
           Error == ErrorKind::NilReturnedToNonnull)
         bugreporter::trackNullOrUndefValue(N, ValueExpr, *R);
     }
     BR.emitReport(std::move(R));
   }
+
+  /// If an SVal wraps a region that should be tracked, it will return a pointer
+  /// to the wrapped region. Otherwise it will return a nullptr.
+  const SymbolicRegion *getTrackRegion(SVal Val,
+                                       bool CheckSuperRegion = false) const;
 };
 
 class NullabilityState {
 public:
   NullabilityState(Nullability Nullab, const Stmt *Source = nullptr)
       : Nullab(Nullab), Source(Source) {}
 
   const Stmt *getNullabilitySource() const { return Source; }
static NullConstraint getNullConstraint(DefinedOrUnknownSVal Val,
   ConditionTruthVal Nullness = State->isNull(Val);
   if (Nullness.isConstrainedFalse())
     return NullConstraint::IsNotNull;
   if (Nullness.isConstrainedTrue())
     return NullConstraint::IsNull;
   return NullConstraint::Unknown;
 }
 
-// If an SVal wraps a region that should be tracked, it will return a pointer
-// to the wrapped region. Otherwise it will return a nullptr.
-static const SymbolicRegion *getTrackRegion(SVal Val,
-                                            bool CheckSuperRegion = false) {
+const SymbolicRegion *
+NullabilityChecker::getTrackRegion(SVal Val, bool CheckSuperRegion) const {
+  if (!NeedTracking)
+    return nullptr;
+
   auto RegionSVal = Val.getAs<loc::MemRegionVal>();
   if (!RegionSVal)
     return nullptr;
 
   const MemRegion *Region = RegionSVal->getRegion();
 
   if (CheckSuperRegion) {
     if (auto FieldReg = Region->getAs<FieldRegion>())
void NullabilityChecker::printState(raw_ostream &Out, ProgramStateRef State,
 
   for (NullabilityMapTy::iterator I = B.begin(), E = B.end(); I != E; ++I) {
     Out << I->first << " : ";
     I->second.print(Out);
     Out << NL;
   }
 }
 
-#define REGISTER_CHECKER(name)                                                 \
+#define REGISTER_CHECKER(name, trackingRequired)                               \
   void ento::register##name##Checker(CheckerManager &mgr) {                    \
     NullabilityChecker *checker = mgr.registerChecker<NullabilityChecker>();   \
     checker->Filter.Check##name = true;                                        \
     checker->Filter.CheckName##name = mgr.getCurrentCheckName();               \
+    checker->NeedTracking = checker->NeedTracking || trackingRequired;         \
   }
 
-REGISTER_CHECKER(NullPassedToNonnull)
-REGISTER_CHECKER(NullReturnedFromNonnull)
-REGISTER_CHECKER(NullableDereferenced)
-REGISTER_CHECKER(NullablePassedToNonnull)
-REGISTER_CHECKER(NullableReturnedFromNonnull)
+// The checks are likely to be turned on by default and it is possible to do
+// them without tracking any nullability related information. As an optimization
+// no nullability information will be tracked when only these two checks are
+// enables.
+REGISTER_CHECKER(NullPassedToNonnull, false)
+REGISTER_CHECKER(NullReturnedFromNonnull, false)
+
+REGISTER_CHECKER(NullableDereferenced, true)
+REGISTER_CHECKER(NullablePassedToNonnull, true)
+REGISTER_CHECKER(NullableReturnedFromNonnull, true)

cfe/trunk/test/Analysis/nullability.mm

-// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core.nullability -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.nullability -verify %s
 
 #define nil 0
 #define BOOL int
 
 @protocol NSObject
 + (id)alloc;
 - (id)init;
 @end

cfe/trunk/test/Analysis/nullability_nullonly.mm

+// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.nullability.NullPassedToNonnull,alpha.nullability.NullReturnedFromNonnull -verify %s
+
+int getRandom();
+
+typedef struct Dummy { int val; } Dummy;
+
+void takesNullable(Dummy *_Nullable);
+void takesNonnull(Dummy *_Nonnull);
+Dummy *_Nullable returnsNullable();
+
+void testBasicRules() {
+  // The tracking of nullable values is turned off.
+  Dummy *p = returnsNullable();
+  takesNonnull(p); // no warning
+  Dummy *q = 0;
+  if (getRandom()) {
+    takesNullable(q);
+    takesNonnull(q); // expected-warning {{}}
+  }
+}
+
+Dummy *_Nonnull testNullReturn() {
+  Dummy *p = 0;
+  return p; // expected-warning {{}}
+}
+
+void onlyReportFirstPreconditionViolationOnPath() {
+  Dummy *p = 0;
+  takesNonnull(p); // expected-warning {{}}
+  takesNonnull(p); // No warning.
+  // Passing null to nonnull is a sink. Stop the analysis.
+  int i = 0;
+  i = 5 / i; // no warning
+  (void)i;
+}
+
+Dummy *_Nonnull doNotWarnWhenPreconditionIsViolatedInTopFunc(
+    Dummy *_Nonnull p) {
+  if (!p) {
+    Dummy *ret =
+        0; // avoid compiler warning (which is not generated by the analyzer)
+    if (getRandom())
+      return ret; // no warning
+    else
+      return p; // no warning
+  } else {
+    return p;
+  }
+}
+
+Dummy *_Nonnull doNotWarnWhenPreconditionIsViolated(Dummy *_Nonnull p) {
+  if (!p) {
+    Dummy *ret =
+        0; // avoid compiler warning (which is not generated by the analyzer)
+    if (getRandom())
+      return ret; // no warning
+    else
+      return p; // no warning
+  } else {
+    return p;
+  }
+}
+
+void testPreconditionViolationInInlinedFunction(Dummy *p) {
+  doNotWarnWhenPreconditionIsViolated(p);
+}
+
+void inlinedNullable(Dummy *_Nullable p) {
+  if (p) return;
+}
+void inlinedNonnull(Dummy *_Nonnull p) {
+  if (p) return;
+}
+void inlinedUnspecified(Dummy *p) {
+  if (p) return;
+}
+
+Dummy *_Nonnull testDefensiveInlineChecks(Dummy * p) {
+  switch (getRandom()) {
+  case 1: inlinedNullable(p); break;
+  case 2: inlinedNonnull(p); break;
+  case 3: inlinedUnspecified(p); break;
+  }
+  if (getRandom())
+    takesNonnull(p);
+  return p;
+}