Skip to content

Commit 43aef4c

Browse files
committed
Add a new checker, cert-err58-cpp, that checks for static or thread_local objects that use a throwing constructor.
This check corresponds to the CERT secure coding rule: https://www.securecoding.cert.org/confluence/display/cplusplus/ERR58-CPP.+Constructors+of+objects+with+static+or+thread+storage+duration+must+not+throw+exceptions llvm-svn: 254415
1 parent a0a5039 commit 43aef4c

File tree

9 files changed

+170
-16
lines changed

9 files changed

+170
-16
lines changed

clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "../misc/StaticAssertCheck.h"
1818
#include "../misc/ThrowByValueCatchByReferenceCheck.h"
1919
#include "SetLongJmpCheck.h"
20+
#include "StaticObjectExceptionCheck.h"
2021
#include "ThrownExceptionTypeCheck.h"
2122
#include "VariadicFunctionDefCheck.h"
2223

@@ -41,6 +42,8 @@ class CERTModule : public ClangTidyModule {
4142
// ERR
4243
CheckFactories.registerCheck<SetLongJmpCheck>(
4344
"cert-err52-cpp");
45+
CheckFactories.registerCheck<StaticObjectExceptionCheck>(
46+
"cert-err58-cpp");
4447
CheckFactories.registerCheck<ThrownExceptionTypeCheck>(
4548
"cert-err60-cpp");
4649
CheckFactories.registerCheck<ThrowByValueCatchByReferenceCheck>(

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ set(LLVM_LINK_COMPONENTS support)
33
add_clang_library(clangTidyCERTModule
44
CERTTidyModule.cpp
55
SetLongJmpCheck.cpp
6+
StaticObjectExceptionCheck.cpp
67
ThrownExceptionTypeCheck.cpp
78
VariadicFunctionDefCheck.cpp
89

@@ -14,4 +15,5 @@ add_clang_library(clangTidyCERTModule
1415
clangTidy
1516
clangTidyGoogleModule
1617
clangTidyMiscModule
18+
clangTidyUtils
1719
)
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
//===--- StaticObjectExceptionCheck.cpp - clang-tidy-----------------------===//
2+
//
3+
// The LLVM Compiler Infrastructure
4+
//
5+
// This file is distributed under the University of Illinois Open Source
6+
// License. See LICENSE.TXT for details.
7+
//
8+
//===----------------------------------------------------------------------===//
9+
10+
#include "StaticObjectExceptionCheck.h"
11+
#include "../utils/Matchers.h"
12+
#include "clang/AST/ASTContext.h"
13+
#include "clang/ASTMatchers/ASTMatchFinder.h"
14+
15+
using namespace clang::ast_matchers;
16+
17+
namespace clang {
18+
namespace tidy {
19+
20+
void StaticObjectExceptionCheck::registerMatchers(MatchFinder *Finder) {
21+
if (!getLangOpts().CPlusPlus)
22+
return;
23+
24+
// Match any static or thread_local variable declaration that is initialized
25+
// with a constructor that can throw.
26+
Finder->addMatcher(
27+
varDecl(anyOf(hasThreadStorageDuration(), hasStaticStorageDuration()),
28+
hasInitializer(cxxConstructExpr(hasDeclaration(
29+
cxxConstructorDecl(unless(matchers::isNoThrow()))
30+
.bind("ctor")))))
31+
.bind("var"),
32+
this);
33+
}
34+
35+
void StaticObjectExceptionCheck::check(const MatchFinder::MatchResult &Result) {
36+
const auto *VD = Result.Nodes.getNodeAs<VarDecl>("var");
37+
const auto *Ctor = Result.Nodes.getNodeAs<CXXConstructorDecl>("ctor");
38+
39+
diag(VD->getLocation(),
40+
"construction of %0 with %select{static|thread_local}1 storage "
41+
"duration may throw an exception that cannot be caught")
42+
<< VD << (VD->getStorageDuration() == SD_Static ? 0 : 1);
43+
diag(Ctor->getLocation(), "possibly throwing constructor declared here",
44+
DiagnosticIDs::Note);
45+
}
46+
47+
} // namespace tidy
48+
} // namespace clang
49+
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
//===--- StaticObjectExceptionCheck.h - clang-tidy---------------*- C++ -*-===//
2+
//
3+
// The LLVM Compiler Infrastructure
4+
//
5+
// This file is distributed under the University of Illinois Open Source
6+
// License. See LICENSE.TXT for details.
7+
//
8+
//===----------------------------------------------------------------------===//
9+
10+
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_ERR58_CPP_H
11+
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_ERR58_CPP_H
12+
13+
#include "../ClangTidy.h"
14+
15+
namespace clang {
16+
namespace tidy {
17+
18+
/// Checks whether the constructor for a static or thread_local object will
19+
/// throw.
20+
///
21+
/// For the user-facing documentation see:
22+
/// http://clang.llvm.org/extra/clang-tidy/checks/cert-static-object-exception.html
23+
class StaticObjectExceptionCheck : public ClangTidyCheck {
24+
public:
25+
StaticObjectExceptionCheck(StringRef Name, ClangTidyContext *Context)
26+
: ClangTidyCheck(Name, Context) {}
27+
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
28+
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
29+
};
30+
31+
} // namespace tidy
32+
} // namespace clang
33+
34+
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_ERR58_CPP_H
35+

clang-tools-extra/clang-tidy/cert/ThrownExceptionTypeCheck.cpp

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,26 +8,13 @@
88
//===----------------------------------------------------------------------===//
99

1010
#include "ThrownExceptionTypeCheck.h"
11+
#include "../utils/Matchers.h"
1112
#include "clang/AST/ASTContext.h"
1213
#include "clang/ASTMatchers/ASTMatchFinder.h"
1314

1415
using namespace clang::ast_matchers;
1516

1617
namespace clang {
17-
namespace {
18-
AST_MATCHER(CXXConstructorDecl, isNoThrowCopyConstructor) {
19-
if (!Node.isCopyConstructor())
20-
return false;
21-
22-
const auto *FnTy = Node.getType()->getAs<FunctionProtoType>();
23-
// Assume the best for any unresolved exception specification.
24-
if (isUnresolvedExceptionSpec(FnTy->getExceptionSpecType()))
25-
return true;
26-
27-
return FnTy->isNothrow(Node.getASTContext());
28-
}
29-
} // end namespace
30-
3118
namespace tidy {
3219
void ThrownExceptionTypeCheck::registerMatchers(MatchFinder *Finder) {
3320
if (!getLangOpts().CPlusPlus)
@@ -36,7 +23,7 @@ void ThrownExceptionTypeCheck::registerMatchers(MatchFinder *Finder) {
3623
Finder->addMatcher(
3724
cxxThrowExpr(
3825
has(cxxConstructExpr(hasDeclaration(cxxConstructorDecl(
39-
isCopyConstructor(), unless(isNoThrowCopyConstructor()))))
26+
isCopyConstructor(), unless(matchers::isNoThrow()))))
4027
.bind("expr"))),
4128
this);
4229
}

clang-tools-extra/clang-tidy/utils/Matchers.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,22 @@ AST_MATCHER(QualType, isExpensiveToCopy) {
2323
return IsExpensive && *IsExpensive;
2424
}
2525

26+
AST_MATCHER(FunctionDecl, isNoThrow) {
27+
const auto *FnTy = Node.getType()->getAs<FunctionProtoType>();
28+
29+
// If the function does not have a prototype, then it is assumed to be a
30+
// throwing function (as it would if the function did not have any exception
31+
// specification).
32+
if (!FnTy)
33+
return false;
34+
35+
// Assume the best for any unresolved exception specification.
36+
if (isUnresolvedExceptionSpec(FnTy->getExceptionSpecType()))
37+
return true;
38+
39+
return FnTy->isNothrow(Node.getASTContext());
40+
}
41+
2642
} // namespace matchers
2743
} // namespace tidy
2844
} // namespace clang
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
cert-err58-cpp
2+
==============
3+
4+
This check flags all static or thread_local variable declarations where the
5+
constructor for the object may throw an exception.
6+
7+
This check corresponds to the CERT C++ Coding Standard rule
8+
`ERR58-CPP. Constructors of objects with static or thread storage duration must not throw exceptions
9+
<https://www.securecoding.cert.org/confluence/display/cplusplus/ERR58-CPP.+Constructors+of+objects+with+static+or+thread+storage+duration+must+not+throw+exceptions>`_.

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
List of clang-tidy Checks
22
=========================
33

