r199790 - Add basic checking for returning null from functions/methods marked 'returns_nonnull'.
Jordan Rose
jordan_rose at apple.com
Wed Jan 22 09:21:22 PST 2014
Should this be combined with the check that a throwing operator new never returns non-null? (r199452)
On Jan 21, 2014, at 22:10 , Ted Kremenek <kremenek at apple.com> wrote:
> Author: kremenek
> Date: Wed Jan 22 00:10:28 2014
> New Revision: 199790
>
> URL: http://llvm.org/viewvc/llvm-project?rev=199790&view=rev
> Log:
> Add basic checking for returning null from functions/methods marked 'returns_nonnull'.
>
> This involved making CheckReturnStackAddr into a static function, which
> is now called by a top-level return value checking routine called
> CheckReturnValExpr.
>
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/include/clang/Sema/Sema.h
> cfe/trunk/lib/Sema/SemaChecking.cpp
> cfe/trunk/lib/Sema/SemaStmt.cpp
> cfe/trunk/test/Sema/nonnull.c
> cfe/trunk/test/SemaObjC/nonnull.m
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=199790&r1=199789&r2=199790&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Jan 22 00:10:28 2014
> @@ -6225,6 +6225,9 @@ def note_printf_c_str: Note<"did you mea
> def warn_null_arg : Warning<
> "null passed to a callee which 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">,
> + InGroup<NonNull>;
>
> // CHECK: returning address/reference of stack memory
> def warn_ret_stack_addr : Warning<
>
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=199790&r1=199789&r2=199790&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Wed Jan 22 00:10:28 2014
> @@ -7899,8 +7899,11 @@ private:
> void CheckStrncatArguments(const CallExpr *Call,
> IdentifierInfo *FnName);
>
> - void CheckReturnStackAddr(Expr *RetValExp, QualType lhsType,
> - SourceLocation ReturnLoc);
> + void CheckReturnValExpr(Expr *RetValExp, QualType lhsType,
> + SourceLocation ReturnLoc,
> + bool isObjCMethod = false,
> + const AttrVec *Attrs = 0);
> +
> void CheckFloatComparison(SourceLocation Loc, Expr* LHS, Expr* RHS);
> void CheckImplicitConversions(Expr *E, SourceLocation CC = SourceLocation());
> void CheckForIntOverflow(Expr *E);
>
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=199790&r1=199789&r2=199790&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Wed Jan 22 00:10:28 2014
> @@ -713,22 +713,33 @@ bool Sema::getFormatStringInfo(const For
> return true;
> }
>
> -static void CheckNonNullArgument(Sema &S,
> - const Expr *ArgExpr,
> - SourceLocation CallSiteLoc) {
> +/// Checks if a the given expression evaluates to null.
> +///
> +/// \brief Returns true if the value evaluates to null.
> +static bool CheckNonNullExpr(Sema &S,
> + const Expr *Expr) {
> // As a special case, transparent unions initialized with zero are
> // considered null for the purposes of the nonnull attribute.
> - if (const RecordType *UT = ArgExpr->getType()->getAsUnionType()) {
> + if (const RecordType *UT = Expr->getType()->getAsUnionType()) {
> if (UT->getDecl()->hasAttr<TransparentUnionAttr>())
> if (const CompoundLiteralExpr *CLE =
> - dyn_cast<CompoundLiteralExpr>(ArgExpr))
> + dyn_cast<CompoundLiteralExpr>(Expr))
> if (const InitListExpr *ILE =
> dyn_cast<InitListExpr>(CLE->getInitializer()))
> - ArgExpr = ILE->getInit(0);
> + Expr = ILE->getInit(0);
> }
>
> bool Result;
> - if (ArgExpr->EvaluateAsBooleanCondition(Result, S.Context) && !Result)
> + if (Expr->EvaluateAsBooleanCondition(Result, S.Context) && !Result)
> + return true;
> +
> + return false;
> +}
> +
> +static void CheckNonNullArgument(Sema &S,
> + const Expr *ArgExpr,
> + SourceLocation CallSiteLoc) {
> + if (CheckNonNullExpr(S, ArgExpr))
> S.Diag(CallSiteLoc, diag::warn_null_arg) << ArgExpr->getSourceRange();
> }
>
> @@ -4019,9 +4030,9 @@ static Expr *EvalAddr(Expr* E, SmallVect
>
> /// CheckReturnStackAddr - Check if a return statement returns the address
> /// of a stack variable.
> -void
> -Sema::CheckReturnStackAddr(Expr *RetValExp, QualType lhsType,
> - SourceLocation ReturnLoc) {
> +static void
> +CheckReturnStackAddr(Sema &S, Expr *RetValExp, QualType lhsType,
> + SourceLocation ReturnLoc) {
>
> Expr *stackE = 0;
> SmallVector<DeclRefExpr *, 8> refVars;
> @@ -4029,7 +4040,7 @@ Sema::CheckReturnStackAddr(Expr *RetValE
> // Perform checking for returned stack addresses, local blocks,
> // label addresses or references to temporaries.
> if (lhsType->isPointerType() ||
> - (!getLangOpts().ObjCAutoRefCount && lhsType->isBlockPointerType())) {
> + (!S.getLangOpts().ObjCAutoRefCount && lhsType->isBlockPointerType())) {
> stackE = EvalAddr(RetValExp, refVars, /*ParentDecl=*/0);
> } else if (lhsType->isReferenceType()) {
> stackE = EvalVal(RetValExp, refVars, /*ParentDecl=*/0);
> @@ -4053,16 +4064,16 @@ Sema::CheckReturnStackAddr(Expr *RetValE
> }
>
> if (DeclRefExpr *DR = dyn_cast<DeclRefExpr>(stackE)) { //address of local var.
> - Diag(diagLoc, lhsType->isReferenceType() ? diag::warn_ret_stack_ref
> + S.Diag(diagLoc, lhsType->isReferenceType() ? diag::warn_ret_stack_ref
> : diag::warn_ret_stack_addr)
> << DR->getDecl()->getDeclName() << diagRange;
> } else if (isa<BlockExpr>(stackE)) { // local block.
> - Diag(diagLoc, diag::err_ret_local_block) << diagRange;
> + S.Diag(diagLoc, diag::err_ret_local_block) << diagRange;
> } else if (isa<AddrLabelExpr>(stackE)) { // address of label.
> - Diag(diagLoc, diag::warn_ret_addr_label) << diagRange;
> + S.Diag(diagLoc, diag::warn_ret_addr_label) << diagRange;
> } else { // local temporary.
> - Diag(diagLoc, lhsType->isReferenceType() ? diag::warn_ret_local_temp_ref
> - : diag::warn_ret_local_temp_addr)
> + S.Diag(diagLoc, lhsType->isReferenceType() ? diag::warn_ret_local_temp_ref
> + : diag::warn_ret_local_temp_addr)
> << diagRange;
> }
>
> @@ -4075,8 +4086,8 @@ Sema::CheckReturnStackAddr(Expr *RetValE
> // show the range of the expression.
> SourceRange range = (i < e-1) ? refVars[i+1]->getSourceRange()
> : stackE->getSourceRange();
> - Diag(VD->getLocation(), diag::note_ref_var_local_bind)
> - << VD->getDeclName() << range;
> + S.Diag(VD->getLocation(), diag::note_ref_var_local_bind)
> + << VD->getDeclName() << range;
> }
> }
>
> @@ -4371,6 +4382,26 @@ do {
> } while (true);
> }
>
> +void
> +Sema::CheckReturnValExpr(Expr *RetValExp, QualType lhsType,
> + SourceLocation ReturnLoc,
> + bool isObjCMethod,
> + const AttrVec *Attrs) {
> + CheckReturnStackAddr(*this, RetValExp, lhsType, ReturnLoc);
> +
> + // Check if the return value is null but should not be.
> + if (Attrs)
> + for (specific_attr_iterator<ReturnsNonNullAttr>
> + I = specific_attr_iterator<ReturnsNonNullAttr>(Attrs->begin()),
> + E = specific_attr_iterator<ReturnsNonNullAttr>(Attrs->end());
> + I != E; ++I) {
> + if (CheckNonNullExpr(*this, RetValExp))
> + Diag(ReturnLoc, diag::warn_null_ret)
> + << (isObjCMethod ? 1 : 0) << RetValExp->getSourceRange();
> + break;
> + }
> +}
> +
> //===--- CHECK: Floating-Point comparisons (-Wfloat-equal) ---------------===//
>
> /// Check for comparisons of floating point operands using != and ==.
>
> Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=199790&r1=199789&r2=199790&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaStmt.cpp Wed Jan 22 00:10:28 2014
> @@ -2650,7 +2650,7 @@ Sema::ActOnCapScopeReturnStmt(SourceLoca
> return StmtError();
> }
> RetValExp = Res.take();
> - CheckReturnStackAddr(RetValExp, FnRetType, ReturnLoc);
> + CheckReturnValExpr(RetValExp, FnRetType, ReturnLoc);
> }
>
> if (RetValExp) {
> @@ -2774,13 +2774,21 @@ Sema::ActOnReturnStmt(SourceLocation Ret
>
> QualType FnRetType;
> QualType RelatedRetType;
> + const AttrVec *Attrs = 0;
> + bool isObjCMethod = false;
> +
> if (const FunctionDecl *FD = getCurFunctionDecl()) {
> FnRetType = FD->getResultType();
> + if (FD->hasAttrs())
> + Attrs = &FD->getAttrs();
> if (FD->isNoReturn())
> Diag(ReturnLoc, diag::warn_noreturn_function_has_return_expr)
> << FD->getDeclName();
> } else if (ObjCMethodDecl *MD = getCurMethodDecl()) {
> FnRetType = MD->getResultType();
> + isObjCMethod = true;
> + if (MD->hasAttrs())
> + Attrs = &MD->getAttrs();
> if (MD->hasRelatedResultType() && MD->getClassInterface()) {
> // In the implementation of a method with a related return type, the
> // type used to type-check the validity of return statements within the
> @@ -2935,7 +2943,7 @@ Sema::ActOnReturnStmt(SourceLocation Ret
> RetValExp = Res.takeAs<Expr>();
> }
>
> - CheckReturnStackAddr(RetValExp, FnRetType, ReturnLoc);
> + CheckReturnValExpr(RetValExp, FnRetType, ReturnLoc, isObjCMethod, Attrs);
>
> // C++11 [basic.stc.dynamic.allocation]p4:
> // If an allocation function declared with a non-throwing
>
> Modified: cfe/trunk/test/Sema/nonnull.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/nonnull.c?rev=199790&r1=199789&r2=199790&view=diff
> ==============================================================================
> --- cfe/trunk/test/Sema/nonnull.c (original)
> +++ cfe/trunk/test/Sema/nonnull.c Wed Jan 22 00:10:28 2014
> @@ -39,3 +39,9 @@ void *test_ptr_returns_nonnull(void) __a
> int i __attribute__((nonnull)); // expected-warning {{'nonnull' attribute only applies to functions, methods, and parameters}}
> int j __attribute__((returns_nonnull)); // expected-warning {{'returns_nonnull' attribute only applies to functions and methods}}
> void *test_no_fn_proto() __attribute__((returns_nonnull)); // expected-warning {{'returns_nonnull' attribute only applies to functions and methods}}
> +
> +__attribute__((returns_nonnull))
> +void *test_bad_returns_null(void) {
> + return 0; // expected-warning {{null returned from function that requires a non-null return value}}
> +}
> +
>
> Modified: cfe/trunk/test/SemaObjC/nonnull.m
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/nonnull.m?rev=199790&r1=199789&r2=199790&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaObjC/nonnull.m (original)
> +++ cfe/trunk/test/SemaObjC/nonnull.m Wed Jan 22 00:10:28 2014
> @@ -85,6 +85,7 @@ extern void DoSomethingNotNull(void *db)
> {
> void * vp;
> }
> +- (void*) testRetNull __attribute__((returns_nonnull));
> @end
>
> @implementation IMP
> @@ -96,10 +97,13 @@ extern void DoSomethingNotNull(void *db)
> DoSomethingNotNull(NULL); // expected-warning {{null passed to a callee which requires a non-null argument}}
> [object doSomethingWithNonNullPointer:vp:1:vp];
> }
> +- (void*) testRetNull {
> + return 0; // expected-warning {{null returned from method that requires a non-null return value}}
> +}
> @end
>
> __attribute__((objc_root_class))
> - @interface TestNonNullParameters
> + at 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}}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list