[clang-tidy] Add new check `bugprone-unhandled-exception-at-sto`

[Sockke]

Summary

Calls to stoi and stod families of functions may throw exceptions so they need to be surrounded by try/catch.

[Eugene.Zelenko]

I think will be good idea to make check more generic and allow user-defined list of unsafe functions.

clang-tools-extra/docs/ReleaseNotes.rst

135

Please keep entries in alphabetical order in section.

138

Please follow 80 characters limit.

clang-tools-extra/docs/clang-tidy/checks/bugprone/unhandled-exception-at-sto.rst

6

Ditto.

15

Ditto.

[LegalizeAdulthood]

This whole check seems weird to me. I mean, almost every use of a standard container could throw std::bad_alloc but we don't insist on a local catch for bad_alloc at every possible throwing call site.

Why would we assume that every call site of stoi or stod must have an exception handler immediately around it? It's perfectly acceptable for an application to handle this at an outer scope that can't be detected by clang-tidy.

[Sockke]

In D128697#3615404, @Eugene.Zelenko wrote:

I think will be good idea to make check more generic and allow user-defined list of unsafe functions.

I understand that user-defined is implemented through the config, but some types of information of user customization cannot be well described here. Maybe use function name and argument counts only to match unsafe functions?

[Sockke]

In D128697#3619310, @LegalizeAdulthood wrote:

This whole check seems weird to me. I mean, almost every use of a standard container could throw std::bad_alloc but we don't insist on a local catch for bad_alloc at every possible throwing call site.

Why would we assume that every call site of stoi or stod must have an exception handler immediately around it? It's perfectly acceptable for an application to handle this at an outer scope that can't be detected by clang-tidy.

Makes sense, I implemented this check here because some projects in ByteDance used stoi with missing exception handlers caused an online crash, I think this is a relatively common problem. Perhaps only report diagnostics for stoi calls in nothrow functions?

[Sockke]

Friendly ping.

[Sockke]

Friendly ping.

[LegalizeAdulthood]

In D128697#3638467, @Sockke wrote:

In D128697#3619310, @LegalizeAdulthood wrote:

This whole check seems weird to me. I mean, almost every use of a standard container could throw std::bad_alloc but we don't insist on a local catch for bad_alloc at every possible throwing call site.

Why would we assume that every call site of stoi or stod must have an exception handler immediately around it? It's perfectly acceptable for an application to handle this at an outer scope that can't be detected by clang-tidy.

Makes sense, I implemented this check here because some projects in ByteDance used stoi with missing exception handlers caused an online crash, I think this is a relatively common problem. Perhaps only report diagnostics for stoi calls in nothrow functions?

I don't have anything more to add; I mean, you answered my question with another question. I still don't see that this check is making things better. Have you run it on large code bases to see what kind of nuisance it creates?

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

 #include "SwappedArgumentsCheck.h"
 #include "TerminatingContinueCheck.h"
 #include "ThrowKeywordMissingCheck.h"
 #include "TooSmallLoopVariableCheck.h"
 #include "UncheckedOptionalAccessCheck.h"
 #include "UndefinedMemoryManipulationCheck.h"
 #include "UndelegatedConstructorCheck.h"
 #include "UnhandledExceptionAtNewCheck.h"
