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 <[email protected]> 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 > +@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 > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
