[cfe-dev] [analyzer] introducing exceptions checkers

[Kirill Bobyrev via cfe-dev]

I've done the initial version of exceptions checkers from the potential 
checkers list <http://clang-analyzer.llvm.org/potential_checkers.html>. 
I actually found /exceptions.ThrowSpecButNotThrow/ and 
/exceptions.NoThrowSpecButThrows/ very similar and decided to put them 
together into /exceptions.ExceptionSpec/. It checks for "throw calls" in 
functions specified as noexcept and functions, which do not have "throw 
calls" but should have such calls inside them by declaration.

The first version of checker is attached. I wonder if it's good enough 
and I can push it for the review.

Feedback and suggestions highly appreciated!

--
Kirill Bobyrev
https://github.com/omtcyf0
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20150906/0e73903e/attachment.html>
-------------- next part --------------
diff --git a/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index 3fc11be..b0c016e 100644
--- a/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -33,6 +33,7 @@ add_clang_library(clangStaticAnalyzerCheckers
   DirectIvarAssignment.cpp
   DivZeroChecker.cpp
   DynamicTypePropagation.cpp
+  ExceptionSpecChecker.cpp
   ExprInspectionChecker.cpp
   FixedAddressChecker.cpp
   GenericTaintChecker.cpp
diff --git a/lib/StaticAnalyzer/Checkers/Checkers.td b/lib/StaticAnalyzer/Checkers/Checkers.td
index 2632b31..a6dce30 100644
--- a/lib/StaticAnalyzer/Checkers/Checkers.td
+++ b/lib/StaticAnalyzer/Checkers/Checkers.td
@@ -47,6 +47,8 @@ def Containers : Package<"containers">, InPackage<CoreFoundation>;
 def LLVM : Package<"llvm">;
 def Debug : Package<"debug">;

+def Exceptions : Package<"exceptions">;
+
 //===----------------------------------------------------------------------===//
 // Core Checkers.
 //===----------------------------------------------------------------------===//
@@ -584,3 +586,15 @@ def ExplodedGraphViewer : Checker<"ViewExplodedGraph">,
   DescFile<"DebugCheckers.cpp">;

 } // end "debug"
