Author: Valeriy Savchenko Date: 2021-03-30T15:58:06+03:00 New Revision: 90377308de6cac8239bc1a1dcd32b57b9ec91444
URL: https://github.com/llvm/llvm-project/commit/90377308de6cac8239bc1a1dcd32b57b9ec91444 DIFF: https://github.com/llvm/llvm-project/commit/90377308de6cac8239bc1a1dcd32b57b9ec91444.diff LOG: [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 Added: Modified: clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp clang/test/Analysis/os_object_base.h clang/test/Analysis/osobjectcstylecastchecker_test.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp b/clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp index 270b66dab020..0a8379d9ab99 100644 --- a/clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp @@ -32,7 +32,21 @@ class OSObjectCStyleCastChecker : public Checker<check::ASTCodeBody> { void checkASTCodeBody(const Decl *D, AnalysisManager &AM, BugReporter &BR) const; }; +} // namespace + +namespace clang { +namespace ast_matchers { +AST_MATCHER_P(StringLiteral, mentionsBoundType, std::string, BindingID) { + return Builder->removeBindings([this, &Node](const BoundNodesMap &Nodes) { + const auto &BN = Nodes.getNode(this->BindingID); + if (const auto *ND = BN.get<NamedDecl>()) { + return ND->getName() != Node.getString(); + } + return true; + }); } +} // end namespace ast_matchers +} // end namespace clang static void emitDiagnostics(const BoundNodes &Nodes, BugReporter &BR, @@ -63,22 +77,41 @@ static decltype(auto) hasTypePointingTo(DeclarationMatcher DeclM) { return hasType(pointerType(pointee(hasDeclaration(DeclM)))); } -void OSObjectCStyleCastChecker::checkASTCodeBody(const Decl *D, AnalysisManager &AM, +void OSObjectCStyleCastChecker::checkASTCodeBody(const Decl *D, + AnalysisManager &AM, BugReporter &BR) const { AnalysisDeclContext *ADC = AM.getAnalysisDeclContext(D); auto DynamicCastM = callExpr(callee(functionDecl(hasName("safeMetaCast")))); - - auto OSObjTypeM = hasTypePointingTo(cxxRecordDecl(isDerivedFrom("OSMetaClassBase"))); + // 'allocClassWithName' allocates an object with the given type. + // The type is actually provided as a string argument (type's name). + // This makes the following pattern possible: + // + // Foo *object = (Foo *)allocClassWithName("Foo"); + // + // While OSRequiredCast can be used here, it is still not a useful warning. + auto AllocClassWithNameM = callExpr( + callee(functionDecl(hasName("allocClassWithName"))), + // Here we want to make sure that the string argument matches the + // type in the cast expression. + hasArgument(0, stringLiteral(mentionsBoundType(WarnRecordDecl)))); + + auto OSObjTypeM = + hasTypePointingTo(cxxRecordDecl(isDerivedFrom("OSMetaClassBase"))); auto OSObjSubclassM = hasTypePointingTo( - cxxRecordDecl(isDerivedFrom("OSObject")).bind(WarnRecordDecl)); - - auto CastM = cStyleCastExpr( - allOf(hasSourceExpression(allOf(OSObjTypeM, unless(DynamicCastM))), - OSObjSubclassM)).bind(WarnAtNode); - - auto Matches = match(stmt(forEachDescendant(CastM)), *D->getBody(), AM.getASTContext()); + cxxRecordDecl(isDerivedFrom("OSObject")).bind(WarnRecordDecl)); + + auto CastM = + cStyleCastExpr( + allOf(OSObjSubclassM, + hasSourceExpression( + allOf(OSObjTypeM, + unless(anyOf(DynamicCastM, AllocClassWithNameM)))))) + .bind(WarnAtNode); + + auto Matches = + match(stmt(forEachDescendant(CastM)), *D->getBody(), AM.getASTContext()); for (BoundNodes Match : Matches) emitDiagnostics(Match, BR, ADC, this); } diff --git a/clang/test/Analysis/os_object_base.h b/clang/test/Analysis/os_object_base.h index 4698185f2b3c..c3d5d6271d48 100644 --- a/clang/test/Analysis/os_object_base.h +++ b/clang/test/Analysis/os_object_base.h @@ -66,6 +66,7 @@ struct OSObject : public OSMetaClassBase { struct OSMetaClass : public OSMetaClassBase { virtual OSObject * alloc() const; + static OSObject * allocClassWithName(const char * name); virtual ~OSMetaClass(){} }; diff --git a/clang/test/Analysis/osobjectcstylecastchecker_test.cpp b/clang/test/Analysis/osobjectcstylecastchecker_test.cpp index a5ebb2823a92..aa43a18e6346 100644 --- a/clang/test/Analysis/osobjectcstylecastchecker_test.cpp +++ b/clang/test/Analysis/osobjectcstylecastchecker_test.cpp @@ -37,3 +37,12 @@ unsigned no_warn_on_other_type_cast(A *a) { return b->getCount(); } +unsigned no_warn_alloc_class_with_name() { + OSArray *a = (OSArray *)OSMetaClass::allocClassWithName("OSArray"); // no warning + return a->getCount(); +} + +unsigned warn_alloc_class_with_name() { + 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}} + return a->getCount(); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
