Fix representation of __attribute__((nonnull)) to support correctly modeling

the no-arguments case. Don't expand this to an __attribute__((nonnull(A, B,
C))) attribute, since that does the wrong thing for function templates and
varargs functions.

In passing, fix a grammar error in the diagnostic, a crash if
__attribute__((nonnull(N))) is applied to a varargs function,
a bug where the same null argument could be diagnosed multiple
times if there were multiple nonnull attributes referring to it,
and a bug where nonnull attributes would not be accumulated correctly
across redeclarations.

llvm-svn: 216520

clang/include/clang/Basic/Attr.td

@@ -845,11 +845,15 @@ def NonNull : InheritableAttr {
   let Args = [VariadicUnsignedArgument<"Args">];
   let AdditionalMembers =
 [{bool isNonNull(unsigned idx) const {
+    if (!args_size())
+      return true;
     for (const auto &V : args())
       if (V == idx)
         return true;
     return false;
   } }];
+  // FIXME: We should merge duplicates into a single nonnull attribute.
+  let DuplicatesAllowedWhileMerging = 1;
   let Documentation = [Undocumented];
 }
 

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -6470,7 +6470,7 @@ def note_format_fix_specifier : Note<"did you mean to use '%0'?">;
 def note_printf_c_str: Note<"did you mean to call the %0 method?">;
 
 def warn_null_arg : Warning<
-  "null passed to a callee which requires a non-null argument">,
+  "null passed to a callee that requires a non-null argument">,
   InGroup<NonNull>;
 def warn_null_ret : Warning<
   "null returned from %select{function|method}0 that requires a non-null return value">,

clang/include/clang/Sema/Sema.h

