From: Douglas Gregor Date: Fri, 19 Jun 2015 18:13:19 +0000 (+0000) Subject: Diagnose unsafe uses of nil and __nonnull pointers. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=1bfd760890a3349d7350ae87e53f9700e1e924f8;p=clang Diagnose unsafe uses of nil and __nonnull pointers. This generalizes the checking of null arguments to also work with values of pointer-to-function, reference-to-function, and block pointer type, using the nullability information within the underling function prototype to extend non-null checking, and diagnoses returns of 'nil' within a function with a __nonnull return type. Note that we don't warn about nil returns from Objective-C methods, because it's common for Objective-C methods to mimic the nil-swallowing behavior of the receiver by checking ostensibly non-null parameters and returning nil from otherwise non-null methods in that case. It also diagnoses (via a separate flag) conversions from nullable to nonnull pointers. It's a separate flag because this warning can be noisy. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@240153 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td index 84ffe09bbe..296b5f05d8 100644 --- a/include/clang/Basic/DiagnosticGroups.td +++ b/include/clang/Basic/DiagnosticGroups.td @@ -250,6 +250,7 @@ def ModuleConflict : DiagGroup<"module-conflict">; def NewlineEOF : DiagGroup<"newline-eof">; def Nullability : DiagGroup<"nullability">; def NullabilityDeclSpec : DiagGroup<"nullability-declspec">; +def NullableToNonNullConversion : DiagGroup<"nullable-to-nonnull-conversion">; def NullArithmetic : DiagGroup<"null-arithmetic">; def NullCharacter : DiagGroup<"null-character">; def NullDereference : DiagGroup<"null-dereference">; diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index ac534436e7..a4c6f4d4fb 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -7700,6 +7700,11 @@ def err_nullability_conflicting : Error< "'%select{__nonnull|__nullable|__null_unspecified}0' conflicts with existing " "specifier '%select{__nonnull|__nullable|__null_unspecified}1'">; +def warn_nullability_lost : Warning< + "implicit conversion from nullable pointer %0 to non-nullable pointer " + "type %1">, + InGroup, DefaultIgnore; + } } // end of sema component. diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index a05aa98761..ec47a2603c 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -8580,9 +8580,10 @@ private: const FunctionProtoType *Proto, SourceLocation Loc); - void checkCall(NamedDecl *FDecl, ArrayRef Args, - unsigned NumParams, bool IsMemberFunction, SourceLocation Loc, - SourceRange Range, VariadicCallType CallType); + void checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto, + ArrayRef Args, bool IsMemberFunction, + SourceLocation Loc, SourceRange Range, + VariadicCallType CallType); bool CheckObjCString(Expr *Arg); diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp index 46fec7324b..db1251f097 100644 --- a/lib/Sema/Sema.cpp +++ b/lib/Sema/Sema.cpp @@ -356,6 +356,19 @@ ExprResult Sema::ImpCastExprToType(Expr *E, QualType Ty, assert((VK == VK_RValue || !E->isRValue()) && "can't cast rvalue to lvalue"); #endif + // Check whether we're implicitly casting from a nullable type to a nonnull + // type. + if (auto exprNullability = E->getType()->getNullability(Context)) { + if (*exprNullability == NullabilityKind::Nullable) { + if (auto typeNullability = Ty->getNullability(Context)) { + if (*typeNullability == NullabilityKind::NonNull) { + Diag(E->getLocStart(), diag::warn_nullability_lost) + << E->getType() << Ty; + } + } + } + } + QualType ExprTy = Context.getCanonicalType(E->getType()); QualType TypeTy = Context.getCanonicalType(Ty); diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index 20140525bb..f76727cad8 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -1115,6 +1115,13 @@ bool Sema::getFormatStringInfo(const FormatAttr *Format, bool IsCXXMember, /// \brief Returns true if the value evaluates to null. static bool CheckNonNullExpr(Sema &S, const Expr *Expr) { + // If the expression has non-null type, it doesn't evaluate to null. + if (auto nullability + = Expr->IgnoreImplicit()->getType()->getNullability(S.Context)) { + if (*nullability == NullabilityKind::NonNull) + return false; + } + // As a special case, transparent unions initialized with zero are // considered null for the purposes of the nonnull attribute. if (const RecordType *UT = Expr->getType()->getAsUnionType()) { @@ -1190,56 +1197,111 @@ DiagnoseCStringFormatDirectiveInCFAPI(Sema &S, } } +/// Determine whether the given type has a non-null nullability annotation. +static bool isNonNullType(ASTContext &ctx, QualType type) { + if (auto nullability = type->getNullability(ctx)) + return *nullability == NullabilityKind::NonNull; + + return false; +} + static void CheckNonNullArguments(Sema &S, const NamedDecl *FDecl, + const FunctionProtoType *Proto, ArrayRef Args, SourceLocation CallSiteLoc) { + assert((FDecl || Proto) && "Need a function declaration or prototype"); + // Check the attributes attached to the method/function itself. llvm::SmallBitVector NonNullArgs; - for (const auto *NonNull : FDecl->specific_attrs()) { - if (!NonNull->args_size()) { - // Easy case: all pointer arguments are nonnull. - for (const auto *Arg : Args) - if (S.isValidPointerAttrType(Arg->getType())) - CheckNonNullArgument(S, Arg, CallSiteLoc); - return; - } + if (FDecl) { + // Handle the nonnull attribute on the function/method declaration itself. + for (const auto *NonNull : FDecl->specific_attrs()) { + if (!NonNull->args_size()) { + // Easy case: all pointer arguments are nonnull. + for (const auto *Arg : Args) + if (S.isValidPointerAttrType(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); + 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. - 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() || - (ArgIndex < NonNullArgs.size() && NonNullArgs[ArgIndex])) - CheckNonNullArgument(S, Args[ArgIndex], CallSiteLoc); + if (FDecl && (isa(FDecl) || isa(FDecl))) { + // Handle the nonnull attribute on the parameters of the + // function/method. + ArrayRef parms; + if (const FunctionDecl *FD = dyn_cast(FDecl)) + parms = FD->parameters(); + else + parms = cast(FDecl)->parameters(); + + unsigned ParamIndex = 0; + for (ArrayRef::iterator I = parms.begin(), E = parms.end(); + I != E; ++I, ++ParamIndex) { + const ParmVarDecl *PVD = *I; + if (PVD->hasAttr() || + isNonNullType(S.Context, PVD->getType())) { + if (NonNullArgs.empty()) + NonNullArgs.resize(Args.size()); + + NonNullArgs.set(ParamIndex); + } + } + } else { + // If we have a non-function, non-method declaration but no + // function prototype, try to dig out the function prototype. + if (!Proto) { + if (const ValueDecl *VD = dyn_cast(FDecl)) { + QualType type = VD->getType().getNonReferenceType(); + if (auto pointerType = type->getAs()) + type = pointerType->getPointeeType(); + else if (auto blockType = type->getAs()) + type = blockType->getPointeeType(); + // FIXME: data member pointers? + + // Dig out the function prototype, if there is one. + Proto = type->getAs(); + } + } + + // Fill in non-null argument information from the nullability + // information on the parameter types (if we have them). + if (Proto) { + unsigned Index = 0; + for (auto paramType : Proto->getParamTypes()) { + if (isNonNullType(S.Context, paramType)) { + if (NonNullArgs.empty()) + NonNullArgs.resize(Args.size()); + + NonNullArgs.set(Index); + } + + ++Index; + } + } } - // In case this is a variadic call, check any remaining arguments. - for (/**/; ArgIndex < NonNullArgs.size(); ++ArgIndex) + // Check for non-null arguments. + for (unsigned ArgIndex = 0, ArgIndexEnd = NonNullArgs.size(); + ArgIndex != ArgIndexEnd; ++ArgIndex) { if (NonNullArgs[ArgIndex]) CheckNonNullArgument(S, Args[ArgIndex], CallSiteLoc); + } } /// Handles the checks for format strings, non-POD arguments to vararg /// functions, and NULL arguments passed to non-NULL parameters. -void Sema::checkCall(NamedDecl *FDecl, ArrayRef Args, - unsigned NumParams, bool IsMemberFunction, +void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto, + ArrayRef Args, bool IsMemberFunction, SourceLocation Loc, SourceRange Range, VariadicCallType CallType) { // FIXME: We should check as much as we can in the template definition. @@ -1261,6 +1323,13 @@ void Sema::checkCall(NamedDecl *FDecl, ArrayRef Args, // Refuse POD arguments that weren't caught by the format string // checks above. if (CallType != VariadicDoesNotApply) { + unsigned NumParams = Proto ? Proto->getNumParams() + : FDecl && isa(FDecl) + ? cast(FDecl)->getNumParams() + : FDecl && isa(FDecl) + ? cast(FDecl)->param_size() + : 0; + for (unsigned ArgIdx = NumParams; ArgIdx < Args.size(); ++ArgIdx) { // Args[ArgIdx] can be null in malformed code. if (const Expr *Arg = Args[ArgIdx]) { @@ -1270,12 +1339,14 @@ void Sema::checkCall(NamedDecl *FDecl, ArrayRef Args, } } - if (FDecl) { - CheckNonNullArguments(*this, FDecl, Args, Loc); + if (FDecl || Proto) { + CheckNonNullArguments(*this, FDecl, Proto, Args, Loc); // Type safety checking. - for (const auto *I : FDecl->specific_attrs()) - CheckArgumentWithTypeTag(I, Args.data()); + if (FDecl) { + for (const auto *I : FDecl->specific_attrs()) + CheckArgumentWithTypeTag(I, Args.data()); + } } } @@ -1287,8 +1358,8 @@ void Sema::CheckConstructorCall(FunctionDecl *FDecl, SourceLocation Loc) { VariadicCallType CallType = Proto->isVariadic() ? VariadicConstructor : VariadicDoesNotApply; - checkCall(FDecl, Args, Proto->getNumParams(), - /*IsMemberFunction=*/true, Loc, SourceRange(), CallType); + checkCall(FDecl, Proto, Args, /*IsMemberFunction=*/true, Loc, SourceRange(), + CallType); } /// CheckFunctionCall - Check a direct function call for various correctness @@ -1301,7 +1372,6 @@ bool Sema::CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall, IsMemberOperatorCall; VariadicCallType CallType = getVariadicCallType(FDecl, Proto, TheCall->getCallee()); - unsigned NumParams = Proto ? Proto->getNumParams() : 0; Expr** Args = TheCall->getArgs(); unsigned NumArgs = TheCall->getNumArgs(); if (IsMemberOperatorCall) { @@ -1311,7 +1381,7 @@ bool Sema::CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall, ++Args; --NumArgs; } - checkCall(FDecl, llvm::makeArrayRef(Args, NumArgs), NumParams, + checkCall(FDecl, Proto, llvm::makeArrayRef(Args, NumArgs), IsMemberFunction, TheCall->getRParenLoc(), TheCall->getCallee()->getSourceRange(), CallType); @@ -1345,9 +1415,9 @@ bool Sema::CheckObjCMethodCall(ObjCMethodDecl *Method, SourceLocation lbrac, VariadicCallType CallType = Method->isVariadic() ? VariadicMethod : VariadicDoesNotApply; - checkCall(Method, Args, Method->param_size(), - /*IsMemberFunction=*/false, - lbrac, Method->getSourceRange(), CallType); + checkCall(Method, nullptr, Args, + /*IsMemberFunction=*/false, lbrac, Method->getSourceRange(), + CallType); return false; } @@ -1356,13 +1426,14 @@ bool Sema::CheckPointerCall(NamedDecl *NDecl, CallExpr *TheCall, const FunctionProtoType *Proto) { QualType Ty; if (const auto *V = dyn_cast(NDecl)) - Ty = V->getType(); + Ty = V->getType().getNonReferenceType(); else if (const auto *F = dyn_cast(NDecl)) - Ty = F->getType(); + Ty = F->getType().getNonReferenceType(); else return false; - if (!Ty->isBlockPointerType() && !Ty->isFunctionPointerType()) + if (!Ty->isBlockPointerType() && !Ty->isFunctionPointerType() && + !Ty->isFunctionProtoType()) return false; VariadicCallType CallType; @@ -1373,11 +1444,10 @@ bool Sema::CheckPointerCall(NamedDecl *NDecl, CallExpr *TheCall, } else { // Ty->isFunctionPointerType() CallType = VariadicFunction; } - unsigned NumParams = Proto ? Proto->getNumParams() : 0; - checkCall(NDecl, llvm::makeArrayRef(TheCall->getArgs(), - TheCall->getNumArgs()), - NumParams, /*IsMemberFunction=*/false, TheCall->getRParenLoc(), + checkCall(NDecl, Proto, + llvm::makeArrayRef(TheCall->getArgs(), TheCall->getNumArgs()), + /*IsMemberFunction=*/false, TheCall->getRParenLoc(), TheCall->getCallee()->getSourceRange(), CallType); return false; @@ -1388,11 +1458,9 @@ bool Sema::CheckPointerCall(NamedDecl *NDecl, CallExpr *TheCall, bool Sema::CheckOtherCall(CallExpr *TheCall, const FunctionProtoType *Proto) { VariadicCallType CallType = getVariadicCallType(/*FDecl=*/nullptr, Proto, TheCall->getCallee()); - unsigned NumParams = Proto ? Proto->getNumParams() : 0; - - checkCall(/*FDecl=*/nullptr, + checkCall(/*FDecl=*/nullptr, Proto, llvm::makeArrayRef(TheCall->getArgs(), TheCall->getNumArgs()), - NumParams, /*IsMemberFunction=*/false, TheCall->getRParenLoc(), + /*IsMemberFunction=*/false, TheCall->getRParenLoc(), TheCall->getCallee()->getSourceRange(), CallType); return false; @@ -5680,7 +5748,8 @@ Sema::CheckReturnValExpr(Expr *RetValExp, QualType lhsType, CheckReturnStackAddr(*this, RetValExp, lhsType, ReturnLoc); // Check if the return value is null but should not be. - if (Attrs && hasSpecificAttr(*Attrs) && + if (((Attrs && hasSpecificAttr(*Attrs)) || + (!isObjCMethod && isNonNullType(Context, lhsType))) && CheckNonNullExpr(*this, RetValExp)) Diag(ReturnLoc, diag::warn_null_ret) << (isObjCMethod ? 1 : 0) << RetValExp->getSourceRange(); diff --git a/lib/Sema/SemaOverload.cpp b/lib/Sema/SemaOverload.cpp index f8610e009b..a0fdcd78e5 100644 --- a/lib/Sema/SemaOverload.cpp +++ b/lib/Sema/SemaOverload.cpp @@ -11260,7 +11260,7 @@ Sema::CreateOverloadedBinOp(SourceLocation OpLoc, if (Op == OO_Equal) DiagnoseSelfMove(Args[0], Args[1], OpLoc); - checkCall(FnDecl, ArgsArray, 0, isa(FnDecl), OpLoc, + checkCall(FnDecl, nullptr, ArgsArray, isa(FnDecl), OpLoc, TheCall->getSourceRange(), VariadicDoesNotApply); return MaybeBindToTemporary(TheCall); diff --git a/test/Sema/non-null-warning.c b/test/Sema/non-null-warning.c index 0cabc713ba..6cd98e298e 100644 --- a/test/Sema/non-null-warning.c +++ b/test/Sema/non-null-warning.c @@ -33,3 +33,10 @@ int *foo3(int * __nonnull x) { // expected-warning {{nullability specifier '__no return 0; } +int * ret_nonnull() { + return 0; // expected-warning {{null returned from function that requires a non-null return value}} +} + +int main () { + foo(0); // expected-warning {{null passed to a callee that requires a non-null argument}} +} diff --git a/test/Sema/nullability.c b/test/Sema/nullability.c index bf35b4ca1a..6144b7e8e9 100644 --- a/test/Sema/nullability.c +++ b/test/Sema/nullability.c @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fsyntax-only -fblocks -Wno-nullability-declspec %s -verify +// RUN: %clang_cc1 -fsyntax-only -fblocks -Wnullable-to-nonnull-conversion -Wno-nullability-declspec %s -verify #if __has_feature(nullability) #else @@ -85,3 +85,29 @@ void printing_nullability(void) { int * __nullable * __nonnull iptrptr2; float * *fptrptr2 = iptrptr2; // expected-warning{{incompatible pointer types initializing 'float **' with an expression of type 'int * __nullable * __nonnull'}} } + +// Check passing null to a __nonnull argument. +void accepts_nonnull_1(__nonnull int *ptr); +void (*accepts_nonnull_2)(__nonnull int *ptr); +void (^accepts_nonnull_3)(__nonnull int *ptr); + +void test_accepts_nonnull_null_pointer_literal() { + accepts_nonnull_1(0); // expected-warning{{null passed to a callee that requires a non-null argument}} + accepts_nonnull_2(0); // expected-warning{{null passed to a callee that requires a non-null argument}} + accepts_nonnull_3(0); // expected-warning{{null passed to a callee that requires a non-null argument}} +} + +// Check returning nil from a __nonnull-returning function. +__nonnull int *returns_int_ptr(int x) { + if (x) { + return 0; // expected-warning{{null returned from function that requires a non-null return value}} + } + + return (__nonnull int *)0; +} + +// Check nullable-to-nonnull conversions. +void nullable_to_nonnull(__nullable int *ptr) { + int *a = ptr; // okay + __nonnull int *b = ptr; // expected-warning{{implicit conversion from nullable pointer 'int * __nullable' to non-nullable pointer type 'int * __nonnull'}} +} diff --git a/test/SemaCXX/nullability.cpp b/test/SemaCXX/nullability.cpp index 391675fed7..f0aa13be8f 100644 --- a/test/SemaCXX/nullability.cpp +++ b/test/SemaCXX/nullability.cpp @@ -40,3 +40,25 @@ struct AddNonNull2 { // cannot apply to that specific dependent type. typedef __nonnull AddNonNull (*invalid4); // expected-error{{nullability specifier '__nonnull' cannot be applied to non-pointer type 'AddNonNull'}} }; + +// Check passing null to a __nonnull argument. +void (*accepts_nonnull_1)(__nonnull int *ptr); +void (*& accepts_nonnull_2)(__nonnull int *ptr) = accepts_nonnull_1; +void (X::* accepts_nonnull_3)(__nonnull int *ptr); +void accepts_nonnull_4(__nonnull int *ptr); +void (&accepts_nonnull_5)(__nonnull int *ptr) = accepts_nonnull_4; + +void test_accepts_nonnull_null_pointer_literal(X *x) { + accepts_nonnull_1(0); // expected-warning{{null passed to a callee that requires a non-null argument}} + accepts_nonnull_2(0); // expected-warning{{null passed to a callee that requires a non-null argument}} + (x->*accepts_nonnull_3)(0); // expected-warning{{null passed to a callee that requires a non-null argument}} + accepts_nonnull_4(0); // expected-warning{{null passed to a callee that requires a non-null argument}} + accepts_nonnull_5(0); // expected-warning{{null passed to a callee that requires a non-null argument}} +} + +template +void test_accepts_nonnull_null_pointer_literal_template() { + FP(0); // expected-warning{{null passed to a callee that requires a non-null argument}} +} + +template void test_accepts_nonnull_null_pointer_literal_template<&accepts_nonnull_4>(); // expected-note{{instantiation of function template specialization}} diff --git a/test/SemaObjC/nullability-arc.m b/test/SemaObjC/nullability-arc.m new file mode 100644 index 0000000000..917a808386 --- /dev/null +++ b/test/SemaObjC/nullability-arc.m @@ -0,0 +1,10 @@ +// RUN: %clang_cc1 -fobjc-arc -fsyntax-only -Woverriding-method-mismatch %s -verify + +__attribute__((objc_root_class)) +@interface NSFoo +@end + +// ARC qualifiers stacked with nullability. +void accepts_arc_qualified(NSFoo * __unsafe_unretained __nonnull obj) { + accepts_arc_qualified(0); // expected-warning{{null passed to a callee that requires a non-null argument}} +} diff --git a/test/SemaObjC/nullability.m b/test/SemaObjC/nullability.m index a93072f3dc..0bcc0cb8fa 100644 --- a/test/SemaObjC/nullability.m +++ b/test/SemaObjC/nullability.m @@ -1,6 +1,9 @@ // RUN: %clang_cc1 -fsyntax-only -fblocks -Woverriding-method-mismatch -Wno-nullability-declspec %s -verify +__attribute__((objc_root_class)) @interface NSFoo +- (void)methodTakingIntPtr:(__nonnull int *)ptr; +- (__nonnull int *)methodReturningIntPtr; @end // Nullability applies to all pointer types. @@ -17,3 +20,14 @@ typedef __nonnull NSFoo * __nullable conflict_NSFoo_ptr_2; // expected-error{{'_ void testBlocksPrinting(NSFoo * __nullable (^bp)(int)) { int *ip = bp; // expected-error{{'NSFoo * __nullable (^)(int)'}} } +void test_accepts_nonnull_null_pointer_literal(NSFoo *foo) { + [foo methodTakingIntPtr: 0]; // expected-warning{{null passed to a callee that requires a non-null argument}} +} + +// Check returning nil from a __nonnull-returning method. +@implementation NSFoo +- (void)methodTakingIntPtr:(__nonnull int *)ptr { } +- (__nonnull int *)methodReturningIntPtr { + return 0; // no warning +} +@end