Skip to content

Commit 7599261

Browse files
t-rasmudrniwa
t-rasmud
authored and
rniwa
committed
[Webkit Checkers] Introduce a Webkit checker for memory unsafe casts (llvm#114606)
This PR introduces a new checker `[alpha.webkit.MemoryUnsafeCastChecker]` that warns all downcasts from a base type to a derived type. rdar://137766829
1 parent cb4a48d commit 7599261

File tree

6 files changed

+548
-0
lines changed

6 files changed

+548
-0
lines changed

‎clang/docs/analyzer/checkers.rst

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3435,6 +3435,31 @@ alpha.WebKit
34353435
34363436
.. _alpha-webkit-NoUncheckedPtrMemberChecker:
34373437
3438+
alpha.webkit.MemoryUnsafeCastChecker
3439+
""""""""""""""""""""""""""""""""""""""
3440+
Check for all casts from a base type to its derived type as these might be memory-unsafe.
3441+
3442+
Example:
3443+
3444+
.. code-block:: cpp
3445+
3446+
class Base { };
3447+
class Derived : public Base { };
3448+
3449+
void f(Base* base) {
3450+
Derived* derived = static_cast<Derived*>(base); // ERROR
3451+
}
3452+
3453+
For all cast operations (C-style casts, static_cast, reinterpret_cast, dynamic_cast), if the source type a `Base*` and the destination type is `Derived*`, where `Derived` inherits from `Base`, the static analyzer should signal an error.
3454+
3455+
This applies to:
3456+
3457+
- C structs, C++ structs and classes, and Objective-C classes and protocols.
3458+
- Pointers and references.
3459+
- Inside template instantiations and macro expansions that are visible to the compiler.
3460+
3461+
For types like this, instead of using built in casts, the programmer will use helper functions that internally perform the appropriate type check and disable static analysis.
3462+
34383463
alpha.webkit.NoUncheckedPtrMemberChecker
34393464
""""""""""""""""""""""""""""""""""""""""
34403465
Raw pointers and references to an object which supports CheckedPtr or CheckedRef can't be used as class members. Only CheckedPtr, CheckedRef, RefPtr, or Ref are allowed.

‎clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1787,6 +1787,10 @@ def UncountedLambdaCapturesChecker : Checker<"UncountedLambdaCapturesChecker">,
17871787

17881788
let ParentPackage = WebKitAlpha in {
17891789

1790+
def MemoryUnsafeCastChecker : Checker<"MemoryUnsafeCastChecker">,
1791+
HelpText<"Check for memory unsafe casts from base type to derived type.">,
1792+
Documentation<HasDocumentation>;
1793+
17901794
def NoUncheckedPtrMemberChecker : Checker<"NoUncheckedPtrMemberChecker">,
17911795
HelpText<"Check for no unchecked member variables.">,
17921796
Documentation<HasDocumentation>;

‎clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ add_clang_library(clangStaticAnalyzerCheckers
135135
VirtualCallChecker.cpp
136136
WebKit/RawPtrRefMemberChecker.cpp
137137
WebKit/ASTUtils.cpp
138+
WebKit/MemoryUnsafeCastChecker.cpp
138139
WebKit/PtrTypesSemantics.cpp
139140
WebKit/RefCntblBaseVirtualDtorChecker.cpp
140141
WebKit/RawPtrRefCallArgsChecker.cpp

‎clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
//=======- MemoryUnsafeCastChecker.cpp -------------------------*- C++ -*-==//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
//
9+
// This file defines MemoryUnsafeCast checker, which checks for casts from a
10+
// base type to a derived type.
11+
//===----------------------------------------------------------------------===//
12+
13+
#include "clang/ASTMatchers/ASTMatchFinder.h"
14+
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
15+
#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
16+
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
17+
#include "clang/StaticAnalyzer/Core/Checker.h"
18+
#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
19+
20+
using namespace clang;
21+
using namespace ento;
22+
using namespace ast_matchers;
23+
24+
namespace {
25+
static constexpr const char *const BaseNode = "BaseNode";
26+
static constexpr const char *const DerivedNode = "DerivedNode";
27+
static constexpr const char *const FromCastNode = "FromCast";
28+
static constexpr const char *const ToCastNode = "ToCast";
29+
static constexpr const char *const WarnRecordDecl = "WarnRecordDecl";
30+
31+
class MemoryUnsafeCastChecker : public Checker<check::ASTCodeBody> {
32+
BugType BT{this, "Unsafe cast", "WebKit coding guidelines"};
33+
34+
public:
35+
void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
36+
BugReporter &BR) const;
37+
};
38+
} // end namespace
39+
40+
static void emitDiagnostics(const BoundNodes &Nodes, BugReporter &BR,
41+
AnalysisDeclContext *ADC,
42+
const MemoryUnsafeCastChecker *Checker,
43+
const BugType &BT) {
44+
const auto *CE = Nodes.getNodeAs<CastExpr>(WarnRecordDecl);
45+
const NamedDecl *Base = Nodes.getNodeAs<NamedDecl>(BaseNode);
46+
const NamedDecl *Derived = Nodes.getNodeAs<NamedDecl>(DerivedNode);
47+
assert(CE && Base && Derived);
48+
49+
std::string Diagnostics;
50+
llvm::raw_string_ostream OS(Diagnostics);
51+
OS << "Unsafe cast from base type '" << Base->getNameAsString()
52+
<< "' to derived type '" << Derived->getNameAsString() << "'";
53+
PathDiagnosticLocation BSLoc(CE->getSourceRange().getBegin(),
54+
BR.getSourceManager());
55+
auto Report = std::make_unique<BasicBugReport>(BT, OS.str(), BSLoc);
56+
Report->addRange(CE->getSourceRange());
57+
BR.emitReport(std::move(Report));
58+
}
59+
60+
static void emitDiagnosticsUnrelated(const BoundNodes &Nodes, BugReporter &BR,
61+
AnalysisDeclContext *ADC,
62+
const MemoryUnsafeCastChecker *Checker,
63+
const BugType &BT) {
64+
const auto *CE = Nodes.getNodeAs<CastExpr>(WarnRecordDecl);
65+
const NamedDecl *FromCast = Nodes.getNodeAs<NamedDecl>(FromCastNode);
66+
const NamedDecl *ToCast = Nodes.getNodeAs<NamedDecl>(ToCastNode);
67+
assert(CE && FromCast && ToCast);
68+
69+
std::string Diagnostics;
70+
llvm::raw_string_ostream OS(Diagnostics);
71+
OS << "Unsafe cast from type '" << FromCast->getNameAsString()
72+
<< "' to an unrelated type '" << ToCast->getNameAsString() << "'";
73+
PathDiagnosticLocation BSLoc(CE->getSourceRange().getBegin(),
74+
BR.getSourceManager());
75+
auto Report = std::make_unique<BasicBugReport>(BT, OS.str(), BSLoc);
76+
Report->addRange(CE->getSourceRange());
77+
BR.emitReport(std::move(Report));
78+
}
79+
80+
namespace clang {
81+
namespace ast_matchers {
82+
AST_MATCHER_P(StringLiteral, mentionsBoundType, std::string, BindingID) {
83+
return Builder->removeBindings([this, &Node](const BoundNodesMap &Nodes) {
84+
const auto &BN = Nodes.getNode(this->BindingID);
85+
if (const auto *ND = BN.get<NamedDecl>()) {
86+
return ND->getName() != Node.getString();
87+
}
88+
return true;
89+
});
90+
}
91+
} // end namespace ast_matchers
92+
} // end namespace clang
93+
94+
static decltype(auto) hasTypePointingTo(DeclarationMatcher DeclM) {
95+
return hasType(pointerType(pointee(hasDeclaration(DeclM))));
96+
}
97+
98+
void MemoryUnsafeCastChecker::checkASTCodeBody(const Decl *D,
99+
AnalysisManager &AM,
100+
BugReporter &BR) const {
101+
102+
AnalysisDeclContext *ADC = AM.getAnalysisDeclContext(D);
103+
104+
// Match downcasts from base type to derived type and warn
105+
auto MatchExprPtr = allOf(
106+
hasSourceExpression(hasTypePointingTo(cxxRecordDecl().bind(BaseNode))),
107+
hasTypePointingTo(cxxRecordDecl(isDerivedFrom(equalsBoundNode(BaseNode)))
108+
.bind(DerivedNode)),
109+
unless(anyOf(hasSourceExpression(cxxThisExpr()),
110+
hasTypePointingTo(templateTypeParmDecl()))));
111+
auto MatchExprPtrObjC = allOf(
112+
hasSourceExpression(ignoringImpCasts(hasType(objcObjectPointerType(
113+
pointee(hasDeclaration(objcInterfaceDecl().bind(BaseNode))))))),
114+
ignoringImpCasts(hasType(objcObjectPointerType(pointee(hasDeclaration(
115+
objcInterfaceDecl(isDerivedFrom(equalsBoundNode(BaseNode)))
116+
.bind(DerivedNode)))))));
117+
auto MatchExprRefTypeDef =
118+
allOf(hasSourceExpression(hasType(hasUnqualifiedDesugaredType(recordType(
119+
hasDeclaration(decl(cxxRecordDecl().bind(BaseNode))))))),
120+
hasType(hasUnqualifiedDesugaredType(recordType(hasDeclaration(
121+
decl(cxxRecordDecl(isDerivedFrom(equalsBoundNode(BaseNode)))
122+
.bind(DerivedNode)))))),
123+
unless(anyOf(hasSourceExpression(hasDescendant(cxxThisExpr())),
124+
hasType(templateTypeParmDecl()))));
125+
126+
auto ExplicitCast = explicitCastExpr(anyOf(MatchExprPtr, MatchExprRefTypeDef,
127+
MatchExprPtrObjC))
128+
.bind(WarnRecordDecl);
129+
auto Cast = stmt(ExplicitCast);
130+
131+
auto Matches =
132+
match(stmt(forEachDescendant(Cast)), *D->getBody(), AM.getASTContext());
133+
for (BoundNodes Match : Matches)
134+
emitDiagnostics(Match, BR, ADC, this, BT);
135+
136+
// Match casts between unrelated types and warn
137+
auto MatchExprPtrUnrelatedTypes = allOf(
138+
hasSourceExpression(
139+
hasTypePointingTo(cxxRecordDecl().bind(FromCastNode))),
140+
hasTypePointingTo(cxxRecordDecl().bind(ToCastNode)),
141+
unless(anyOf(hasTypePointingTo(cxxRecordDecl(
142+
isSameOrDerivedFrom(equalsBoundNode(FromCastNode)))),
143+
hasSourceExpression(hasTypePointingTo(cxxRecordDecl(
144+
isSameOrDerivedFrom(equalsBoundNode(ToCastNode))))))));
145+
auto MatchExprPtrObjCUnrelatedTypes = allOf(
146+
hasSourceExpression(ignoringImpCasts(hasType(objcObjectPointerType(
147+
pointee(hasDeclaration(objcInterfaceDecl().bind(FromCastNode))))))),
148+
ignoringImpCasts(hasType(objcObjectPointerType(
149+
pointee(hasDeclaration(objcInterfaceDecl().bind(ToCastNode)))))),
150+
unless(anyOf(
151+
ignoringImpCasts(hasType(
152+
objcObjectPointerType(pointee(hasDeclaration(objcInterfaceDecl(
153+
isSameOrDerivedFrom(equalsBoundNode(FromCastNode)))))))),
154+
hasSourceExpression(ignoringImpCasts(hasType(
155+
objcObjectPointerType(pointee(hasDeclaration(objcInterfaceDecl(
156+
isSameOrDerivedFrom(equalsBoundNode(ToCastNode))))))))))));
157+
auto MatchExprRefTypeDefUnrelated = allOf(
158+
hasSourceExpression(hasType(hasUnqualifiedDesugaredType(recordType(
159+
hasDeclaration(decl(cxxRecordDecl().bind(FromCastNode))))))),
160+
hasType(hasUnqualifiedDesugaredType(
161+
recordType(hasDeclaration(decl(cxxRecordDecl().bind(ToCastNode)))))),
162+
unless(anyOf(
163+
hasType(hasUnqualifiedDesugaredType(
164+
recordType(hasDeclaration(decl(cxxRecordDecl(
165+
isSameOrDerivedFrom(equalsBoundNode(FromCastNode)))))))),
166+
hasSourceExpression(hasType(hasUnqualifiedDesugaredType(
167+
recordType(hasDeclaration(decl(cxxRecordDecl(
168+
isSameOrDerivedFrom(equalsBoundNode(ToCastNode))))))))))));
169+
170+
auto ExplicitCastUnrelated =
171+
explicitCastExpr(anyOf(MatchExprPtrUnrelatedTypes,
172+
MatchExprPtrObjCUnrelatedTypes,
173+
MatchExprRefTypeDefUnrelated))
174+
.bind(WarnRecordDecl);
175+
auto CastUnrelated = stmt(ExplicitCastUnrelated);
176+
auto MatchesUnrelatedTypes = match(stmt(forEachDescendant(CastUnrelated)),
177+
*D->getBody(), AM.getASTContext());
178+
for (BoundNodes Match : MatchesUnrelatedTypes)
179+
emitDiagnosticsUnrelated(Match, BR, ADC, this, BT);
180+
}
181+
182+
void ento::registerMemoryUnsafeCastChecker(CheckerManager &Mgr) {
183+
Mgr.registerChecker<MemoryUnsafeCastChecker>();
184+
}
185+
186+
bool ento::shouldRegisterMemoryUnsafeCastChecker(const CheckerManager &mgr) {
187+
return true;
188+
}

0 commit comments

Comments
 (0)