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. llvm-svn: 199790
llvm · Jan 22, 2014 · ef9e7f8 · ef9e7f8
1 parent 4675351
commit ef9e7f8
Show file tree

Hide file tree

Showing 6 changed files with 78 additions and 23 deletions.
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6225,6 +6225,9 @@ 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">,
   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<

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
@@ -7899,8 +7899,11 @@ class Sema {
   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);

diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
@@ -713,22 +713,33 @@ bool Sema::getFormatStringInfo(const FormatAttr *Format, bool IsCXXMember,
   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,17 +4030,17 @@ static Expr *EvalAddr(Expr* E, SmallVectorImpl<DeclRefExpr *> &refVars,
 
 /// 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;
 
   // 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 *RetValExp, QualType lhsType,
   }
 
   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 *RetValExp, QualType lhsType,
     // 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 ==.

diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
@@ -2650,7 +2650,7 @@ Sema::ActOnCapScopeReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
       return StmtError();
     }
     RetValExp = Res.take();
-    CheckReturnStackAddr(RetValExp, FnRetType, ReturnLoc);
+    CheckReturnValExpr(RetValExp, FnRetType, ReturnLoc);
   }
 
   if (RetValExp) {
@@ -2774,13 +2774,21 @@ Sema::ActOnReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
 
   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 ReturnLoc, Expr *RetValExp) {
         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

diff --git a/clang/test/Sema/nonnull.c b/clang/test/Sema/nonnull.c
@@ -39,3 +39,9 @@ void *test_ptr_returns_nonnull(void) __attribute__((returns_nonnull)); // no-war
 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}}
+}
+
diff --git a/clang/test/SemaObjC/nonnull.m b/clang/test/SemaObjC/nonnull.m
@@ -85,6 +85,7 @@ @interface IMP
 {
   void * vp;
 }
+- (void*) testRetNull __attribute__((returns_nonnull));
 @end
 
 @implementation IMP
@@ -96,10 +97,13 @@ - (void) Meth {
   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}}