Skip to content

Commit 8e81c52

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 c643f77 commit 8e81c52

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
@@ -3022,6 +3022,31 @@ alpha.WebKit
30223022
30233023
.. _alpha-webkit-NoUncheckedPtrMemberChecker:
30243024
3025+
alpha.webkit.MemoryUnsafeCastChecker
3026+
""""""""""""""""""""""""""""""""""""""
3027+
Check for all casts from a base type to its derived type as these might be memory-unsafe.
3028+
3029+
Example:
3030+
3031+
.. code-block:: cpp
3032+
3033+
class Base { };
3034+
class Derived : public Base { };
3035+
3036+
void f(Base* base) {
3037+
Derived* derived = static_cast<Derived*>(base); // ERROR
3038+
}
3039+
3040+
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.
3041+
3042+
This applies to:
3043+
3044+
- C structs, C++ structs and classes, and Objective-C classes and protocols.
3045+
- Pointers and references.
3046+
- Inside template instantiations and macro expansions that are visible to the compiler.
3047+
3048+
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.
3049+
30253050
alpha.webkit.NoUncheckedPtrMemberChecker
30263051
""""""""""""""""""""""""""""""""""""""""
30273052
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
@@ -1719,6 +1719,10 @@ def UncountedLambdaCapturesChecker : Checker<"UncountedLambdaCapturesChecker">,
17191719

17201720
let ParentPackage = WebKitAlpha in {
17211721

1722+
def MemoryUnsafeCastChecker : Checker<"MemoryUnsafeCastChecker">,
1723+
HelpText<"Check for memory unsafe casts from base type to derived type.">,
1724+
Documentation<HasDocumentation>;
1725+
17221726
def NoUncheckedPtrMemberChecker : Checker<"NoUncheckedPtrMemberChecker">,
17231727
HelpText<"Check for no unchecked member variables.">,
17241728
Documentation<HasDocumentation>;

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ add_clang_library(clangStaticAnalyzerCheckers
132132
VirtualCallChecker.cpp
133133
WebKit/RawPtrRefMemberChecker.cpp
134134
WebKit/ASTUtils.cpp
135+
WebKit/MemoryUnsafeCastChecker.cpp
135136
WebKit/PtrTypesSemantics.cpp
136137
WebKit/RefCntblBaseVirtualDtorChecker.cpp
137138
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)