From 8a4d3a8337a6c91f2ae6613f67caff9e04d45811 Mon Sep 17 00:00:00 2001
From: Ted Kremenek <kremenek@apple.com>
Date: Wed, 22 Jan 2014 06:10:28 +0000
Subject: [PATCH] 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.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@199790 91177308-0d34-0410-b5e6-96231b3b80d8
---
include/clang/Basic/DiagnosticSemaKinds.td | 3 +
include/clang/Sema/Sema.h | 7 ++-
lib/Sema/SemaChecking.cpp | 67 ++++++++++++++++------
lib/Sema/SemaStmt.cpp | 12 +++-
test/Sema/nonnull.c | 6 ++
test/SemaObjC/nonnull.m | 6 +-
6 files changed, 78 insertions(+), 23 deletions(-)
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index e9780430e64..2d373dec7a1 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/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/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 084942baf65..47e9d9a9cf9 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -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);
diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 4636c92eccb..4ebadb9cf58 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/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,9 +4030,9 @@ 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;
@@ -4029,7 +4040,7 @@ Sema::CheckReturnStackAddr(Expr *RetValExp, QualType lhsType,
// 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/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp
index 50a9d8111da..34a0f84e8f3 100644
--- a/lib/Sema/SemaStmt.cpp
+++ b/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/test/Sema/nonnull.c b/test/Sema/nonnull.c
index 9ec004fdde5..784fd7d08f7 100644
--- a/test/Sema/nonnull.c
+++ b/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/test/SemaObjC/nonnull.m b/test/SemaObjC/nonnull.m
index f6e4f57d2a8..8534fdad633 100644
--- a/test/SemaObjC/nonnull.m
+++ b/test/SemaObjC/nonnull.m
@@ -85,6 +85,7 @@ extern void DoSomethingNotNull(void *db) __attribute__((nonnull(1)));
{
void * vp;
}
+- (void*) testRetNull __attribute__((returns_nonnull));
@end
@implementation IMP
@@ -96,10 +97,13 @@ extern void DoSomethingNotNull(void *db) __attribute__((nonnull(1)));
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}}
--
GitLab