llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-static-analyzer-1 Author: Rashmi Mudduluru (t-rasmud) <details> <summary>Changes</summary> This PR introduces a new checker `[alpha.webkit.MemoryUnsafeCastChecker]` that warns all downcasts from a base type to a derived type. rdar://137766829 --- Full diff: https://github.com/llvm/llvm-project/pull/114606.diff 7 Files Affected: - (modified) clang/docs/analyzer/checkers.rst (+25) - (modified) clang/docs/tools/clang-formatted-files.txt (+1) - (modified) clang/include/clang/StaticAnalyzer/Checkers/Checkers.td (+4) - (modified) clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt (+1) - (added) clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp (+86) - (added) clang/test/Analysis/Checkers/WebKit/memory-unsafe-cast.cpp (+151) - (added) clang/test/Analysis/Checkers/WebKit/memory-unsafe-cast.mm (+29) ``````````diff diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index 87b03438e6e0b9..f01755ce7a236a 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -3452,6 +3452,31 @@ alpha.WebKit .. _alpha-webkit-NoUncheckedPtrMemberChecker: +alpha.webkit.MemoryUnsafeCastChecker +"""""""""""""""""""""""""""""""""""""" +Check for all casts from a base type to its derived type as these might be memory-unsafe. + +Example: + +.. code-block:: cpp + +class Base { }; +class Derived : public Base { }; + +void f(Base* base) { + Derived* derived = static_cast<Derived*>(base); // ERROR +} + +For all cast operations (C-style casts, static_cast, reinterpret_cast, dynamic_cast), if the source type a `Base*` and the destination type is `Derived*`, where `Derived` inherits from `Base`, the static analyzer should signal an error. + +This applies to: + +- C structs, C++ structs and classes, and Objective-C classes and protocols. +- Pointers and references. +- Inside template instantiations and macro expansions that are visible to the compiler. + +For types like this, instead of using built in casts, the programmer will use helper functions that internally perform the appropriate type check and disable static analysis. + alpha.webkit.NoUncheckedPtrMemberChecker """""""""""""""""""""""""""""""""""""""" Raw pointers and references to an object which supports CheckedPtr or CheckedRef can't be used as class members. Only CheckedPtr, CheckedRef, RefPtr, or Ref are allowed. diff --git a/clang/docs/tools/clang-formatted-files.txt b/clang/docs/tools/clang-formatted-files.txt index 67ff085144f4de..74ab155d6174fd 100644 --- a/clang/docs/tools/clang-formatted-files.txt +++ b/clang/docs/tools/clang-formatted-files.txt @@ -537,6 +537,7 @@ clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h clang/lib/StaticAnalyzer/Checkers/WebKit/DiagOutputUtils.h +clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 9a6b35c1b9f774..445379e88ab9e3 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -1752,6 +1752,10 @@ def UncountedLambdaCapturesChecker : Checker<"UncountedLambdaCapturesChecker">, let ParentPackage = WebKitAlpha in { +def MemoryUnsafeCastChecker : Checker<"MemoryUnsafeCastChecker">, + HelpText<"Check for memory unsafe casts from base type to derived type.">, + Documentation<HasDocumentation>; + def NoUncheckedPtrMemberChecker : Checker<"NoUncheckedPtrMemberChecker">, HelpText<"Check for no unchecked member variables.">, Documentation<HasDocumentation>; diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt index 62aa5ff7f002a9..7e987740f9ee2d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -132,6 +132,7 @@ add_clang_library(clangStaticAnalyzerCheckers VirtualCallChecker.cpp WebKit/RawPtrRefMemberChecker.cpp WebKit/ASTUtils.cpp + WebKit/MemoryUnsafeCastChecker.cpp WebKit/PtrTypesSemantics.cpp WebKit/RefCntblBaseVirtualDtorChecker.cpp WebKit/UncountedCallArgsChecker.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp new file mode 100644 index 00000000000000..05a5f89d28c8fe --- /dev/null +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/MemoryUnsafeCastChecker.cpp @@ -0,0 +1,86 @@ +//=======- MemoryUnsafeCastChecker.cpp -------------------------*- C++ -*-==// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This file defines MemoryUnsafeCast checker, which checks for casts from a +// base type to a derived type. +//===----------------------------------------------------------------------===// + +#include "clang/AST/ASTContext.h" +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" + +using namespace clang; +using namespace ento; + +namespace { +class MemoryUnsafeCastChecker : public Checker<check::PreStmt<CastExpr>> { + BugType BT{this, ""}; + +public: + void checkPreStmt(const CastExpr *CE, CheckerContext &C) const; +}; +} // end namespace + +void emitWarning(CheckerContext &C, const CastExpr &CE, const BugType &BT, + QualType FromType, QualType ToType) { + ExplodedNode *errorNode = C.generateNonFatalErrorNode(); + if (!errorNode) + return; + SmallString<192> Buf; + llvm::raw_svector_ostream OS(Buf); + OS << "Memory unsafe cast from base type '"; + QualType::print(FromType.getTypePtr(), Qualifiers(), OS, C.getLangOpts(), + llvm::Twine()); + OS << "' to derived type '"; + QualType::print(ToType.getTypePtr(), Qualifiers(), OS, C.getLangOpts(), + llvm::Twine()); + OS << "'"; + auto R = std::make_unique<PathSensitiveBugReport>(BT, OS.str(), errorNode); + R->addRange(CE.getSourceRange()); + C.emitReport(std::move(R)); +} + +void MemoryUnsafeCastChecker::checkPreStmt(const CastExpr *CE, + CheckerContext &C) const { + auto ExpCast = dyn_cast_or_null<ExplicitCastExpr>(CE); + if (!ExpCast) + return; + + auto ToDerivedQualType = ExpCast->getTypeAsWritten(); + auto *SE = CE->getSubExprAsWritten(); + if (ToDerivedQualType->isObjCObjectPointerType()) { + auto FromBaseQualType = SE->getType(); + bool IsObjCSubType = + !C.getASTContext().hasSameType(ToDerivedQualType, FromBaseQualType) && + C.getASTContext().canAssignObjCInterfaces( + FromBaseQualType->getAsObjCInterfacePointerType(), + ToDerivedQualType->getAsObjCInterfacePointerType()); + if (IsObjCSubType) + emitWarning(C, *CE, BT, FromBaseQualType, ToDerivedQualType); + return; + } + auto ToDerivedType = ToDerivedQualType->getPointeeCXXRecordDecl(); + auto FromBaseType = SE->getType()->getPointeeCXXRecordDecl(); + if (!FromBaseType) + FromBaseType = SE->getType()->getAsCXXRecordDecl(); + if (!FromBaseType) + return; + if (ToDerivedType->isDerivedFrom(FromBaseType)) + emitWarning(C, *CE, BT, SE->getType(), ToDerivedQualType); +} + +void ento::registerMemoryUnsafeCastChecker(CheckerManager &Mgr) { + Mgr.registerChecker<MemoryUnsafeCastChecker>(); +} + +bool ento::shouldRegisterMemoryUnsafeCastChecker(const CheckerManager &) { + return true; +} diff --git a/clang/test/Analysis/Checkers/WebKit/memory-unsafe-cast.cpp b/clang/test/Analysis/Checkers/WebKit/memory-unsafe-cast.cpp new file mode 100644 index 00000000000000..1a4ef6858d2180 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/memory-unsafe-cast.cpp @@ -0,0 +1,151 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.MemoryUnsafeCastChecker -verify %s + +class Base { }; +class Derived : public Base { }; + +void test_pointers(Base *base) { + Derived *derived_static = static_cast<Derived*>(base); + // expected-warning@-1{{Memory unsafe cast from base type 'Base *' to derived type 'Derived *'}} + Derived *derived_reinterpret = reinterpret_cast<Derived*>(base); + // expected-warning@-1{{Memory unsafe cast from base type 'Base *' to derived type 'Derived *'}} + Derived *derived_c = (Derived*)base; + // expected-warning@-1{{Memory unsafe cast from base type 'Base *' to derived type 'Derived *'}} +} + +void test_refs(Base &base) { + Derived &derived_static = static_cast<Derived&>(base); + // expected-warning@-1{{Memory unsafe cast from base type 'Base' to derived type 'Derived &'}} + Derived &derived_reinterpret = reinterpret_cast<Derived&>(base); + // expected-warning@-1{{Memory unsafe cast from base type 'Base' to derived type 'Derived &'}} + Derived &derived_c = (Derived&)base; + // expected-warning@-1{{Memory unsafe cast from base type 'Base' to derived type 'Derived &'}} +} + +class BaseVirtual { + virtual void virtual_base_function(); +}; + +class DerivedVirtual : public BaseVirtual { + void virtual_base_function() override { } +}; + +void test_dynamic_casts(BaseVirtual *base_ptr, BaseVirtual &base_ref) { + DerivedVirtual *derived_dynamic_ptr = dynamic_cast<DerivedVirtual*>(base_ptr); + // expected-warning@-1{{Memory unsafe cast from base type 'BaseVirtual *' to derived type 'DerivedVirtual *'}} + DerivedVirtual &derived_dynamic_ref = dynamic_cast<DerivedVirtual&>(base_ref); + // expected-warning@-1{{Memory unsafe cast from base type 'BaseVirtual' to derived type 'DerivedVirtual &'}} +} + +struct BaseStruct { }; +struct DerivedStruct : BaseStruct { }; + +void test_struct_pointers(struct BaseStruct *base_struct) { + struct DerivedStruct *derived_static = static_cast<struct DerivedStruct*>(base_struct); + // expected-warning@-1{{Memory unsafe cast from base type 'struct BaseStruct *' to derived type 'struct DerivedStruct *'}} + struct DerivedStruct *derived_reinterpret = reinterpret_cast<struct DerivedStruct*>(base_struct); + // expected-warning@-1{{Memory unsafe cast from base type 'struct BaseStruct *' to derived type 'struct DerivedStruct *'}} + struct DerivedStruct *derived_c = (struct DerivedStruct*)base_struct; + // expected-warning@-1{{Memory unsafe cast from base type 'struct BaseStruct *' to derived type 'struct DerivedStruct *'}} +} + +typedef struct BaseStruct BStruct; +typedef struct DerivedStruct DStruct; + +void test_struct_refs(BStruct &base_struct) { + DStruct &derived_static = static_cast<DStruct&>(base_struct); + // expected-warning@-1{{Memory unsafe cast from base type 'BStruct' to derived type 'DStruct &'}} + DStruct &derived_reinterpret = reinterpret_cast<DStruct&>(base_struct); + // expected-warning@-1{{Memory unsafe cast from base type 'BStruct' to derived type 'DStruct &'}} + DStruct &derived_c = (DStruct&)base_struct; + // expected-warning@-1{{Memory unsafe cast from base type 'BStruct' to derived type 'DStruct &'}} +} + +int counter = 0; +void test_recursive(BStruct &base_struct) { + if (counter == 5) + return; + counter++; + DStruct &derived_static = static_cast<DStruct&>(base_struct); + // expected-warning@-1{{Memory unsafe cast from base type 'BStruct' to derived type 'DStruct &'}} +} + +template<typename T> +class BaseTemplate { }; + +template<typename T> +class DerivedTemplate : public BaseTemplate<T> { }; + +void test_templates(BaseTemplate<int> *base, BaseTemplate<int> &base_ref) { + DerivedTemplate<int> *derived_static = static_cast<DerivedTemplate<int>*>(base); + // expected-warning@-1{{Memory unsafe cast from base type 'BaseTemplate<int> *' to derived type 'DerivedTemplate<int> *'}} + DerivedTemplate<int> *derived_reinterpret = reinterpret_cast<DerivedTemplate<int>*>(base); + // expected-warning@-1{{Memory unsafe cast from base type 'BaseTemplate<int> *' to derived type 'DerivedTemplate<int> *'}} + DerivedTemplate<int> *derived_c = (DerivedTemplate<int>*)base; + // expected-warning@-1{{Memory unsafe cast from base type 'BaseTemplate<int> *' to derived type 'DerivedTemplate<int> *'}} + DerivedTemplate<int> &derived_static_ref = static_cast<DerivedTemplate<int>&>(base_ref); + // expected-warning@-1{{Memory unsafe cast from base type 'BaseTemplate<int>' to derived type 'DerivedTemplate<int> &'}} + DerivedTemplate<int> &derived_reinterpret_ref = reinterpret_cast<DerivedTemplate<int>&>(base_ref); + // expected-warning@-1{{Memory unsafe cast from base type 'BaseTemplate<int>' to derived type 'DerivedTemplate<int> &'}} + DerivedTemplate<int> &derived_c_ref = (DerivedTemplate<int>&)base_ref; + // expected-warning@-1{{Memory unsafe cast from base type 'BaseTemplate<int>' to derived type 'DerivedTemplate<int> &'}} +} + +#define CAST_MACRO_STATIC(X,Y) (static_cast<Y>(X)) +#define CAST_MACRO_REINTERPRET(X,Y) (reinterpret_cast<Y>(X)) +#define CAST_MACRO_C(X,Y) ((Y)X) + +void test_macro_static(Base *base, Derived *derived, Base &base_ref) { + Derived *derived_static = CAST_MACRO_STATIC(base, Derived*); + // expected-warning@-1{{Memory unsafe cast from base type 'Base *' to derived type 'Derived *'}} + Derived &derived_static_ref = CAST_MACRO_STATIC(base_ref, Derived&); + // expected-warning@-1{{Memory unsafe cast from base type 'Base' to derived type 'Derived &'}} + Base *base_static_same = CAST_MACRO_STATIC(base, Base*); // no warning + Base *base_static_upcast = CAST_MACRO_STATIC(derived, Base*); // no warning +} + +void test_macro_reinterpret(Base *base, Derived *derived, Base &base_ref) { + Derived *derived_reinterpret = CAST_MACRO_REINTERPRET(base, Derived*); + // expected-warning@-1{{Memory unsafe cast from base type 'Base *' to derived type 'Derived *'}} + Derived &derived_reinterpret_ref = CAST_MACRO_REINTERPRET(base_ref, Derived&); + // expected-warning@-1{{Memory unsafe cast from base type 'Base' to derived type 'Derived &' [alpha.webkit.MemoryUnsafeCastChecker]}} + Base *base_reinterpret_same = CAST_MACRO_REINTERPRET(base, Base*); // no warning + Base *base_reinterpret_upcast = CAST_MACRO_REINTERPRET(derived, Base*); // no warning +} + +void test_macro_c(Base *base, Derived *derived, Base &base_ref) { + Derived *derived_c = CAST_MACRO_C(base, Derived*); + // expected-warning@-1{{Memory unsafe cast from base type 'Base *' to derived type 'Derived *' [alpha.webkit.MemoryUnsafeCastChecker]}} + Derived &derived_c_ref = CAST_MACRO_C(base_ref, Derived&); + // expected-warning@-1{{Memory unsafe cast from base type 'Base' to derived type 'Derived &' [alpha.webkit.MemoryUnsafeCastChecker]}} + Base *base_c_same = CAST_MACRO_C(base, Base*); // no warning + Base *base_c_upcast = CAST_MACRO_C(derived, Base*); // no warning +} + +struct BaseStructCpp { + int t; + void increment() { t++; } +}; +struct DerivedStructCpp : BaseStructCpp { + void increment_t() {increment();} +}; + +void test_struct_cpp_pointers(struct BaseStructCpp *base_struct) { + struct DerivedStructCpp *derived_static = static_cast<struct DerivedStructCpp*>(base_struct); + // expected-warning@-1{{Memory unsafe cast from base type 'struct BaseStructCpp *' to derived type 'struct DerivedStructCpp *'}} + struct DerivedStructCpp *derived_reinterpret = reinterpret_cast<struct DerivedStructCpp*>(base_struct); + // expected-warning@-1{{Memory unsafe cast from base type 'struct BaseStructCpp *' to derived type 'struct DerivedStructCpp *'}} + struct DerivedStructCpp *derived_c = (struct DerivedStructCpp*)base_struct; + // expected-warning@-1{{Memory unsafe cast from base type 'struct BaseStructCpp *' to derived type 'struct DerivedStructCpp *'}} +} + +typedef struct BaseStructCpp BStructCpp; +typedef struct DerivedStructCpp DStructCpp; + +void test_struct_cpp_refs(BStructCpp &base_struct) { + DStructCpp &derived_static = static_cast<DStructCpp&>(base_struct); + // expected-warning@-1{{Memory unsafe cast from base type 'BStructCpp' to derived type 'DStructCpp &'}} + DStructCpp &derived_reinterpret = reinterpret_cast<DStructCpp&>(base_struct); + // expected-warning@-1{{Memory unsafe cast from base type 'BStructCpp' to derived type 'DStructCpp &'}} + DStructCpp &derived_c = (DStructCpp&)base_struct; + // expected-warning@-1{{Memory unsafe cast from base type 'BStructCpp' to derived type 'DStructCpp &'}} +} diff --git a/clang/test/Analysis/Checkers/WebKit/memory-unsafe-cast.mm b/clang/test/Analysis/Checkers/WebKit/memory-unsafe-cast.mm new file mode 100644 index 00000000000000..1ea2f4aa472715 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/memory-unsafe-cast.mm @@ -0,0 +1,29 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.MemoryUnsafeCastChecker -verify %s + +@protocol NSObject ++alloc; +-init; +@end + +@interface NSObject <NSObject> {} +@end + +@interface BaseClass : NSObject +@end + +@interface DerivedClass : BaseClass +-(void)testCasts:(BaseClass*)base; +@end + +@implementation DerivedClass +-(void)testCasts:(BaseClass*)base { + DerivedClass *derived = (DerivedClass*)base; + // expected-warning@-1{{Memory unsafe cast from base type 'BaseClass *' to derived type 'DerivedClass *'}} + DerivedClass *derived_static = static_cast<DerivedClass*>(base); + // expected-warning@-1{{Memory unsafe cast from base type 'BaseClass *' to derived type 'DerivedClass *'}} + DerivedClass *derived_reinterpret = reinterpret_cast<DerivedClass*>(base); + // expected-warning@-1{{Memory unsafe cast from base type 'BaseClass *' to derived type 'DerivedClass *'}} + base = (BaseClass*)derived; // no warning + base = (BaseClass*)base; // no warning +} +@end `````````` </details> https://github.com/llvm/llvm-project/pull/114606 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
