From: Ted Kremenek Date: Fri, 17 Jan 2014 06:24:56 +0000 (+0000) Subject: Enhance attribute 'nonnull' to be applicable to parameters directly (infix). X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=f5a99a4db7b22c7566333b6ee676c903f963a1d6;p=clang Enhance attribute 'nonnull' to be applicable to parameters directly (infix). This allows the following syntax: void baz(__attribute__((nonnull)) const char *str); instead of: void baz(const char *str) __attribute__((nonnull(1))); This also extends to Objective-C methods. The checking logic in Sema is not as clean as I would like. Effectively now we need to check both the FunctionDecl/ObjCMethodDecl and the parameters, so the point of truth is spread in two places, but the logic isn't that cumbersome. Implements . git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@199467 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/Attr.td b/include/clang/Basic/Attr.td index 6426bc1ab2..cb739fb99f 100644 --- a/include/clang/Basic/Attr.td +++ b/include/clang/Basic/Attr.td @@ -679,8 +679,9 @@ def NoMips16 : InheritableAttr, TargetSpecificAttr { def NonNull : InheritableAttr { let Spellings = [GNU<"nonnull">, CXX11<"gnu", "nonnull">]; - let Subjects = SubjectList<[ObjCMethod, FunctionLike, HasFunctionProto], - WarnDiag, "ExpectedFunctionOrMethod">; + let Subjects = SubjectList<[ObjCMethod, FunctionLike, HasFunctionProto, + ParmVar], + WarnDiag, "ExpectedFunctionMethodOrParameter">; let Args = [VariadicUnsignedArgument<"Args">]; let AdditionalMembers = [{bool isNonNull(unsigned idx) const { diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 77807cec83..a421725441 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -2351,6 +2351,9 @@ def err_attr_wrong_decl : Error< def warn_attribute_nonnull_no_pointers : Warning< "'nonnull' attribute applied to function with no pointer arguments">, InGroup; +def warn_attribute_nonnull_parm_no_args : Warning< + "'nonnull' attribute when used on parameters takes no arguments">, + InGroup; def warn_attribute_malloc_pointer_only : Warning< "'malloc' attribute only applies to functions returning a pointer type">, InGroup; diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index 714e975e42..c3acd52a1a 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -736,6 +736,7 @@ static void CheckNonNullArguments(Sema &S, const NamedDecl *FDecl, const Expr * const *ExprArgs, SourceLocation CallSiteLoc) { + // Check the attributes attached to the method/function itself. for (specific_attr_iterator I = FDecl->specific_attr_begin(), E = FDecl->specific_attr_end(); I != E; ++I) { @@ -747,6 +748,21 @@ static void CheckNonNullArguments(Sema &S, CheckNonNullArgument(S, ExprArgs[*i], CallSiteLoc); } } + + // Check the attributes on the parameters. + ArrayRef parms; + if (const FunctionDecl *FD = dyn_cast(FDecl)) + parms = FD->parameters(); + else if (const ObjCMethodDecl *MD = dyn_cast(FDecl)) + parms = MD->parameters(); + + unsigned argIndex = 0; + for (ArrayRef::iterator I = parms.begin(), E = parms.end(); + I != E; ++I, ++argIndex) { + const ParmVarDecl *PVD = *I; + if (PVD->hasAttr()) + CheckNonNullArgument(S, ExprArgs[argIndex], CallSiteLoc); + } } /// Handles the checks for format strings, non-POD arguments to vararg diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp index b8116a7e8a..0629067221 100644 --- a/lib/Sema/SemaDeclAttr.cpp +++ b/lib/Sema/SemaDeclAttr.cpp @@ -1156,6 +1156,36 @@ static void possibleTransparentUnionPointerType(QualType &T) { } } +static bool attrNonNullArgCheck(Sema &S, QualType T, const AttributeList &Attr, + SourceRange R) { + T = T.getNonReferenceType(); + possibleTransparentUnionPointerType(T); + + if (!T->isAnyPointerType() && !T->isBlockPointerType()) { + S.Diag(Attr.getLoc(), diag::warn_attribute_pointers_only) + << Attr.getName() << R; + return false; + } + return true; +} + +static void handleNonNullAttrParameter(Sema &S, ParmVarDecl *D, + const AttributeList &Attr) { + // Is the argument a pointer type? + if (!attrNonNullArgCheck(S, D->getType(), Attr, D->getSourceRange())) + return; + + if (Attr.getNumArgs() > 0) { + S.Diag(Attr.getLoc(), diag::warn_attribute_nonnull_parm_no_args) + << D->getSourceRange(); + return; + } + + D->addAttr(::new (S.Context) + NonNullAttr(Attr.getRange(), S.Context, 0, 0, + Attr.getAttributeSpellingListIndex())); +} + static void handleNonNullAttr(Sema &S, Decl *D, const AttributeList &Attr) { SmallVector NonNullArgs; for (unsigned i = 0; i < Attr.getNumArgs(); ++i) { @@ -1165,15 +1195,10 @@ static void handleNonNullAttr(Sema &S, Decl *D, const AttributeList &Attr) { return; // Is the function argument a pointer type? - QualType T = getFunctionOrMethodArgType(D, Idx).getNonReferenceType(); - possibleTransparentUnionPointerType(T); - - if (!T->isAnyPointerType() && !T->isBlockPointerType()) { - // FIXME: Should also highlight argument in decl. - S.Diag(Attr.getLoc(), diag::warn_attribute_pointers_only) - << Attr.getName() << Ex->getSourceRange(); + // FIXME: Should also highlight argument in decl in the diagnostic. + if (!attrNonNullArgCheck(S, getFunctionOrMethodArgType(D, Idx), + Attr, Ex->getSourceRange())) continue; - } NonNullArgs.push_back(Idx); } @@ -3947,7 +3972,12 @@ static void ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, case AttributeList::AT_Mode: handleModeAttr (S, D, Attr); break; case AttributeList::AT_NoCommon: handleSimpleAttribute(S, D, Attr); break; - case AttributeList::AT_NonNull: handleNonNullAttr (S, D, Attr); break; + case AttributeList::AT_NonNull: + if (ParmVarDecl *PVD = dyn_cast(D)) + handleNonNullAttrParameter(S, PVD, Attr); + else + handleNonNullAttr(S, D, Attr); + break; case AttributeList::AT_Overloadable: handleSimpleAttribute(S, D, Attr); break; case AttributeList::AT_Ownership: handleOwnershipAttr (S, D, Attr); break; diff --git a/test/Sema/nonnull.c b/test/Sema/nonnull.c index 70d3d39f8b..b134b1fc66 100644 --- a/test/Sema/nonnull.c +++ b/test/Sema/nonnull.c @@ -21,3 +21,14 @@ int main(void) { void foo(const char *str) __attribute__((nonnull("foo"))); // expected-error{{'nonnull' attribute requires parameter 1 to be an integer constant}} void bar(int i) __attribute__((nonnull(1))); // expected-warning {{'nonnull' attribute only applies to pointer arguments}} expected-warning {{'nonnull' attribute applied to function with no pointer arguments}} + +void baz(__attribute__((nonnull)) const char *str); +void baz2(__attribute__((nonnull(1))) const char *str); // expected-warning {{'nonnull' attribute when used on parameters takes no arguments}} +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}} + baz2(0); // no-warning + baz3(0); // no-warning +} + diff --git a/test/SemaObjC/nonnull.m b/test/SemaObjC/nonnull.m index 902105b924..d9ecbdc37e 100644 --- a/test/SemaObjC/nonnull.m +++ b/test/SemaObjC/nonnull.m @@ -95,3 +95,17 @@ extern void DoSomethingNotNull(void *db) __attribute__((nonnull(1))); } @end +__attribute__((objc_root_class)) + @interface TestNonNullParameters +- (void) doNotPassNullParameterNonPointerArg:(int)__attribute__((nonnull))x; // expected-warning {{'nonnull' attribute only applies to pointer arguments}} +- (void) doNotPassNullParameter:(id)__attribute__((nonnull))x; +- (void) doNotPassNullParameterArgIndex:(id)__attribute__((nonnull(1)))x; // expected-warning {{'nonnull' attribute when used on parameters takes no arguments}} +- (void) doNotPassNullOnMethod:(id)x __attribute__((nonnull(1))); +@end + +void test(TestNonNullParameters *f) { + [f doNotPassNullParameter:0]; // expected-warning {{null passed to a callee which 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}} +} +