+
+//===----------------------------------------------------------------------===//
+// Exceptions checkers.
+//===----------------------------------------------------------------------===//
+
+let ParentPackage = Exceptions in {
+
+def ExceptionSpecChecker : Checker<"ExceptionSpec">,
+	HelpText<"Function declaration has a throw(type) specifier but the function do not throw exceptions.">,
+	DescFile<"ExceptionSpecChecker.cpp">;
+
+} // end "exceptions"
diff --git a/lib/StaticAnalyzer/Checkers/ExceptionSpecChecker.cpp b/lib/StaticAnalyzer/Checkers/ExceptionSpecChecker.cpp
new file mode 100644
index 0000000..7aa9977
--- /dev/null
+++ b/lib/StaticAnalyzer/Checkers/ExceptionSpecChecker.cpp
@@ -0,0 +1,175 @@
+//== ExceptionSpecChecker.cpp -----------------------------------*- C++ -*--==//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This defines ExceptionSpecChecker, which checks for wrong exception
+// specifiers usage.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ClangSACheckers.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+
+#include <vector>
+
+using namespace clang;
+using namespace ento;
+
+//===----------------------------------------------------------------------===//
+// FunctionExceptionsAnalyzer
+//===----------------------------------------------------------------------===//
+
+namespace {
+class FunctionExceptionsAnalyzer
+    : public RecursiveASTVisitor<FunctionExceptionsAnalyzer> {
+public:
+  FunctionExceptionsAnalyzer(FunctionDecl *FD, const ASTContext &Context);
+  void analyze(FunctionDecl *FD);
+
+  bool VisitCallExpr(CallExpr *CE);
+  bool VisitCXXThrowExpr(CXXThrowExpr *TE);
+
+  bool getCanThrow();
+  std::vector<Stmt *> getThrowCalls();
+
+private:
+  FunctionDecl *FD;
+  bool CanThrow;
+  std::vector<Stmt *> ThrowCalls;
+  const ASTContext &Context;
+};
+} // end anonymous namespace
+
+FunctionExceptionsAnalyzer::FunctionExceptionsAnalyzer(
+    FunctionDecl *FD, const ASTContext &Context)
+    : FD(FD), CanThrow(false), ThrowCalls(), Context(Context) {
+  TraverseDecl(FD);
+}
+
+bool FunctionExceptionsAnalyzer::VisitCallExpr(CallExpr *CE) {
+  const FunctionDecl *FD = CE->getDirectCallee();
+  if (!FD) {
+    return true;
+  }
+  const FunctionProtoType *FPT = FD->getType()->getAs<FunctionProtoType>();
+  if (!FPT) {
+    return true;
+  }
+
+  if (FPT->hasExceptionSpec() && !FPT->isNothrow(Context)) {
+    CanThrow = true;
+    ThrowCalls.push_back(CE);
+  }
+
+  return true;
+}
+
+bool FunctionExceptionsAnalyzer::VisitCXXThrowExpr(CXXThrowExpr *TE) {
+  CanThrow = true;
+  ThrowCalls.push_back(TE);
+  return true;
+}
+
+bool FunctionExceptionsAnalyzer::getCanThrow() { return CanThrow; }
+
+std::vector<Stmt *> FunctionExceptionsAnalyzer::getThrowCalls() {
+  return ThrowCalls;
+}
+
+//===----------------------------------------------------------------------===//
+// ExceptionSpecChecker
+//===----------------------------------------------------------------------===//
+
+namespace {
+class ExceptionSpecChecker : public Checker<check::ASTDecl<FunctionDecl>> {
+public:
+  void checkASTDecl(const FunctionDecl *FD, AnalysisManager &mgr,
+                    BugReporter &BR) const;
+
+private:
+  mutable std::unique_ptr<BuiltinBug> ThrowSpecButNotThrow,
+      NoThrowSpecButThrows, UnexcpectedThrowCall;
+
+  std::string
+      ThrowSpecButNotThrowMsg = "Function declaration has a throw(type) "
+                                "specifier but the function do not throw "
+                                "exceptions.",
+      NoThrowSpecButThrowsMsg =
+          "An exception is throw from a function having a throw() specifier.",
+      UnexpectedThrowCallMsg = "Unexcpected throw call.";
+};
+} // end anonymous namespace
+
+void ExceptionSpecChecker::checkASTDecl(const FunctionDecl *FD,
+                                        AnalysisManager &mgr,
+                                        BugReporter &BR) const {
+  const FunctionProtoType *FPT = FD->getType()->getAs<FunctionProtoType>();
+
+  if (!ThrowSpecButNotThrow) {
+    ThrowSpecButNotThrow.reset(
+        new BuiltinBug(this, ThrowSpecButNotThrowMsg.c_str()));
+  }
+  if (!NoThrowSpecButThrows) {
+    NoThrowSpecButThrows.reset(
+        new BuiltinBug(this, NoThrowSpecButThrowsMsg.c_str()));
+  }
+  if (!UnexcpectedThrowCall) {
+    UnexcpectedThrowCall.reset(
+        new BuiltinBug(this, UnexpectedThrowCallMsg.c_str()));
+  }
+
+  // There's no need to analyze function without any exception specifier.
+  if (!FPT->hasExceptionSpec()) {
+    return;
+  }
+
+  FunctionExceptionsAnalyzer FunctionAnalyzer(const_cast<FunctionDecl *>(FD),
+                                              mgr.getASTContext());
+
+  // Check whether this function may throw by declaration.
+  bool MayThrow = !FPT->isNothrow(mgr.getASTContext());
+
+  // Check whether this function can actually throw, i.e. whether it calls
+  // throw directly or it calls functions, which may throw (assume they actually
+  // throw.
+  bool CanThrow = FunctionAnalyzer.getCanThrow();
+
+  // Perform first stage of bug reporting: report the FunctionDecl.
+  if (MayThrow != CanThrow) {
+    PathDiagnosticLocation PDL(FD, mgr.getSourceManager());
+    std::unique_ptr<BugReport> R;
+    if (MayThrow && !CanThrow) {
+      R = llvm::make_unique<BugReport>(
+          *ThrowSpecButNotThrow, ThrowSpecButNotThrow->getDescription(), PDL);
+    } else if (!MayThrow && CanThrow) {
+      R = llvm::make_unique<BugReport>(
+          *NoThrowSpecButThrows, NoThrowSpecButThrows->getDescription(), PDL);
+    }
+    BR.emitReport(std::move(R));
+  }
+
+  // Report unexpected exception throwing in functions, which shouldn't throw.
+  if (!MayThrow && CanThrow) {
+    std::vector<Stmt *> ThrowCalls = FunctionAnalyzer.getThrowCalls();
+    for (auto S : ThrowCalls) {
+      PathDiagnosticLocation PDL(S->getLocStart(), mgr.getSourceManager());
+      auto R = llvm::make_unique<BugReport>(
+          *UnexcpectedThrowCall, UnexcpectedThrowCall->getDescription(), PDL);
+      BR.emitReport(std::move(R));
+    }
+  }
+}
+
+void ento::registerExceptionSpecChecker(CheckerManager &mgr) {
+  mgr.registerChecker<ExceptionSpecChecker>();
+}
diff --git a/test/Analysis/exception-spec.cpp b/test/Analysis/exception-spec.cpp
new file mode 100644
index 0000000..8bf83d8
--- /dev/null
+++ b/test/Analysis/exception-spec.cpp
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -std=c++11 -analyze -analyzer-checker=exceptions.ExceptionSpec -verify %s
+
+void call_throw() throw(int) {
+	throw(1);
+}
+
+void nospec_nothrow() {
+}
+
+void nospec_directly_throws() {
+	throw(1);
+}
+
+void nospec_indirectly_throws() {
+	call_throw();
+}
+
+void nothrowspec_nothrow() throw() {
+
+}
+
+void noexceptspec_nothrow() noexcept {
+
+}
+
+void nothrowspec_directly_throws() throw() { // expected-warning{{An exception is throw from a function having a throw() specifier}}
+	throw(1); // expected-warning{{Unexcpected throw call}}
+}
+
+void nothrowspec_indirecetly_throws() throw() { // expected-warning{{An exception is throw from a function having a throw() specifier}}
+	call_throw(); // expected-warning{{Unexcpected throw call}}
+}
+
+void noexceptspec_directly_throws() noexcept { // expected-warning{{An exception is throw from a function having a throw() specifier}}
+	throw(1); // expected-warning{{Unexcpected throw call}}
+}
+
+void noexceptspec_indirectly_throws() noexcept { // expected-warning{{An exception is throw from a function having a throw() specifier}}
+	call_throw(); // expected-warning{{Unexcpected throw call}}
+}
+
+void throwspec_nothrow() throw(int) { // expected-warning{{Function declaration has a throw(type) specifier but the function do not throw exceptions}}
+}
+
+void throwspec_directly_throws() throw(int) {
+	throw(1);
+}
+
+void throwspec_indirectly_throws() throw(int) {
+	call_throw();
+}
+
+int main() {
+	return 0;
+}