https://github.com/rniwa created https://github.com/llvm/llvm-project/pull/231833
The checker used equalsBoundNode to relate the source and destination classes of a cast, which compares declaration nodes by identity. When the two types refer to different redeclarations of the same class (e.g. one spelled before and one after a `class X;` redeclaration following the definition), same-type casts such as `const_cast<X*>(p)` and upcasts were reported as casts to an unrelated type, and downcasts were reported as unrelated rather than as base-to-derived casts. Add an isSameDeclAsBoundNode matcher that compares canonical declarations and use it in place of equalsBoundNode. Also compare canonical declarations in the CRTP exclusion. Co-authored with Claude. >From 096dc650124b2a539bf9ea0ff4e7c4ab374a1fc5 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <[email protected]> Date: Wed, 14 Oct 2026 02:26:37 -0700 Subject: [PATCH] [alpha.webkit.MemoryUnsafeCastChecker] Don't emit a warning for const_cast The checker used equalsBoundNode to relate the source and destination classes of a cast, which compares declaration nodes by identity. When the two types refer to different redeclarations of the same class (e.g. one spelled before and one after a `class X;` redeclaration following the definition), same-type casts such as `const_cast<X*>(p)` and upcasts were reported as casts to an unrelated type, and downcasts were reported as unrelated rather than as base-to-derived casts. Add an isSameDeclAsBoundNode matcher that compares canonical declarations and use it in place of equalsBoundNode. Also compare canonical declarations in the CRTP exclusion. Co-authored with Claude. --- .../WebKit/MemoryUnsafeCastChecker.cpp | 38 +++++++++++----- .../WebKit/memory-unsafe-cast-redecl.cpp | 45 +++++++++++++++++++ 2 files changed, 71 insertions(+), 12 deletions(-) create mode 100644 clang/test/Analysis/Checkers/WebKit/memory-unsafe-cast-redecl.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp index acd54ef137314..7199cff7b1b60 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp @@ -105,6 +105,16 @@ using BoundNodesMap = ::clang::ast_matchers::internal::BoundNodesMap; // Matches the plain `id` type. AST_MATCHER(QualType, isObjCIdType) { return Node->isObjCIdType(); } +// Like equalsBoundNode, but compares canonical declarations so that different +// redeclarations of the same class (e.g. a forward declaration and the +// definition) are treated as the same declaration. +AST_MATCHER_P(Decl, isSameDeclAsBoundNode, std::string, ID) { + return Builder->removeBindings([&](const BoundNodesMap &Nodes) { + const auto *Bound = Nodes.getNodeAs<Decl>(ID); + return !Bound || Bound->getCanonicalDecl() != Node.getCanonicalDecl(); + }); +} + // 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 @@ -121,7 +131,11 @@ AST_MATCHER_P2(Expr, isCRTPCast, std::string, BaseID, std::string, DerivedID) { if (Arg.getKind() != TemplateArgument::Type) continue; QualType ArgType = Arg.getAsType(); - if (!ArgType.isNull() && ArgType->getAsCXXRecordDecl() == Derived) + if (ArgType.isNull()) + continue; + const auto *ArgDecl = ArgType->getAsCXXRecordDecl(); + if (ArgDecl && + ArgDecl->getCanonicalDecl() == Derived->getCanonicalDecl()) return false; } return true; @@ -150,7 +164,7 @@ void MemoryUnsafeCastChecker::checkASTCodeBody(const Decl *D, // Match downcasts from base type to derived type and warn auto MatchExprPtr = allOf( hasSourceExpression(hasTypePointingTo(cxxRecordDecl().bind(BaseNode))), - hasTypePointingTo(cxxRecordDecl(isDerivedFrom(equalsBoundNode(BaseNode))) + hasTypePointingTo(cxxRecordDecl(isDerivedFrom(isSameDeclAsBoundNode(BaseNode))) .bind(DerivedNode)), unless(anyOf(hasTypePointingTo(templateTypeParmDecl()), allOf(hasSourceExpression(cxxThisExpr()), @@ -159,13 +173,13 @@ void MemoryUnsafeCastChecker::checkASTCodeBody(const Decl *D, hasSourceExpression(ignoringImpCasts(hasType(objcObjectPointerType( pointee(hasDeclaration(objcInterfaceDecl().bind(BaseNode))))))), ignoringImpCasts(hasType(objcObjectPointerType(pointee(hasDeclaration( - objcInterfaceDecl(isDerivedFrom(equalsBoundNode(BaseNode))) + objcInterfaceDecl(isDerivedFrom(isSameDeclAsBoundNode(BaseNode))) .bind(DerivedNode))))))); auto MatchExprRefTypeDef = allOf(hasSourceExpression(hasType(hasUnqualifiedDesugaredType(recordType( hasDeclaration(decl(cxxRecordDecl().bind(BaseNode))))))), hasType(hasUnqualifiedDesugaredType(recordType(hasDeclaration( - decl(cxxRecordDecl(isDerivedFrom(equalsBoundNode(BaseNode))) + decl(cxxRecordDecl(isDerivedFrom(isSameDeclAsBoundNode(BaseNode))) .bind(DerivedNode)))))), unless(anyOf(hasType(templateTypeParmDecl()), allOf(hasSourceExpression(isThisOrDerefThis()), @@ -179,7 +193,7 @@ void MemoryUnsafeCastChecker::checkASTCodeBody(const Decl *D, callExpr(hasType(pointerType(pointee(voidType()))), hasAnyArgument(ignoringImpCasts(hasTypePointingTo( cxxRecordDecl().bind(BaseNode))))))), - hasTypePointingTo(cxxRecordDecl(isDerivedFrom(equalsBoundNode(BaseNode))) + hasTypePointingTo(cxxRecordDecl(isDerivedFrom(isSameDeclAsBoundNode(BaseNode))) .bind(DerivedNode))); auto ExplicitCast = @@ -202,7 +216,7 @@ void MemoryUnsafeCastChecker::checkASTCodeBody(const Decl *D, auto MatchCallPtrVoidArgCast = callExpr( hasAnyArgument(anyOf(VoidPtrCast, explicitCastExpr(hasSourceExpression(VoidPtrCast)))), - hasTypePointingTo(cxxRecordDecl(isDerivedFrom(equalsBoundNode(BaseNode))) + hasTypePointingTo(cxxRecordDecl(isDerivedFrom(isSameDeclAsBoundNode(BaseNode))) .bind(DerivedNode))); auto CallArgCast = stmt(MatchCallPtrVoidArgCast); auto MatchesCallArgCast = match(stmt(forEachDescendant(CallArgCast)), @@ -216,9 +230,9 @@ void MemoryUnsafeCastChecker::checkASTCodeBody(const Decl *D, hasTypePointingTo(cxxRecordDecl().bind(FromCastNode))), hasTypePointingTo(cxxRecordDecl().bind(ToCastNode)), unless(anyOf(hasTypePointingTo(cxxRecordDecl( - isSameOrDerivedFrom(equalsBoundNode(FromCastNode)))), + isSameOrDerivedFrom(isSameDeclAsBoundNode(FromCastNode)))), hasSourceExpression(hasTypePointingTo(cxxRecordDecl( - isSameOrDerivedFrom(equalsBoundNode(ToCastNode)))))))); + isSameOrDerivedFrom(isSameDeclAsBoundNode(ToCastNode)))))))); auto MatchExprPtrObjCUnrelatedTypes = allOf( hasSourceExpression(ignoringImpCasts(hasType(objcObjectPointerType( pointee(hasDeclaration(objcInterfaceDecl().bind(FromCastNode))))))), @@ -227,10 +241,10 @@ void MemoryUnsafeCastChecker::checkASTCodeBody(const Decl *D, unless(anyOf( ignoringImpCasts(hasType( objcObjectPointerType(pointee(hasDeclaration(objcInterfaceDecl( - isSameOrDerivedFrom(equalsBoundNode(FromCastNode)))))))), + isSameOrDerivedFrom(isSameDeclAsBoundNode(FromCastNode)))))))), hasSourceExpression(ignoringImpCasts(hasType( objcObjectPointerType(pointee(hasDeclaration(objcInterfaceDecl( - isSameOrDerivedFrom(equalsBoundNode(ToCastNode)))))))))))); + isSameOrDerivedFrom(isSameDeclAsBoundNode(ToCastNode)))))))))))); auto MatchExprRefTypeDefUnrelated = allOf( hasSourceExpression(hasType(hasUnqualifiedDesugaredType(recordType( hasDeclaration(decl(cxxRecordDecl().bind(FromCastNode))))))), @@ -239,10 +253,10 @@ void MemoryUnsafeCastChecker::checkASTCodeBody(const Decl *D, unless(anyOf( hasType(hasUnqualifiedDesugaredType( recordType(hasDeclaration(decl(cxxRecordDecl( - isSameOrDerivedFrom(equalsBoundNode(FromCastNode)))))))), + isSameOrDerivedFrom(isSameDeclAsBoundNode(FromCastNode)))))))), hasSourceExpression(hasType(hasUnqualifiedDesugaredType( recordType(hasDeclaration(decl(cxxRecordDecl( - isSameOrDerivedFrom(equalsBoundNode(ToCastNode)))))))))))); + isSameOrDerivedFrom(isSameDeclAsBoundNode(ToCastNode)))))))))))); auto ExplicitCastUnrelated = explicitCastExpr(anyOf(MatchExprPtrUnrelatedTypes, diff --git a/clang/test/Analysis/Checkers/WebKit/memory-unsafe-cast-redecl.cpp b/clang/test/Analysis/Checkers/WebKit/memory-unsafe-cast-redecl.cpp new file mode 100644 index 0000000000000..c39477c1adcab --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/memory-unsafe-cast-redecl.cpp @@ -0,0 +1,45 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.MemoryUnsafeCastChecker -verify %s + +// Types spelled before and after a redeclaration refer to different +// redeclarations of the same class. Casts between them must still be +// recognized as same-type, upcast, or downcast casts, not "unrelated" casts. + +class RedeclBase { +public: + virtual ~RedeclBase(); +}; + +class RedeclDerived : public RedeclBase { +public: + void method(); + void constMethod() const; +}; + +const RedeclDerived *getConstDerived(); +RedeclDerived *getDerived(); +RedeclBase *getBase(); + +class RedeclBase; +class RedeclDerived; + +void RedeclDerived::constMethod() const { + const_cast<RedeclDerived *>(this)->method(); +} + +void test_const_cast_redecl(const RedeclDerived &ref) { + const_cast<RedeclDerived *>(getConstDerived())->method(); + const_cast<RedeclDerived &>(*getConstDerived()).method(); + const_cast<RedeclDerived &>(ref).method(); +} + +void test_upcast_redecl() { + (void)static_cast<RedeclBase *>(getDerived()); + (void)static_cast<RedeclBase &>(*getDerived()); +} + +void test_downcast_redecl() { + (void)static_cast<RedeclDerived *>(getBase()); + // expected-warning@-1{{Unsafe cast from base type 'RedeclBase' to derived type 'RedeclDerived'}} + (void)static_cast<RedeclDerived &>(*getBase()); + // expected-warning@-1{{Unsafe cast from base type 'RedeclBase' to derived type 'RedeclDerived'}} +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
