Skip to content

Commit 9037730

Browse files
[analyzer] Support allocClassWithName in OSObjectCStyleCast checker
`allocClassWithName` allocates an object with the given type. The type is actually provided as a string argument (type's name). This creates a possibility for not particularly useful warnings from the analyzer. In order to combat with those, this patch checks for casts of the `allocClassWithName` results to types mentioned directly as its argument. All other uses of this method should be reasoned about as before. rdar://72165694 Differential Revision: https://reviews.llvm.org/D99500
1 parent efa7df1 commit 9037730

File tree

3 files changed

+53
-10
lines changed

3 files changed

+53
-10
lines changed

clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,21 @@ class OSObjectCStyleCastChecker : public Checker<check::ASTCodeBody> {
3232
void checkASTCodeBody(const Decl *D, AnalysisManager &AM,
3333
BugReporter &BR) const;
3434
};
35+
} // namespace
36+
37+
namespace clang {
38+
namespace ast_matchers {
39+
AST_MATCHER_P(StringLiteral, mentionsBoundType, std::string, BindingID) {
40+
return Builder->removeBindings([this, &Node](const BoundNodesMap &Nodes) {
41+
const auto &BN = Nodes.getNode(this->BindingID);
42+
if (const auto *ND = BN.get<NamedDecl>()) {
43+
return ND->getName() != Node.getString();
44+
}
45+
return true;
46+
});
3547
}
48+
} // end namespace ast_matchers
49+
} // end namespace clang
3650

3751
static void emitDiagnostics(const BoundNodes &Nodes,
3852
BugReporter &BR,
@@ -63,22 +77,41 @@ static decltype(auto) hasTypePointingTo(DeclarationMatcher DeclM) {
6377
return hasType(pointerType(pointee(hasDeclaration(DeclM))));
6478
}
6579

66-
void OSObjectCStyleCastChecker::checkASTCodeBody(const Decl *D, AnalysisManager &AM,
80+
void OSObjectCStyleCastChecker::checkASTCodeBody(const Decl *D,
81+
AnalysisManager &AM,
6782
BugReporter &BR) const {
6883

6984
AnalysisDeclContext *ADC = AM.getAnalysisDeclContext(D);
7085

7186
auto DynamicCastM = callExpr(callee(functionDecl(hasName("safeMetaCast"))));
72-
73-
auto OSObjTypeM = hasTypePointingTo(cxxRecordDecl(isDerivedFrom("OSMetaClassBase")));
87+
// 'allocClassWithName' allocates an object with the given type.
88+
// The type is actually provided as a string argument (type's name).
89+
// This makes the following pattern possible:
90+
//
91+
// Foo *object = (Foo *)allocClassWithName("Foo");
92+
//
93+
// While OSRequiredCast can be used here, it is still not a useful warning.
94+
auto AllocClassWithNameM = callExpr(
95+
callee(functionDecl(hasName("allocClassWithName"))),
96+
// Here we want to make sure that the string argument matches the
97+
// type in the cast expression.
98+
hasArgument(0, stringLiteral(mentionsBoundType(WarnRecordDecl))));
99+
100+
auto OSObjTypeM =
101+
hasTypePointingTo(cxxRecordDecl(isDerivedFrom("OSMetaClassBase")));
74102
auto OSObjSubclassM = hasTypePointingTo(
75-
cxxRecordDecl(isDerivedFrom("OSObject")).bind(WarnRecordDecl));
76-
77-
auto CastM = cStyleCastExpr(
78-
allOf(hasSourceExpression(allOf(OSObjTypeM, unless(DynamicCastM))),
79-
OSObjSubclassM)).bind(WarnAtNode);
80-
81-
auto Matches = match(stmt(forEachDescendant(CastM)), *D->getBody(), AM.getASTContext());
103+
cxxRecordDecl(isDerivedFrom("OSObject")).bind(WarnRecordDecl));
104+
105+
auto CastM =
106+
cStyleCastExpr(
107+
allOf(OSObjSubclassM,
108+
hasSourceExpression(
109+
allOf(OSObjTypeM,
110+
unless(anyOf(DynamicCastM, AllocClassWithNameM))))))
111+
.bind(WarnAtNode);
112+
113+
auto Matches =
114+
match(stmt(forEachDescendant(CastM)), *D->getBody(), AM.getASTContext());
82115
for (BoundNodes Match : Matches)
83116
emitDiagnostics(Match, BR, ADC, this);
84117
}

clang/test/Analysis/os_object_base.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ struct OSObject : public OSMetaClassBase {
6666

6767
struct OSMetaClass : public OSMetaClassBase {
6868
virtual OSObject * alloc() const;
69+
static OSObject * allocClassWithName(const char * name);
6970
virtual ~OSMetaClass(){}
7071
};
7172

clang/test/Analysis/osobjectcstylecastchecker_test.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,3 +37,12 @@ unsigned no_warn_on_other_type_cast(A *a) {
3737
return b->getCount();
3838
}
3939

40+
unsigned no_warn_alloc_class_with_name() {
41+
OSArray *a = (OSArray *)OSMetaClass::allocClassWithName("OSArray"); // no warning
42+
return a->getCount();
43+
}
44+
45+
unsigned warn_alloc_class_with_name() {
46+
OSArray *a = (OSArray *)OSMetaClass::allocClassWithName("OSObject"); // expected-warning{{C-style cast of an OSObject is prone to type confusion attacks; use 'OSRequiredCast' if the object is definitely of type 'OSArray', or 'OSDynamicCast' followed by a null check if unsure}}
47+
return a->getCount();
48+
}

0 commit comments

Comments
 (0)