r199790 - Add basic checking for returning null from functions/methods marked 'returns_nonnull'.
Ted Kremenek
kremenek at apple.com
Tue Jan 21 22:10:28 PST 2014
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}}
More information about the cfe-commits
mailing list