@@ -129,7 +129,6 @@ namespace clang {
   class ModuleLoader;
   class MultiLevelTemplateArgumentList;
   class NamedDecl;
-  class NonNullAttr;
   class ObjCCategoryDecl;
   class ObjCCategoryImplDecl;
   class ObjCCompatibleAliasDecl;
@@ -2706,6 +2705,9 @@ public:
 
   void checkUnusedDeclAttributes(Declarator &D);
 
+  /// Determine if type T is a valid subject for a nonnull attribute.
+  bool isValidNonNullAttrType(QualType T);
+
   bool CheckRegparmAttr(const AttributeList &attr, unsigned &value);
   bool CheckCallingConvAttr(const AttributeList &attr, CallingConv &CC, 
                             const FunctionDecl *FD = nullptr);

clang/lib/Sema/SemaChecking.cpp

@@ -764,12 +764,26 @@ static void CheckNonNullArgument(Sema &S,
 
 static void CheckNonNullArguments(Sema &S,
                                   const NamedDecl *FDecl,
-                                  const Expr * const *ExprArgs,
+                                  ArrayRef<const Expr *> Args,
                                   SourceLocation CallSiteLoc) {
   // Check the attributes attached to the method/function itself.
+  llvm::SmallBitVector NonNullArgs;
   for (const auto *NonNull : FDecl->specific_attrs<NonNullAttr>()) {
-    for (const auto &Val : NonNull->args())
-      CheckNonNullArgument(S, ExprArgs[Val], CallSiteLoc);
+    if (!NonNull->args_size()) {
+      // Easy case: all pointer arguments are nonnull.
+      for (const auto *Arg : Args)
+        if (S.isValidNonNullAttrType(Arg->getType()))
+          CheckNonNullArgument(S, Arg, CallSiteLoc);
+      return;
+    }
+
+    for (unsigned Val : NonNull->args()) {
+      if (Val >= Args.size())
+        continue;
+      if (NonNullArgs.empty())
+        NonNullArgs.resize(Args.size());
+      NonNullArgs.set(Val);
+    }
   }
 
   // Check the attributes on the parameters.
@@ -779,13 +793,19 @@ static void CheckNonNullArguments(Sema &S,
   else if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(FDecl))
     parms = MD->parameters();
 
-  unsigned argIndex = 0;
+  unsigned ArgIndex = 0;
   for (ArrayRef<ParmVarDecl*>::iterator I = parms.begin(), E = parms.end();
-       I != E; ++I, ++argIndex) {
+       I != E; ++I, ++ArgIndex) {
     const ParmVarDecl *PVD = *I;
-    if (PVD->hasAttr<NonNullAttr>())
-      CheckNonNullArgument(S, ExprArgs[argIndex], CallSiteLoc);
+    if (PVD->hasAttr<NonNullAttr>() ||
+        (ArgIndex < NonNullArgs.size() && NonNullArgs[ArgIndex]))
+      CheckNonNullArgument(S, Args[ArgIndex], CallSiteLoc);
   }
+
+  // In case this is a variadic call, check any remaining arguments.
+  for (/**/; ArgIndex < NonNullArgs.size(); ++ArgIndex)
+    if (NonNullArgs[ArgIndex])
+      CheckNonNullArgument(S, Args[ArgIndex], CallSiteLoc);
 }
 
 /// Handles the checks for format strings, non-POD arguments to vararg
@@ -823,7 +843,7 @@ void Sema::checkCall(NamedDecl *FDecl, ArrayRef<const Expr *> Args,
   }
 
   if (FDecl) {
-    CheckNonNullArguments(*this, FDecl, Args.data(), Loc);
+    CheckNonNullArguments(*this, FDecl, Args, Loc);
 
     // Type safety checking.
     for (const auto *I : FDecl->specific_attrs<ArgumentWithTypeTagAttr>())

clang/lib/Sema/SemaDeclAttr.cpp

@@ -1125,28 +1125,30 @@ static void handleIBOutletCollection(Sema &S, Decl *D,
                                     Attr.getAttributeSpellingListIndex()));
 }
 
-static void possibleTransparentUnionPointerType(QualType &T) {
-  if (const RecordType *UT = T->getAsUnionType())
+bool Sema::isValidNonNullAttrType(QualType T) {
+  T = T.getNonReferenceType();
+
+  // The nonnull attribute can be applied to a transparent union that
+  // contains a pointer type.
+  if (const RecordType *UT = T->getAsUnionType()) {
     if (UT && UT->getDecl()->hasAttr<TransparentUnionAttr>()) {
       RecordDecl *UD = UT->getDecl();
       for (const auto *I : UD->fields()) {
         QualType QT = I->getType();
-        if (QT->isAnyPointerType() || QT->isBlockPointerType()) {
-          T = QT;
-          return;
-        }
+        if (QT->isAnyPointerType() || QT->isBlockPointerType())
+          return true;
       }
     }
+  }
+
+  return T->isAnyPointerType() || T->isBlockPointerType();
 }
 
 static bool attrNonNullArgCheck(Sema &S, QualType T, const AttributeList &Attr,
                                 SourceRange AttrParmRange,
                                 SourceRange NonNullTypeRange,
                                 bool isReturnValue = false) {
-  T = T.getNonReferenceType();
-  possibleTransparentUnionPointerType(T);
-
-  if (!T->isAnyPointerType() && !T->isBlockPointerType()) {
+  if (!S.isValidNonNullAttrType(T)) {
     S.Diag(Attr.getLoc(), isReturnValue
                               ? diag::warn_attribute_return_pointers_only
                               : diag::warn_attribute_pointers_only)
@@ -1158,14 +1160,15 @@ static bool attrNonNullArgCheck(Sema &S, QualType T, const AttributeList &Attr,
 
 static void handleNonNullAttr(Sema &S, Decl *D, const AttributeList &Attr) {
   SmallVector<unsigned, 8> NonNullArgs;
-  for (unsigned i = 0; i < Attr.getNumArgs(); ++i) {
-    Expr *Ex = Attr.getArgAsExpr(i);
+  for (unsigned I = 0; I < Attr.getNumArgs(); ++I) {
+    Expr *Ex = Attr.getArgAsExpr(I);
     uint64_t Idx;
-    if (!checkFunctionOrMethodParameterIndex(S, D, Attr, i + 1, Ex, Idx))
+    if (!checkFunctionOrMethodParameterIndex(S, D, Attr, I + 1, Ex, Idx))
       return;
 
     // Is the function argument a pointer type?
-    if (!attrNonNullArgCheck(S, getFunctionOrMethodParamType(D, Idx), Attr,
+    if (Idx < getFunctionOrMethodNumParams(D) &&
+        !attrNonNullArgCheck(S, getFunctionOrMethodParamType(D, Idx), Attr,
                              Ex->getSourceRange(),
                              getFunctionOrMethodParamRange(D, Idx)))
       continue;
@@ -1174,30 +1177,28 @@ static void handleNonNullAttr(Sema &S, Decl *D, const AttributeList &Attr) {
   }
 
   // If no arguments were specified to __attribute__((nonnull)) then all pointer
-  // arguments have a nonnull attribute.
-  if (NonNullArgs.empty()) {
-    for (unsigned i = 0, e = getFunctionOrMethodNumParams(D); i != e; ++i) {
-      QualType T = getFunctionOrMethodParamType(D, i).getNonReferenceType();
-      possibleTransparentUnionPointerType(T);
-      if (T->isAnyPointerType() || T->isBlockPointerType())
-        NonNullArgs.push_back(i);
+  // arguments have a nonnull attribute; warn if there aren't any. Skip this
+  // check if the attribute came from a macro expansion or a template
+  // instantiation.
+  if (NonNullArgs.empty() && Attr.getLoc().isFileID() &&
+      S.ActiveTemplateInstantiations.empty()) {
+    bool AnyPointers = isFunctionOrMethodVariadic(D);
+    for (unsigned I = 0, E = getFunctionOrMethodNumParams(D);
+         I != E && !AnyPointers; ++I) {
+      QualType T = getFunctionOrMethodParamType(D, I);
+      if (T->isDependentType() || S.isValidNonNullAttrType(T))
+        AnyPointers = true;
     }
 
-    // No pointer arguments?
-    if (NonNullArgs.empty()) {
-      // Warn the trivial case only if attribute is not coming from a
-      // macro instantiation.
-      if (Attr.getLoc().isFileID())
-        S.Diag(Attr.getLoc(), diag::warn_attribute_nonnull_no_pointers);
-      return;
-    }
+    if (!AnyPointers)
+      S.Diag(Attr.getLoc(), diag::warn_attribute_nonnull_no_pointers);
   }
 
-  unsigned *start = &NonNullArgs[0];
-  unsigned size = NonNullArgs.size();
-  llvm::array_pod_sort(start, start + size);
+  unsigned *Start = NonNullArgs.data();
+  unsigned Size = NonNullArgs.size();
+  llvm::array_pod_sort(Start, Start + Size);
   D->addAttr(::new (S.Context)
-             NonNullAttr(Attr.getRange(), S.Context, start, size,
+             NonNullAttr(Attr.getRange(), S.Context, Start, Size,
                          Attr.getAttributeSpellingListIndex()));
 }
 

clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp

@@ -49,6 +49,8 @@ void NonNullParamChecker::checkPreCall(const CallEvent &Call,
   if (!FD)
     return;
 
+  // FIXME: This is wrong; there can be multiple attributes with different sets
+  // of non-null parameter indices.
   const NonNullAttr *Att = FD->getAttr<NonNullAttr>();
 
   ProgramStateRef state = C.getState();

clang/test/Sema/nonnull.c

@@ -15,7 +15,7 @@ __attribute__((nonnull(1))) void Class_init(Instance this, char *str) {
 
 int main(void) {
 	Class *obj;
-	Class_init(0, "Hello World"); // expected-warning {{null passed to a callee which requires a non-null argument}}
+	Class_init(0, "Hello World"); // expected-warning {{null passed to a callee that requires a non-null argument}}
 	Class_init(obj, "Hello World");
 }
 
@@ -27,7 +27,7 @@ void baz2(__attribute__((nonnull(1))) const char *str); // expected-warning {{'n
 void baz3(__attribute__((nonnull)) int x); // expected-warning {{'nonnull' attribute only applies to pointer arguments}}
 
 void test_baz() {
-  baz(0); // expected-warning {{null passed to a callee which requires a non-null argument}}
+  baz(0); // expected-warning {{null passed to a callee that requires a non-null argument}}
   baz2(0); // no-warning
   baz3(0); // no-warning
 }
@@ -47,10 +47,39 @@ void *test_bad_returns_null(void) {
 }
 
 void PR18795(int (*g)(const char *h, ...) __attribute__((nonnull(1))) __attribute__((nonnull))) {
-  g(0); // expected-warning{{null passed to a callee which requires a non-null argument}}
+  g(0); // expected-warning{{null passed to a callee that requires a non-null argument}}
 }
 void PR18795_helper() {
-  PR18795(0); // expected-warning{{null passed to a callee which requires a non-null argument}}
+  PR18795(0); // expected-warning{{null passed to a callee that requires a non-null argument}}
 }
 
+void vararg1(int n, ...) __attribute__((nonnull(2)));
+void vararg1_test() {
+  vararg1(0);
+  vararg1(1, (void*)0); // expected-warning{{null passed}}
+  vararg1(2, (void*)0, (void*)0); // expected-warning{{null passed}}
+  vararg1(2, (void*)&vararg1, (void*)0);
+}
 
+void vararg2(int n, ...) __attribute__((nonnull, nonnull, nonnull));
+void vararg2_test() {
+  vararg2(0);
+  vararg2(1, (void*)0); // expected-warning{{null passed}}
+  vararg2(2, (void*)0, (void*)0); // expected-warning 2{{null passed}}
+}
+
+void vararg3(int n, ...) __attribute__((nonnull, nonnull(2), nonnull(3)));
+void vararg3_test() {
+  vararg3(0);
+  vararg3(1, (void*)0); // expected-warning{{null passed}}
+  vararg3(2, (void*)0, (void*)0); // expected-warning 2{{null passed}}
+}
+
+void redecl(void *, void *);
+void redecl(void *, void *) __attribute__((nonnull(1)));
+void redecl(void *, void *) __attribute__((nonnull(2)));
+void redecl(void *, void *);
+void redecl_test(void *p) {
+  redecl(p, 0); // expected-warning{{null passed}}
+  redecl(0, p); // expected-warning{{null passed}}
+}

clang/test/Sema/static-array.c

@@ -11,13 +11,13 @@ void f(int *p) {
 
   cat0(0);
 
-  cat(0); // expected-warning {{null passed to a callee which requires a non-null argument}}
+  cat(0); // expected-warning {{null passed to a callee that requires a non-null argument}}
   cat(a); // expected-warning {{array argument is too small; contains 2 elements, callee requires at least 3}}
   cat(b);
   cat(c);
   cat(p);
 
-  vat(1, 0); // expected-warning {{null passed to a callee which requires a non-null argument}}
+  vat(1, 0); // expected-warning {{null passed to a callee that requires a non-null argument}}
   vat(3, b);
 }
 

clang/test/SemaCXX/attr-nonnull.cpp

@@ -27,8 +27,8 @@ namespace rdar8769025 {
   __attribute__((nonnull(2))) void f2(int i, int * const &p);
 
   void test_f1() {
-    f1(0); // expected-warning{{null passed to a callee which requires a non-null argument}}
-    f2(0, 0); // expected-warning{{null passed to a callee which requires a non-null argument}}
+    f1(0); // expected-warning{{null passed to a callee that requires a non-null argument}}
+    f2(0, 0); // expected-warning{{null passed to a callee that requires a non-null argument}}
   }
 }
 

clang/test/SemaCXX/nonnull.cpp

@@ -13,3 +13,8 @@ struct TS {
   }
 };
 
+namespace Template {
+  template<typename T> __attribute__((nonnull)) void f(T t);
+  void g() { f((void*)0); } // expected-warning {{null passed to a callee that requires a non-null argument}}
+  void h() { f(0); }
+}

clang/test/SemaObjC/nonnull.m

@@ -31,16 +31,16 @@ foo (int i1, int i2, int i3, void (^cp1)(), void (^cp2)(), void (^cp3)())
 {
   func1(cp1, cp2, i1);
   
-  func1(0, cp2, i1);  // expected-warning {{null passed to a callee which requires a non-null argument}}
-  func1(cp1, 0, i1);  // expected-warning {{null passed to a callee which requires a non-null argument}}
+  func1(0, cp2, i1);  // expected-warning {{null passed to a callee that requires a non-null argument}}
+  func1(cp1, 0, i1);  // expected-warning {{null passed to a callee that requires a non-null argument}}
   func1(cp1, cp2, 0);
   
   
-  func3(0, i2, cp3, i3); // expected-warning {{null passed to a callee which requires a non-null argument}}
-  func3(cp3, i2, 0, i3);  // expected-warning {{null passed to a callee which requires a non-null argument}}
+  func3(0, i2, cp3, i3); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  func3(cp3, i2, 0, i3);  // expected-warning {{null passed to a callee that requires a non-null argument}}
   
-  func4(0, cp1); // expected-warning {{null passed to a callee which requires a non-null argument}}
-  func4(cp1, 0); // expected-warning {{null passed to a callee which requires a non-null argument}}
+  func4(0, cp1); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  func4(cp1, 0); // expected-warning {{null passed to a callee that requires a non-null argument}}
   
   // Shouldn't these emit warnings?  Clang doesn't, and neither does GCC.  It
   // seems that the checking should handle Objective-C pointers.
@@ -64,7 +64,7 @@ __attribute__((nonnull))
 void _dispatch_queue_push_list(dispatch_object_t _head); // no warning
 
 void func6(dispatch_object_t _head) {
-  _dispatch_queue_push_list(0); // expected-warning {{null passed to a callee which requires a non-null argument}}
+  _dispatch_queue_push_list(0); // expected-warning {{null passed to a callee that requires a non-null argument}}
   _dispatch_queue_push_list(_head._do);  // no warning
 }
 
@@ -91,10 +91,10 @@ extern void DoSomethingNotNull(void *db) __attribute__((nonnull(1)));
 @implementation IMP
 - (void) Meth {
   NSObject *object;
-  [object doSomethingWithNonNullPointer:NULL:1:NULL]; // expected-warning 2 {{null passed to a callee which requires a non-null argument}}
-  [object doSomethingWithNonNullPointer:vp:1:NULL]; // expected-warning {{null passed to a callee which requires a non-null argument}}
-  [NSObject doSomethingClassyWithNonNullPointer:NULL]; // expected-warning {{null passed to a callee which requires a non-null argument}}
-  DoSomethingNotNull(NULL); // expected-warning {{null passed to a callee which requires a non-null argument}}
+  [object doSomethingWithNonNullPointer:NULL:1:NULL]; // expected-warning 2 {{null passed to a callee that requires a non-null argument}}
+  [object doSomethingWithNonNullPointer:vp:1:NULL]; // expected-warning {{null passed to a callee that requires a non-null argument}}
+  [NSObject doSomethingClassyWithNonNullPointer:NULL]; // expected-warning {{null passed to a callee that requires a non-null argument}}
+  DoSomethingNotNull(NULL); // expected-warning {{null passed to a callee that requires a non-null argument}}
   [object doSomethingWithNonNullPointer:vp:1:vp];
 }
 - (void*) testRetNull {
@@ -111,15 +111,15 @@ __attribute__((objc_root_class))
 @end
 
 void test(TestNonNullParameters *f) {
-  [f doNotPassNullParameter:0]; // expected-warning {{null passed to a callee which requires a non-null argument}}
+  [f doNotPassNullParameter:0]; // expected-warning {{null passed to a callee that requires a non-null argument}}
   [f doNotPassNullParameterArgIndex:0]; // no-warning
-  [f doNotPassNullOnMethod:0]; // expected-warning {{null passed to a callee which requires a non-null argument}}
+  [f doNotPassNullOnMethod:0]; // expected-warning {{null passed to a callee that requires a non-null argument}}
 }
 
 
 void PR18795(int (^g)(const char *h, ...) __attribute__((nonnull(1))) __attribute__((nonnull))) {
-  g(0); // expected-warning{{null passed to a callee which requires a non-null argument}}
+  g(0); // expected-warning{{null passed to a callee that requires a non-null argument}}
 }
 void PR18795_helper() {
-  PR18795(0); // expected-warning{{null passed to a callee which requires a non-null argument}}
+  PR18795(0); // expected-warning{{null passed to a callee that requires a non-null argument}}
 }