[cfe-commits] r76821 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/Sema.h lib/Sema/SemaDecl.cpp test/Sema/return.c test/SemaCXX/return.cpp test/SemaObjC/return.m
Mike Stump
mrs at apple.com
Wed Jul 22 16:56:58 PDT 2009
Author: mrs
Date: Wed Jul 22 18:56:57 2009
New Revision: 76821
URL: http://llvm.org/viewvc/llvm-project?rev=76821&view=rev
Log:
Add warning for falling off the end of a function that should return a
value. This is on by default, and controlled by -Wreturn-type (-Wmost
-Wall). I believe there should be very few false positives, though
the most interesting case would be:
int() { bar(); }
when bar does:
bar() { while (1) ; }
Here, we assume functions return, unless they are marked with the
noreturn attribute. I can envision a fixit note for functions that
never return normally that don't have a noreturn attribute to add a
noreturn attribute.
If anyone spots other false positives, let me know!
Added:
cfe/trunk/test/SemaCXX/return.cpp
cfe/trunk/test/SemaObjC/return.m
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/Sema.h
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/test/Sema/return.c
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=76821&r1=76820&r2=76821&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Jul 22 18:56:57 2009
@@ -104,6 +104,13 @@
def err_thread_unsupported : Error<
"thread-local storage is unsupported for the current target">;
+def warn_maybe_falloff_nonvoid_function : Warning<
+ "control may reach end of non-void function">,
+ InGroup<ReturnType>;
+def warn_falloff_nonvoid_function : Warning<
+ "control reaches end of non-void function">,
+ InGroup<ReturnType>;
+
/// Built-in functions.
def ext_implicit_lib_function_decl : ExtWarn<
"implicitly declaring C library function '%0' with type %1">;
Modified: cfe/trunk/lib/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.h?rev=76821&r1=76820&r2=76821&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/Sema.h (original)
+++ cfe/trunk/lib/Sema/Sema.h Wed Jul 22 18:56:57 2009
@@ -801,10 +801,14 @@
SourceLocation MemberLoc,
IdentifierInfo &Member);
- /// Helpers for dealing with function parameters.
+ /// Helpers for dealing with functions.
+ void CheckFallThroughForFunctionDef(Decl *D, Stmt *Body);
bool CheckParmsForFunctionDef(FunctionDecl *FD);
void CheckCXXDefaultArguments(FunctionDecl *FD);
void CheckExtraCXXDefaultArguments(Declarator &D);
+ enum ControlFlowKind { NeverFallThrough = 0, MaybeFallThrough = 1,
+ AlwaysFallThrough = 2 };
+ ControlFlowKind CheckFallThrough(Stmt *);
Scope *getNonFieldDeclScope(Scope *S);
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=76821&r1=76820&r2=76821&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Jul 22 18:56:57 2009
@@ -16,10 +16,12 @@
#include "clang/AST/APValue.h"
#include "clang/AST/ASTConsumer.h"
#include "clang/AST/ASTContext.h"
+#include "clang/Analysis/CFG.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/DeclTemplate.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/StmtCXX.h"
+#include "clang/AST/StmtObjc.h"
#include "clang/Parse/DeclSpec.h"
#include "clang/Basic/TargetInfo.h"
#include "clang/Basic/SourceManager.h"
@@ -30,6 +32,7 @@
#include "llvm/ADT/STLExtras.h"
#include <algorithm>
#include <functional>
+#include <queue>
using namespace clang;
/// getDeclName - Return a pretty name for the specified decl if possible, or
@@ -1011,6 +1014,114 @@
New->setPreviousDeclaration(Old);
}
+Sema::ControlFlowKind Sema::CheckFallThrough(Stmt *Root) {
+ llvm::OwningPtr<CFG> cfg (CFG::buildCFG(Root, &Context));
+
+ // FIXME: They should never return 0, fix that, delete this code.
+ if (cfg == 0)
+ return NeverFallThrough;
+ // The CFG leaves in dead things, run a simple mark and sweep on it
+ // to weed out the trivially dead things.
+ std::queue<CFGBlock*> workq;
+ llvm::OwningArrayPtr<char> live(new char[cfg->getNumBlockIDs()]);
+ // Prep work queue
+ workq.push(&cfg->getEntry());
+ // Solve
+ while (!workq.empty()) {
+ CFGBlock *item = workq.front();
+ workq.pop();
+ live[item->getBlockID()] = 1;
+ CFGBlock::succ_iterator i;
+ for (i=item->succ_begin(); i != item->succ_end(); ++i) {
+ if ((*i) && ! live[(*i)->getBlockID()]) {
+ live[(*i)->getBlockID()] = 1;
+ workq.push(*i);
+ }
+ }
+ }
+
+ CFGBlock::succ_iterator i;
+ bool HasLiveReturn = false;
+ bool HasFakeEdge = false;
+ bool HasPlainEdge = false;
+ for (i=cfg->getExit().pred_begin(); i != cfg->getExit().pred_end(); ++i) {
+ if (!live[(*i)->getBlockID()])
+ continue;
+ if ((*i)->size() == 0) {
+ // A labeled empty statement, or the entry block...
+ HasPlainEdge = true;
+ continue;
+ }
+ Stmt *S = (**i)[(*i)->size()-1];
+ if (isa<ReturnStmt>(S)) {
+ HasLiveReturn = true;
+ continue;
+ }
+ if (isa<ObjCAtThrowStmt>(S)) {
+ HasFakeEdge = true;
+ continue;
+ }
+ if (isa<CXXThrowExpr>(S)) {
+ HasFakeEdge = true;
+ continue;
+ }
+ bool NoReturnEdge = false;
+ if (CallExpr *C = dyn_cast<CallExpr>(S))
+ {
+ Expr *CEE = C->getCallee()->IgnoreParenCasts();
+ if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(CEE)) {
+ if (FunctionDecl *FD = dyn_cast<FunctionDecl>(DRE->getDecl())) {
+ if (FD->hasAttr<NoReturnAttr>()) {
+ NoReturnEdge = true;
+ HasFakeEdge = true;
+ }
+ }
+ }
+ }
+ if (NoReturnEdge == false)
+ HasPlainEdge = true;
+ }
+ if (HasPlainEdge) {
+ if (HasFakeEdge|HasLiveReturn)
+ return MaybeFallThrough;
+ // This says never for calls to functions that are not marked noreturn, that
+ // don't return. For people that don't like this, we encourage marking the
+ // functions as noreturn.
+ return AlwaysFallThrough;
+ }
+ return NeverFallThrough;
+}
+
+/// CheckFallThroughForFunctionDef - Check that we don't fall off the end of a
+/// function that should return a value.
+///
+/// \returns Never iff we can never alter control flow (we always fall off the
+/// end of the statement, Conditional iff we might or might not alter
+/// control-flow and Always iff we always alter control flow (we never fall off
+/// the end of the statement).
+void Sema::CheckFallThroughForFunctionDef(Decl *D, Stmt *Body) {
+ // FIXME: Would be nice if we had a better way to control cascding errors, but
+ // for now, avoid them.
+ if (getDiagnostics().hasErrorOccurred())
+ return;
+ if (Diags.getDiagnosticLevel(diag::warn_maybe_falloff_nonvoid_function)
+ == Diagnostic::Ignored)
+ return;
+ // FIXME: Funtion try block
+ if (CompoundStmt *Compound = dyn_cast<CompoundStmt>(Body)) {
+ switch (CheckFallThrough(Body)) {
+ case MaybeFallThrough:
+ Diag(Compound->getRBracLoc(), diag::warn_maybe_falloff_nonvoid_function);
+ break;
+ case AlwaysFallThrough:
+ Diag(Compound->getRBracLoc(), diag::warn_falloff_nonvoid_function);
+ break;
+ case NeverFallThrough:
+ break;
+ }
+ }
+}
+
/// CheckParmsForFunctionDef - Check that the parameters of the given
/// function are appropriate for the definition of a function. This
/// takes care of any checks that cannot be performed on the
@@ -3246,6 +3357,10 @@
Stmt *Body = BodyArg.takeAs<Stmt>();
if (FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(dcl)) {
FD->setBody(Body);
+ if (!FD->getResultType()->isVoidType()
+ // C and C++ allow for main to automagically return 0.
+ && !FD->isMain())
+ CheckFallThroughForFunctionDef(FD, Body);
if (!FD->isInvalidDecl())
DiagnoseUnusedParameters(FD->param_begin(), FD->param_end());
@@ -3260,6 +3375,8 @@
} else if (ObjCMethodDecl *MD = dyn_cast_or_null<ObjCMethodDecl>(dcl)) {
assert(MD == getCurMethodDecl() && "Method parsing confused");
MD->setBody(Body);
+ if (!MD->getResultType()->isVoidType())
+ CheckFallThroughForFunctionDef(MD, Body);
MD->setEndLoc(Body->getLocEnd());
if (!MD->isInvalidDecl())
Modified: cfe/trunk/test/Sema/return.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/return.c?rev=76821&r1=76820&r2=76821&view=diff
==============================================================================
--- cfe/trunk/test/Sema/return.c (original)
+++ cfe/trunk/test/Sema/return.c Wed Jul 22 18:56:57 2009
@@ -10,3 +10,176 @@
void t15() {
return 1; // expected-warning {{void function 't15' should not return a value}}
}
+
+int unknown();
+
+void test0() {
+}
+
+int test1() {
+} // expected-warning {{control reaches end of non-void function}}
+
+int test2() {
+ a: goto a;
+}
+
+int test3() {
+ goto a;
+ a: ;
+} // expected-warning {{control reaches end of non-void function}}
+
+
+void halt() {
+ a: goto a;
+}
+
+void halt2() __attribute__((noreturn));
+
+int test4() {
+ halt2();
+}
+
+int test5() {
+ halt2(), (void)1;
+}
+
+int test6() {
+ 1, halt2();
+}
+
+int j;
+int unknown_nohalt() {
+ return j;
+}
+
+int test7() {
+ unknown();
+} // expected-warning {{control reaches end of non-void function}}
+
+int test8() {
+ (void)(1 + unknown());
+} // expected-warning {{control reaches end of non-void function}}
+
+int halt3() __attribute__((noreturn));
+
+int test9() {
+ (void)(halt3() + unknown());
+}
+
+int test10() {
+ (void)(unknown() || halt3());
+} // expected-warning {{control may reach end of non-void function}}
+
+int test11() {
+ (void)(unknown() && halt3());
+} // expected-warning {{control may reach end of non-void function}}
+
+int test12() {
+ (void)(halt3() || unknown());
+}
+
+int test13() {
+ (void)(halt3() && unknown());
+}
+
+int test14() {
+ (void)(1 || unknown());
+} // expected-warning {{control reaches end of non-void function}}
+
+int test15() {
+ (void)(0 || unknown());
+} // expected-warning {{control reaches end of non-void function}}
+
+int test16() {
+ (void)(0 && unknown());
+} // expected-warning {{control reaches end of non-void function}}
+
+int test17() {
+ (void)(1 && unknown());
+} // expected-warning {{control reaches end of non-void function}}
+
+int test18() {
+ (void)(unknown_nohalt() && halt3());
+} // expected-warning {{control may reach end of non-void function}}
+
+int test19() {
+ (void)(unknown_nohalt() && unknown());
+} // expected-warning {{control reaches end of non-void function}}
+
+int test20() {
+ int i;
+ if (i)
+ return 0;
+ else if (0)
+ return 2;
+} // expected-warning {{control may reach end of non-void function}}
+
+int test21() {
+ int i;
+ if (i)
+ return 0;
+ else if (1)
+ return 2;
+}
+
+int test22() {
+ int i;
+ switch (i) default: ;
+} // expected-warning {{control reaches end of non-void function}}
+
+int test23() {
+ int i;
+ switch (i) {
+ case 0:
+ return 0;
+ case 2:
+ return 2;
+ }
+} // expected-warning {{control may reach end of non-void function}}
+
+int test24() {
+ int i;
+ switch (i) {
+ case 0:
+ return 0;
+ case 2:
+ return 2;
+ default:
+ return -1;
+ }
+}
+
+int test25() {
+ 1 ? halt3() : unknown();
+}
+
+int test26() {
+ 0 ? halt3() : unknown();
+} // expected-warning {{control reaches end of non-void function}}
+
+int j;
+int test27() {
+ switch (j) {
+ case 1:
+ do { } while (1);
+ break;
+ case 2:
+ for (;;) ;
+ break;
+ case 3:
+ for (;1;) ;
+ for (;0;) {
+ goto done;
+ }
+ return 1;
+ case 4:
+ while (0) { goto done; }
+ return 1;
+ case 5:
+ while (1) { return 1; }
+ break;
+ default:
+ return 1;
+ }
+ done: ;
+}
Added: cfe/trunk/test/SemaCXX/return.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/return.cpp?rev=76821&view=auto
==============================================================================
--- cfe/trunk/test/SemaCXX/return.cpp (added)
+++ cfe/trunk/test/SemaCXX/return.cpp Wed Jul 22 18:56:57 2009
@@ -0,0 +1,5 @@
+// RUN: clang-cc %s -fsyntax-only -verify
+
+int test1() {
+ throw;
+}
Added: cfe/trunk/test/SemaObjC/return.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/return.m?rev=76821&view=auto
==============================================================================
--- cfe/trunk/test/SemaObjC/return.m (added)
+++ cfe/trunk/test/SemaObjC/return.m Wed Jul 22 18:56:57 2009
@@ -0,0 +1,6 @@
+// RUN: clang-cc %s -fsyntax-only -verify
+
+int test1() {
+ id a;
+ @throw a;
+}
More information about the cfe-commits
mailing list