Author: Rashmi Mudduluru Date: 2026-07-31T08:36:16-07:00 New Revision: cb1642b6f9e458bf8ff5d2c41e22a7f7cf6ee419
URL: https://github.com/llvm/llvm-project/commit/cb1642b6f9e458bf8ff5d2c41e22a7f7cf6ee419 DIFF: https://github.com/llvm/llvm-project/commit/cb1642b6f9e458bf8ff5d2c41e22a7f7cf6ee419.diff LOG: [Webkit Checkers][SaferCpp] Detect implicit `id`-to-specific-type casts in MemoryUnsafeCastChecker (#213113) Flag arguments of type `id` implicitly converted to a specific Objective-C pointer type at a call, message send, or constructor call (e.g. passing `id` where an `NSString *` parameter is expected). These conversions compile without a visible cast but throw at runtime if the object is not actually of that type. rdar://148708396 Added: Modified: clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp clang/test/Analysis/Checkers/WebKit/memory-unsafe-cast.mm Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp index f5e8247d17179..acd54ef137314 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp @@ -54,6 +54,7 @@ static void emitDiagnostics(const BoundNodes &Nodes, BugReporter &BR, BR.getSourceManager()); auto Report = std::make_unique<BasicBugReport>(BT, OS.str(), BSLoc); Report->addRange(CE->getSourceRange()); + Report->setDeclWithIssue(ADC->getDecl()); BR.emitReport(std::move(Report)); } @@ -74,21 +75,36 @@ static void emitDiagnosticsUnrelated(const BoundNodes &Nodes, BugReporter &BR, BR.getSourceManager()); auto Report = std::make_unique<BasicBugReport>(BT, OS.str(), BSLoc); Report->addRange(CE->getSourceRange()); + Report->setDeclWithIssue(ADC->getDecl()); BR.emitReport(std::move(Report)); } -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; - }); +static void emitDiagnosticsIdArg(const BoundNodes &Nodes, BugReporter &BR, + AnalysisDeclContext *ADC, + const MemoryUnsafeCastChecker *Checker, + const BugType &BT) { + const auto *CE = Nodes.getNodeAs<CastExpr>(WarnRecordDecl); + const NamedDecl *Derived = Nodes.getNodeAs<NamedDecl>(DerivedNode); + assert(CE && Derived); + + std::string Diagnostics; + llvm::raw_string_ostream OS(Diagnostics); + OS << "Unsafe implicit cast from 'id' to specific type '" + << Derived->getNameAsString() << "'"; + PathDiagnosticLocation BSLoc(CE->getSourceRange().getBegin(), + BR.getSourceManager()); + auto Report = std::make_unique<BasicBugReport>(BT, OS.str(), BSLoc); + Report->addRange(CE->getSourceRange()); + Report->setDeclWithIssue(ADC->getDecl()); + BR.emitReport(std::move(Report)); } +namespace { +using BoundNodesMap = ::clang::ast_matchers::internal::BoundNodesMap; + +// Matches the plain `id` type. +AST_MATCHER(QualType, isObjCIdType) { return Node->isObjCIdType(); } + // Matches a cast whose previously-bound BaseID node is a class template // specialization and whose previously-bound DerivedID node is one of that // specialization's type template arguments, i.e. the CRTP pattern @@ -111,8 +127,7 @@ AST_MATCHER_P2(Expr, isCRTPCast, std::string, BaseID, std::string, DerivedID) { return true; }); } -} // end namespace ast_matchers -} // end namespace clang +} // end anonymous namespace static decltype(auto) hasTypePointingTo(DeclarationMatcher DeclM) { return hasType(pointerType(pointee(hasDeclaration(DeclM)))); @@ -239,6 +254,29 @@ void MemoryUnsafeCastChecker::checkASTCodeBody(const Decl *D, *D->getBody(), AM.getASTContext()); for (BoundNodes Match : MatchesUnrelatedTypes) emitDiagnosticsUnrelated(Match, BR, ADC, this, BT); + + // Match an `id`-typed argument implicitly converted to a specific + // Objective-C type at a call, message send, or constructor call, e.g. + // passing an `id` where an `NSString *` parameter is expected. Such + // conversions compile without a visible cast but throw at runtime if the + // object is not actually of that type. + auto CastArgFromIdToSpecificType = + implicitCastExpr( + hasCastKind(CK_BitCast), + hasSourceExpression( + ignoringParenImpCasts(hasType(qualType(isObjCIdType())))), + hasType(qualType(hasCanonicalType(objcObjectPointerType(pointee( + hasDeclaration(objcInterfaceDecl().bind(DerivedNode)))))))) + .bind(WarnRecordDecl); + auto MatchCallArgFromId = + anyOf(callExpr(hasAnyArgument(CastArgFromIdToSpecificType)), + cxxConstructExpr(hasAnyArgument(CastArgFromIdToSpecificType)), + objcMessageExpr(hasAnyArgument(CastArgFromIdToSpecificType))); + auto MatchesCallArgFromId = + match(stmt(forEachDescendant(stmt(MatchCallArgFromId))), *D->getBody(), + AM.getASTContext()); + for (BoundNodes Match : MatchesCallArgFromId) + emitDiagnosticsIdArg(Match, BR, ADC, this, BT); } void ento::registerMemoryUnsafeCastChecker(CheckerManager &Mgr) { diff --git a/clang/test/Analysis/Checkers/WebKit/memory-unsafe-cast.mm b/clang/test/Analysis/Checkers/WebKit/memory-unsafe-cast.mm index 55b02b2657230..a2f1c78955206 100644 --- a/clang/test/Analysis/Checkers/WebKit/memory-unsafe-cast.mm +++ b/clang/test/Analysis/Checkers/WebKit/memory-unsafe-cast.mm @@ -97,3 +97,50 @@ void fn_cast_01(Base* base) { auto* d10 = reinterpret_cast<Derived*>((void*)base); // expected-warning@-1{{Unsafe cast from base type 'Base' to derived type 'Derived'}} } + +void takesNSString(NSString *str); + +@interface IdParamReceiver +- (void)takeString:(NSString *)str; +@end + +struct StringWrapper { + StringWrapper(NSString *str); +}; + +struct String { + String(NSString *str); +}; + +void test_id_passed_to_specific_type_param(id anId, NSString *str, IdParamReceiver *receiver) { + takesNSString(anId); + // expected-warning@-1{{Unsafe implicit cast from 'id' to specific type 'NSString'}} + [receiver takeString:anId]; + // expected-warning@-1{{Unsafe implicit cast from 'id' to specific type 'NSString'}} + StringWrapper wrapper1(anId); + // expected-warning@-1{{Unsafe implicit cast from 'id' to specific type 'NSString'}} + StringWrapper wrapper2 { anId }; + // expected-warning@-1{{Unsafe implicit cast from 'id' to specific type 'NSString'}} + + takesNSString(str); // no warning + [receiver takeString:str]; // no warning + StringWrapper wrapper3(str); // no warning + + NSString *fixed = checked_objc_cast<NSString>(anId); // no warning + takesNSString(fixed); // no warning + RetainPtr<NSString> fixedDynamic = dynamic_objc_cast<NSString>(anId); // no warning + + id array = [NSArray arrayWithObjects:0 count:0]; + String s { array }; + // expected-warning@-1{{Unsafe implicit cast from 'id' to specific type 'NSString'}} +} + +void takesNonnullNSString(NSString * _Nonnull str); +void takesNullableNSString(NSString * _Nullable str); + +void test_id_passed_with_nullability(id anId) { + takesNonnullNSString(anId); + // expected-warning@-1{{Unsafe implicit cast from 'id' to specific type 'NSString'}} + takesNullableNSString(anId); + // expected-warning@-1{{Unsafe implicit cast from 'id' to specific type 'NSString'}} +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
