From: Richard Smith Date: Wed, 27 Aug 2014 04:59:42 +0000 (+0000) Subject: Fix representation of __attribute__((nonnull)) to support correctly modeling X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=2a1084ce0e325230a0f480f2b47ff1128dd98612;p=clang 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@216520 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/Attr.td b/include/clang/Basic/Attr.td index 309f00aa26..fed2b60c50 100644 --- a/include/clang/Basic/Attr.td +++ b/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]; } diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index ca4e4b9b0d..077345f1ae 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/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; def warn_null_ret : Warning< "null returned from %select{function|method}0 that requires a non-null return value">, diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index a9f043d5bf..7652d0afc5 100644 --- a/include/clang/Sema/Sema.h +++ b/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); diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index 331c5b31a7..c1263d45eb 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/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 Args, SourceLocation CallSiteLoc) { // Check the attributes attached to the method/function itself. + llvm::SmallBitVector NonNullArgs; for (const auto *NonNull : FDecl->specific_attrs()) { - 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(FDecl)) parms = MD->parameters(); - unsigned argIndex = 0; + unsigned ArgIndex = 0; for (ArrayRef::iterator I = parms.begin(), E = parms.end(); - I != E; ++I, ++argIndex) { + I != E; ++I, ++ArgIndex) { const ParmVarDecl *PVD = *I; - if (PVD->hasAttr()) - CheckNonNullArgument(S, ExprArgs[argIndex], CallSiteLoc); + if (PVD->hasAttr() || + (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 Args, } if (FDecl) { - CheckNonNullArguments(*this, FDecl, Args.data(), Loc); + CheckNonNullArguments(*this, FDecl, Args, Loc); // Type safety checking. for (const auto *I : FDecl->specific_attrs()) diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp index a1afbdba7a..7c21206fd4 100644 --- a/lib/Sema/SemaDeclAttr.cpp +++ b/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()) { 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 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())); } diff --git a/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp b/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp index 61d2b87cb0..650aa8b875 100644 --- a/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp +++ b/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(); ProgramStateRef state = C.getState(); diff --git a/test/Sema/nonnull.c b/test/Sema/nonnull.c index b1f6920824..3b654a8890 100644 --- a/test/Sema/nonnull.c +++ b/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}} +} diff --git a/test/Sema/static-array.c b/test/Sema/static-array.c index 5ca693b2bf..304485d5af 100644 --- a/test/Sema/static-array.c +++ b/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); } diff --git a/test/SemaCXX/attr-nonnull.cpp b/test/SemaCXX/attr-nonnull.cpp index 8af49d9d29..8fce99759b 100644 --- a/test/SemaCXX/attr-nonnull.cpp +++ b/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}} } } diff --git a/test/SemaCXX/nonnull.cpp b/test/SemaCXX/nonnull.cpp index 9ff6d115bd..97b5455342 100644 --- a/test/SemaCXX/nonnull.cpp +++ b/test/SemaCXX/nonnull.cpp @@ -13,3 +13,8 @@ struct TS { } }; +namespace Template { + template __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); } +} diff --git a/test/SemaObjC/nonnull.m b/test/SemaObjC/nonnull.m index a345eddbf1..cacca07240 100644 --- a/test/SemaObjC/nonnull.m +++ b/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}} }