4-
.. toctree::
4+
.. toctree::
55
cert-setlongjmp
6+
cert-static-object-exception
67
cert-thrown-exception-type
78
cert-variadic-function-def
89
cppcoreguidelines-pro-bounds-array-to-pointer-decay
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// RUN: %check_clang_tidy %s cert-err58-cpp %t
2+
3+
struct S {
4+
S() noexcept(false);
5+
};
6+
7+
struct T {
8+
T() noexcept;
9+
};
10+
11+
struct U {
12+
U() {}
13+
};
14+
15+
struct V {
16+
explicit V(const char *) {} // Can throw
17+
};
18+
19+
20+
S s;
21+
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: construction of 's' with static storage duration may throw an exception that cannot be caught [cert-err58-cpp]
22+
// CHECK-MESSAGES: 4:3: note: possibly throwing constructor declared here
23+
T t; // ok
24+
U u;
25+
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: construction of 'u' with static storage duration may throw an exception that cannot be caught
26+
// CHECK-MESSAGES: 12:3: note: possibly throwing constructor declared here
27+
V v("v");
28+
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: construction of 'v' with static storage duration may throw an exception that cannot be caught
29+
// CHECK-MESSAGES: 16:12: note: possibly throwing constructor declared here
30+
31+
void f(S s1, T t1, U u1, V v1) { // ok, ok, ok, ok
32+
S s2; // ok
33+
T t2; // ok
34+
U u2; // ok
35+
V v2("v"); // ok
36+
37+
thread_local S s3;
38+
// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: construction of 's3' with thread_local storage duration may throw an exception that cannot be caught
39+
thread_local T t3; // ok
40+
thread_local U u3;
41+
// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: construction of 'u3' with thread_local storage duration may throw an exception that cannot be caught
42+
thread_local V v3("v");
43+
// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: construction of 'v3' with thread_local storage duration may throw an exception that cannot be caught
44+
45+
static S s4;
46+
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: construction of 's4' with static storage duration may throw an exception that cannot be caught
47+
static T t4; // ok
48+
static U u4;
49+
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: construction of 'u4' with static storage duration may throw an exception that cannot be caught
50+
static V v4("v");
51+
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: construction of 'v4' with static storage duration may throw an exception that cannot be caught
52+
}

0 commit comments

Comments
 (0)