+#include "UnhandledExceptionAtStoCheck.h"
 #include "UnhandledSelfAssignmentCheck.h"
 #include "UnusedRaiiCheck.h"
 #include "UnusedReturnValueCheck.h"
 #include "UseAfterMoveCheck.h"
 #include "VirtualNearMissCheck.h"
 
 namespace clang {
 namespace tidy {
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<TooSmallLoopVariableCheck>(
         "bugprone-too-small-loop-variable");
     CheckFactories.registerCheck<UncheckedOptionalAccessCheck>(
         "bugprone-unchecked-optional-access");
     CheckFactories.registerCheck<UndefinedMemoryManipulationCheck>(
         "bugprone-undefined-memory-manipulation");
     CheckFactories.registerCheck<UndelegatedConstructorCheck>(
         "bugprone-undelegated-constructor");
+    CheckFactories.registerCheck<UnhandledExceptionAtStoCheck>(
+        "bugprone-unhandled-exception-at-sto");
     CheckFactories.registerCheck<UnhandledSelfAssignmentCheck>(
         "bugprone-unhandled-self-assignment");
     CheckFactories.registerCheck<UnhandledExceptionAtNewCheck>(
         "bugprone-unhandled-exception-at-new");
     CheckFactories.registerCheck<UnusedRaiiCheck>(
         "bugprone-unused-raii");
     CheckFactories.registerCheck<UnusedReturnValueCheck>(
         "bugprone-unused-return-value");

clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

add_clang_library(clangTidyBugproneModule
   SwappedArgumentsCheck.cpp
   TerminatingContinueCheck.cpp
   ThrowKeywordMissingCheck.cpp
   TooSmallLoopVariableCheck.cpp
   UncheckedOptionalAccessCheck.cpp
   UndefinedMemoryManipulationCheck.cpp
   UndelegatedConstructorCheck.cpp
   UnhandledExceptionAtNewCheck.cpp
+  UnhandledExceptionAtStoCheck.cpp
   UnhandledSelfAssignmentCheck.cpp
   UnusedRaiiCheck.cpp
   UnusedReturnValueCheck.cpp
   UseAfterMoveCheck.cpp
   VirtualNearMissCheck.cpp
 
   LINK_LIBS
   clangTidy

clang-tools-extra/clang-tidy/bugprone/UnhandledExceptionAtStoCheck.h

+//===--- UnhandledExceptionAtStoCheck.h - clang-tidy ------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNHANDLEDEXCEPTIONATSTOCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNHANDLEDEXCEPTIONATSTOCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Finds calls to 'stoi' and 'stof' families of functions that may throw
+/// unhandled exception at invalid argument and out of range.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/unhandled-exception-at-sto.html
+class UnhandledExceptionAtStoCheck : public ClangTidyCheck {
+public:
+  UnhandledExceptionAtStoCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus && LangOpts.CXXExceptions;
+  }
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNHANDLEDEXCEPTIONATSTOCHECK_H

clang-tools-extra/clang-tidy/bugprone/UnhandledExceptionAtStoCheck.cpp

+//===--- UnhandledExceptionAtStoCheck.cpp - clang-tidy --------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "UnhandledExceptionAtStoCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+AST_MATCHER_P(CXXTryStmt, hasHandlerFor,
+              ast_matchers::internal::Matcher<QualType>, InnerMatcher) {
+  for (unsigned NH = Node.getNumHandlers(), I = 0; I < NH; ++I) {
+    const CXXCatchStmt *CatchS = Node.getHandler(I);
+    // Check for generic catch handler (match anything).
+    if (CatchS->getCaughtType().isNull())
+      return true;
+    ast_matchers::internal::BoundNodesTreeBuilder Result(*Builder);
+    if (InnerMatcher.matches(CatchS->getCaughtType(), Finder, &Result)) {
+      *Builder = std::move(Result);
+      return true;
+    }
+  }
+  return false;
+}
+
+void UnhandledExceptionAtStoCheck::registerMatchers(MatchFinder *Finder) {
+  auto ExceptionType =
+      recordType(hasDeclaration(cxxRecordDecl(hasName("::std::exception"))));
+  auto LogicErrorType =
+      recordType(hasDeclaration(cxxRecordDecl(hasName("::std::logic_error"))));
+  auto InvalidArgumentType = recordType(
+      hasDeclaration(cxxRecordDecl(hasName("::std::invalid_argument"))));
+  auto OutOfRangeType =
+      recordType(hasDeclaration(cxxRecordDecl(hasName("::std::out_of_range"))));
+
+  auto ExceptionReferenceType = referenceType(pointee(ExceptionType));
+  auto LogicErrorReferenceType = referenceType(pointee(LogicErrorType));
+  auto InvalidArgumentReferenceType =
+      referenceType(pointee(InvalidArgumentType));
+  auto OutOfRangeReferenceType = referenceType(pointee(OutOfRangeType));
+
+  auto CatchStoExceptionType = qualType(
+      hasCanonicalType(anyOf(ExceptionType, ExceptionReferenceType,
+                             LogicErrorType, LogicErrorReferenceType)));
+  auto CatchInvalidArgumentType = qualType(hasCanonicalType(
+      anyOf(InvalidArgumentType, InvalidArgumentReferenceType)));
+  auto CatchOutOfRangeType = qualType(
+      hasCanonicalType(anyOf(OutOfRangeType, OutOfRangeReferenceType)));
+
+  auto StoExceptionCatchingTryBlock =
+      cxxTryStmt(hasHandlerFor(CatchStoExceptionType)).bind("all");
+  auto StoInvalidArgumentCatchingTryBlock =
+      cxxTryStmt(hasHandlerFor(CatchInvalidArgumentType)).bind("i");
+  auto StoOutOfRangeCatchingTryBlock =
+      cxxTryStmt(hasHandlerFor(CatchOutOfRangeType)).bind("o");
+
+  const auto StoCallee = callee(functionDecl(anyOf(
+      hasName("std::stoi"), hasName("std::stol"), hasName("std::stoll"),
+      hasName("std::stof"), hasName("std::stod"), hasName("std::stold"))));
+
+  Finder->addMatcher(
+      callExpr(StoCallee,
+               allOf(unless(hasAncestor(StoExceptionCatchingTryBlock)),
+                     unless(hasAncestor(StoInvalidArgumentCatchingTryBlock)),
+                     unless(hasAncestor(StoOutOfRangeCatchingTryBlock))))
+          .bind("catched-no-exception-call"),
+      this);
+  Finder->addMatcher(
+      callExpr(StoCallee,
+               allOf(unless(hasAncestor(StoExceptionCatchingTryBlock)),
+                     hasAncestor(StoInvalidArgumentCatchingTryBlock),
+                     unless(hasAncestor(StoOutOfRangeCatchingTryBlock))))
+          .bind("catched-invalid-argument-call"),
+      this);
+  Finder->addMatcher(
+      callExpr(StoCallee,
+               allOf(unless(hasAncestor(StoExceptionCatchingTryBlock)),
+                     hasAncestor(StoOutOfRangeCatchingTryBlock),
+                     unless(hasAncestor(StoInvalidArgumentCatchingTryBlock))))
+          .bind("catched-out-of-range-call"),
+      this);
+}
+
+void UnhandledExceptionAtStoCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *CatchedNoExceptionCall =
+      Result.Nodes.getNodeAs<CallExpr>("catched-no-exception-call");
+  const auto *CatchedInvalidArgumentCall =
+      Result.Nodes.getNodeAs<CallExpr>("catched-invalid-argument-call");
+  const auto *CatchedOutOfRangeCall =
+      Result.Nodes.getNodeAs<CallExpr>("catched-out-of-range-call");
+
+  const CallExpr *CatedStoCall = nullptr;
+  const FunctionDecl *StoCallDecl = nullptr;
+  std::string Message = "missing exception handler for ";
+
+  if (CatchedNoExceptionCall) {
+    CatedStoCall = CatchedNoExceptionCall;
+    StoCallDecl = CatchedNoExceptionCall->getDirectCallee();
+    if (!StoCallDecl)
+      return;
+    Message += "invalid argument and out of range";
+  } else if (CatchedInvalidArgumentCall) {
+    CatedStoCall = CatchedInvalidArgumentCall;
+    StoCallDecl = CatchedInvalidArgumentCall->getDirectCallee();
+    if (!StoCallDecl)
+      return;
+    Message += "out of range";
+  } else {
+    CatedStoCall = CatchedOutOfRangeCall;
+    StoCallDecl = CatchedOutOfRangeCall->getDirectCallee();
+    if (!StoCallDecl)
+      return;
+    Message += "invalid argument";
+  }
+  diag(CatedStoCall->getBeginLoc(), Message + " at 'std::%0'")
+      << StoCallDecl->getName();
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang

clang-tools-extra/docs/ReleaseNotes.rst

- New :doc:`bugprone-unchecked-optional-access
 
   Warns when the code is unwrapping a `std::optional<T>`, `absl::optional<T>`,
   or `base::Optional<T>` object without assuring that it contains a value.
 
 - New :doc:`misc-confusable-identifiers <clang-tidy/checks/misc/confusable-identifiers>` check.
 
   Detects confusable Unicode identifiers.
 
+- New :doc:`bugprone-unhandled-exception-at-sto
+  <clang-tidy/checks/bugprone/unhandled-exception-at-sto>` check.
+
+  Finds calls to ``stoi`` and ``stod`` families of functions with missing exception handler for ``std::invalid_argument`` and ``std::out_of_range``.
+
 - New :doc:`modernize-macro-to-enum
   <clang-tidy/checks/modernize/macro-to-enum>` check.
 
   Replaces groups of adjacent macros with an unscoped anonymous enum.
 
 - New :doc:`portability-std-allocator-const <clang-tidy/checks/portability/std-allocator-const>` check.
 
   Report use of ``std::vector<const T>`` (and similar containers of const

clang-tools-extra/docs/clang-tidy/checks/bugprone/unhandled-exception-at-sto.rst

+.. title:: clang-tidy - bugprone-unhandled-exception-at-sto
+
+bugprone-unhandled-exception-at-sto
+===================================
+
+Finds calls to ``stoi`` and ``stof`` families of functions with missing exception handler for ``std::invalid_argument`` and ``std::out_of_range``.
+
+.. code-block:: c++
+
+  int test(const std::string &s) {
+    int v = std::stoi(s); 
+    return v;
+  }
+
+Calls to ``stoi`` and ``stof`` families of functions can throw exceptions of type ``std::invalid_argument`` and ``std::out_of_range`` that should be handled by the code.

clang-tools-extra/docs/clang-tidy/checks/list.rst

.. csv-table::
    `bugprone-swapped-arguments <bugprone/swapped-arguments.html>`_, "Yes"
    `bugprone-terminating-continue <bugprone/terminating-continue.html>`_, "Yes"
    `bugprone-throw-keyword-missing <bugprone/throw-keyword-missing.html>`_,
    `bugprone-too-small-loop-variable <bugprone/too-small-loop-variable.html>`_,
    `bugprone-unchecked-optional-access <bugprone/unchecked-optional-access.html>`_,
    `bugprone-undefined-memory-manipulation <bugprone/undefined-memory-manipulation.html>`_,
    `bugprone-undelegated-constructor <bugprone/undelegated-constructor.html>`_,
    `bugprone-unhandled-exception-at-new <bugprone/unhandled-exception-at-new.html>`_,
+   `bugprone-unhandled-exception-at-sto <bugprone/unhandled-exception-at-sto.html>`_, "Yes"
    `bugprone-unhandled-self-assignment <bugprone/unhandled-self-assignment.html>`_,
    `bugprone-unused-raii <bugprone/unused-raii.html>`_, "Yes"
    `bugprone-unused-return-value <bugprone/unused-return-value.html>`_,
    `bugprone-use-after-move <bugprone/use-after-move.html>`_,
    `bugprone-virtual-near-miss <bugprone/virtual-near-miss.html>`_, "Yes"
    `cert-dcl21-cpp <cert/dcl21-cpp.html>`_, "Yes"
    `cert-dcl50-cpp <cert/dcl50-cpp.html>`_,
    `cert-dcl58-cpp <cert/dcl58-cpp.html>`_,

clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-exception-at-sto.cpp

+// RUN: %check_clang_tidy %s bugprone-unhandled-exception-at-sto %t
+
+namespace std {
+
+template <typename Char>
+struct basic_string {
+  basic_string(const Char *);
+  ~basic_string();
+};
+typedef basic_string<char> string;
+
+typedef unsigned int size_t;
+
+int stoi(const std::string &str, size_t *pos = nullptr, int base = 10) { return 0; }
+long stol(const std::string &str, size_t *pos = nullptr, int base = 10) { return 0; }
+long long stoll(const std::string &str, size_t *pos = nullptr, int base = 10) { return 0; }
+
+float stof(const std::string &str, size_t *pos = nullptr) { return 0; }
+double stod(const std::string &str, size_t *pos = nullptr) { return 0; }
+long double stold(const std::string &str, size_t *pos = nullptr) { return 0; }
+
+struct exception {};
+struct logic_error : public exception {};
+struct invalid_argument : public logic_error {};
+struct out_of_range : public logic_error {};
+
+} // namespace std
+
+void test() {
+  std::string s("123");
+
+  int a = std::stoi(s);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: missing exception handler for invalid argument and out of range at 'std::stoi' [bugprone-unhandled-exception-at-sto]
+  long b = std::stol(s);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: missing exception handler for invalid argument and out of range at 'std::stol' [bugprone-unhandled-exception-at-sto]
+  long long c = std::stoll(s);
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: missing exception handler for invalid argument and out of range at 'std::stoll' [bugprone-unhandled-exception-at-sto]
+  float d = std::stof(s);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: missing exception handler for invalid argument and out of range at 'std::stof' [bugprone-unhandled-exception-at-sto]
+  double e = std::stod(s);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: missing exception handler for invalid argument and out of range at 'std::stod' [bugprone-unhandled-exception-at-sto]
+  long double f = std::stold(s);
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: missing exception handler for invalid argument and out of range at 'std::stold' [bugprone-unhandled-exception-at-sto]
+
+  try {
+    int v = std::stoi(s);
+    // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: missing exception handler for invalid argument at 'std::stoi' [bugprone-unhandled-exception-at-sto]
+  } catch (std::out_of_range) {
+  }
+  try {
+    int v = std::stoi(s);
+    // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: missing exception handler for invalid argument at 'std::stoi' [bugprone-unhandled-exception-at-sto]
+  } catch (std::out_of_range &) {
+  }
+  try {
+    int v = std::stoi(s);
+    // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: missing exception handler for invalid argument at 'std::stoi' [bugprone-unhandled-exception-at-sto]
+  } catch (const std::out_of_range &) {
+  }
+  try {
+    int v = std::stoi(s);
+    // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: missing exception handler for invalid argument and out of range at 'std::stoi' [bugprone-unhandled-exception-at-sto]
+  } catch (std::out_of_range *) {
+  }
+
+  try {
+    int v = std::stoi(s);
+    // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: missing exception handler for out of range at 'std::stoi' [bugprone-unhandled-exception-at-sto]
+  } catch (std::invalid_argument) {
+  }
+  try {
+    int v = std::stoi(s);
+    // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: missing exception handler for out of range at 'std::stoi' [bugprone-unhandled-exception-at-sto]
+  } catch (std::invalid_argument &) {
+  }
+  try {
+    int v = std::stoi(s);
+    // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: missing exception handler for out of range at 'std::stoi' [bugprone-unhandled-exception-at-sto]
+  } catch (const std::invalid_argument &) {
+  }
+  try {
+    int v = std::stoi(s);
+    // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: missing exception handler for invalid argument and out of range at 'std::stoi' [bugprone-unhandled-exception-at-sto]
+  } catch (std::invalid_argument *) {
+  }
+
+  try {
+    int v = std::stoi(s);
+  } catch (std::exception) {
+  }
+  try {
+    int v = std::stoi(s);
+  } catch (std::exception &) {
+  }
+  try {
+    int v = std::stoi(s);
+  } catch (const std::exception &) {
+  }
+  try {
+    int v = std::stoi(s);
+    // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: missing exception handler for invalid argument and out of range at 'std::stoi' [bugprone-unhandled-exception-at-sto]
+  } catch (std::exception *) {
+  }
+
+  try {
+    int v = std::stoi(s);
+  } catch (std::logic_error) {
+  }
+  try {
+    int v = std::stoi(s);
+  } catch (std::logic_error &) {
+  }
+  try {
+    int v = std::stoi(s);
+  } catch (const std::logic_error &) {
+  }
+  try {
+    int v = std::stoi(s);
+    // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: missing exception handler for invalid argument and out of range at 'std::stoi' [bugprone-unhandled-exception-at-sto]
+  } catch (std::logic_error *) {
+  }
+
+  try {
+    int v = std::stoi(s);
+  } catch (const std::invalid_argument &) {
+  } catch (const std::out_of_range &) {
+  }
+}