https://github.com/t-rasmud updated https://github.com/llvm/llvm-project/pull/213113
>From 46df8bb595fdb64aefa2d550ea841c037a2bbf03 Mon Sep 17 00:00:00 2001 From: Rashmi Mudduluru <[email protected]> Date: Thu, 30 Jul 2026 12:24:56 -0700 Subject: [PATCH 1/4] [Webkit Checkers][SaferCpp] Detect implicit `id`-to-specific-type casts in MemoryUnsafeCastChecker 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 --- .../WebKit/MemoryUnsafeCastChecker.cpp | 45 ++++++++++++++++++ .../Checkers/WebKit/memory-unsafe-cast.mm | 47 +++++++++++++++++++ 2 files changed, 92 insertions(+) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp index f5e8247d17179..0adb3c5480e9d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp @@ -77,6 +77,25 @@ static void emitDiagnosticsUnrelated(const BoundNodes &Nodes, BugReporter &BR, BR.emitReport(std::move(Report)); } +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()); + BR.emitReport(std::move(Report)); +} + namespace clang { namespace ast_matchers { AST_MATCHER_P(StringLiteral, mentionsBoundType, std::string, BindingID) { @@ -89,6 +108,9 @@ AST_MATCHER_P(StringLiteral, mentionsBoundType, std::string, BindingID) { }); } +// 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 @@ -239,6 +261,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'}} +} >From 5a8af1f05468ca06a81488d775f2642ad12526e4 Mon Sep 17 00:00:00 2001 From: Rashmi Mudduluru <[email protected]> Date: Thu, 30 Jul 2026 12:36:53 -0700 Subject: [PATCH 2/4] Fix formatting --- .../Checkers/WebKit/MemoryUnsafeCastChecker.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp index 0adb3c5480e9d..0846df6990e62 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp @@ -279,9 +279,9 @@ void MemoryUnsafeCastChecker::checkASTCodeBody(const Decl *D, anyOf(callExpr(hasAnyArgument(CastArgFromIdToSpecificType)), cxxConstructExpr(hasAnyArgument(CastArgFromIdToSpecificType)), objcMessageExpr(hasAnyArgument(CastArgFromIdToSpecificType))); - auto MatchesCallArgFromId = match( - stmt(forEachDescendant(stmt(MatchCallArgFromId))), *D->getBody(), - AM.getASTContext()); + auto MatchesCallArgFromId = + match(stmt(forEachDescendant(stmt(MatchCallArgFromId))), *D->getBody(), + AM.getASTContext()); for (BoundNodes Match : MatchesCallArgFromId) emitDiagnosticsIdArg(Match, BR, ADC, this, BT); } >From 67b48e9fc825af65ff67bd52165ec7a3a8e04dd7 Mon Sep 17 00:00:00 2001 From: Rashmi Mudduluru <[email protected]> Date: Thu, 30 Jul 2026 12:45:36 -0700 Subject: [PATCH 3/4] Fix formatting --- .../Checkers/WebKit/MemoryUnsafeCastChecker.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp index 0846df6990e62..d90adad87acb6 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp @@ -270,10 +270,10 @@ void MemoryUnsafeCastChecker::checkASTCodeBody(const Decl *D, auto CastArgFromIdToSpecificType = implicitCastExpr( hasCastKind(CK_BitCast), - hasSourceExpression(ignoringParenImpCasts( - hasType(qualType(isObjCIdType())))), - hasType(qualType(hasCanonicalType(objcObjectPointerType( - pointee(hasDeclaration(objcInterfaceDecl().bind(DerivedNode)))))))) + hasSourceExpression( + ignoringParenImpCasts(hasType(qualType(isObjCIdType())))), + hasType(qualType(hasCanonicalType(objcObjectPointerType(pointee( + hasDeclaration(objcInterfaceDecl().bind(DerivedNode)))))))) .bind(WarnRecordDecl); auto MatchCallArgFromId = anyOf(callExpr(hasAnyArgument(CastArgFromIdToSpecificType)), >From 5d783861f2423cca82394c8e73a52d2ee72f871b Mon Sep 17 00:00:00 2001 From: Rashmi Mudduluru <[email protected]> Date: Thu, 30 Jul 2026 16:45:00 -0700 Subject: [PATCH 4/4] Address comments: anonymous namespace for AST_MATCHERS and setDeclWithIssue --- .../WebKit/MemoryUnsafeCastChecker.cpp | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp index d90adad87acb6..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,6 +75,7 @@ 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)); } @@ -93,20 +95,12 @@ static void emitDiagnosticsIdArg(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; - }); -} +namespace { +using BoundNodesMap = ::clang::ast_matchers::internal::BoundNodesMap; // Matches the plain `id` type. AST_MATCHER(QualType, isObjCIdType) { return Node->isObjCIdType(); } @@ -133,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)))); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
