[Clang] Diagnose down casting static_cast

Invalid down casts are a source of vulnerabilities.
Its allowance in static_cast - which is otherwise
relatively safe - are recognized as dangerous by
various safety guidelines such as the core guidelines and misra.

While there exist a clang-tidy check, it seems reasonable,
in the interest of safety to warn about that construct in clang
and to propose its deprecation in WG21.

This is inspired by
[p3081r0](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2024/p3081r0.pdf)

which propose to promote static_cast to dynamic_cast silently, which
would have further issues (suddently static_cast could produce a null pointer).

Part of the goal of this PR is therefore to demonstrate the viability
of deprecating this construct.

Author: cor3ntin

clang/docs/ReleaseNotes.rst

@@ -593,6 +593,10 @@ Improvements to Clang's diagnostics
 - Clang now supports using alias templates in deduction guides, aligning with the C++ standard,
   which treats alias templates as synonyms for their underlying types (#GH54909).
 
+- Added ``-Wstatic-downcast`` to diagnose potentially dangerous uses of ``static_cast`` that perform a base-to-derived cast
+  on pointers and references of polymorphic types.
+
+
 Improvements to Clang's time-trace
 ----------------------------------
 

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -7982,6 +7982,9 @@ def err_bad_reinterpret_cast_reference : Error<
 def warn_undefined_reinterpret_cast : Warning<
   "reinterpret_cast from %0 to %1 has undefined behavior">,
   InGroup<UndefinedReinterpretCast>, DefaultIgnore;
+def warn_static_downcast : Warning<
+  "static downcast from %0 to %1 is potentially dangerous%select{|; did you mean 'dynamic_cast'?}2">,
+   InGroup<DiagGroup<"static-downcast">>;
 
 // These messages don't adhere to the pattern.
 // FIXME: Display the path somehow better.

clang/lib/Sema/SemaCast.cpp

@@ -1759,6 +1759,16 @@ TryStaticDowncast(Sema &Self, CanQualType SrcType, CanQualType DestType,
 
   Self.BuildBasePathArray(Paths, BasePath);
   Kind = CK_BaseToDerived;
+
+  if (!CStyle && Self.LangOpts.CPlusPlus && SrcType->getAsCXXRecordDecl()->isPolymorphic()) {
+    auto D = Self.Diag(OpRange.getBegin(), diag::warn_static_downcast)
+        << SrcType << DestType
+        << OpRange
+        << Self.LangOpts.RTTI;
+    if(Self.LangOpts.RTTI)
+       D << FixItHint::CreateReplacement(OpRange.getBegin(), "dynamic_cast");
+  }
+
   return TC_Success;
 }
 

clang/test/Analysis/ArrayDelete.cpp

@@ -21,7 +21,7 @@ void sink(Base *b) {
 }
 
 void sink_cast(Base *b) {
-    delete[] static_cast<Derived*>(b); // no-warning
+    delete[] static_cast<Derived*>(b); // expected-warning {{static downcast from 'Base' to 'Derived'}}
 }
 
 void sink_derived(Derived *d) {
@@ -62,7 +62,7 @@ void safe_function() {
     delete[] d; // no-warning
 
     Base *b = new Derived[10];
-    delete[] static_cast<Derived*>(b); // no-warning
+    delete[] static_cast<Derived*>(b); // expected-warning {{static downcast from 'Base' to 'Derived'}}
 
     Base *sb = new Derived[10];
     sink_cast(sb); // no-warning
@@ -77,7 +77,8 @@ void multiple_derived() {
     // expected-note@-1{{Deleting an array of 'DoubleDerived' objects as their base class 'Base' is undefined}}
 
     Base *b2 = new DoubleDerived[10]; // expected-note{{Casting from 'DoubleDerived' to 'Base' here}}
-    Derived *d2 = static_cast<Derived*>(b2); // expected-note{{Casting from 'Base' to 'Derived' here}}
+    Derived *d2 = static_cast<Derived*>(b2); // expected-note{{Casting from 'Base' to 'Derived' here}} \
+                                             // expected-warning {{static downcast from 'Base' to 'Derived'}}
     delete[] d2; // expected-warning{{Deleting an array of 'DoubleDerived' objects as their base class 'Derived' is undefined}}
     // expected-note@-1{{Deleting an array of 'DoubleDerived' objects as their base class 'Derived' is undefined}}
 
@@ -87,12 +88,13 @@ void multiple_derived() {
     // expected-note@-1{{Deleting an array of 'DoubleDerived' objects as their base class 'Base' is undefined}}
 
     Base *b4 = new DoubleDerived[10];
-    Derived *d4 = static_cast<Derived*>(b4);
-    DoubleDerived *dd4 = static_cast<DoubleDerived*>(d4);
+    Derived *d4 = static_cast<Derived*>(b4); // expected-warning {{static downcast from 'Base' to 'Derived'}}
+    DoubleDerived *dd4 = static_cast<DoubleDerived*>(d4); // expected-warning {{static downcast from 'Derived' to 'DoubleDerived'}}
     delete[] dd4; // no-warning
 
     Base *b5 = new DoubleDerived[10]; // expected-note{{Casting from 'DoubleDerived' to 'Base' here}}
-    DoubleDerived *dd5 = static_cast<DoubleDerived*>(b5); // expected-note{{Casting from 'Base' to 'DoubleDerived' here}}
+    DoubleDerived *dd5 = static_cast<DoubleDerived*>(b5); // expected-note{{Casting from 'Base' to 'DoubleDerived' here}} \
+                                                          // expected-warning {{static downcast from 'Base' to 'DoubleDerived'}}
     Derived *d5 = dd5; // expected-note{{Casting from 'DoubleDerived' to 'Derived' here}}
     delete[] d5; // expected-warning{{Deleting an array of 'DoubleDerived' objects as their base class 'Derived' is undefined}}
     // expected-note@-1{{Deleting an array of 'DoubleDerived' objects as their base class 'Derived' is undefined}}
@@ -103,7 +105,8 @@ void unrelated_casts() {
     Base &b2 = *b; // no-note: See the FIXME.
 
     // FIXME: Displaying casts of reference types is not supported.
-    Derived &d2 = static_cast<Derived&>(b2); // no-note: See the FIXME.
+    Derived &d2 = static_cast<Derived&>(b2); // expected-warning {{static downcast from 'Base' to 'Derived'}}
+                                             // no-note: See the FIXME.
 
     Derived *d = &d2; // no-note: See the FIXME.
     delete[] d; // expected-warning{{Deleting an array of 'DoubleDerived' objects as their base class 'Derived' is undefined}}

clang/test/Analysis/Checkers/WebKit/call-args-safe-functions.cpp

@@ -1,5 +1,4 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s
-// expected-no-diagnostics
 
 class Base {
 public:
@@ -30,27 +29,30 @@ template<typename Target, typename Source>
 inline Target* dynamicDowncast(Source* source)
 {
     return static_cast<Target*>(source);
+    // expected-warning@-1 {{static downcast from 'Derived' to 'SubDerived'}}
 }
 
 template<typename Target, typename Source>
 inline Target* checkedDowncast(Source* source)
 {
     return static_cast<Target*>(source);
+    // expected-warning@-1 {{static downcast from 'Derived' to 'SubDerived'}}
 }
 
 template<typename Target, typename Source>
 inline Target* uncheckedDowncast(Source* source)
 {
     return static_cast<Target*>(source);
+    // expected-warning@-1 {{static downcast from 'Derived' to 'SubDerived'}}
 }
 
 template<typename... Types>
 String toString(const Types&... values);
 
 void foo(OtherObject* other)
 {
-    dynamicDowncast<SubDerived>(other->obj());
-    checkedDowncast<SubDerived>(other->obj());
-    uncheckedDowncast<SubDerived>(other->obj());
+    dynamicDowncast<SubDerived>(other->obj()); // expected-note {{in instantiation}}
+    checkedDowncast<SubDerived>(other->obj()); // expected-note {{in instantiation}}
+    uncheckedDowncast<SubDerived>(other->obj()); // expected-note {{in instantiation}}
     toString(other->obj());
 }

clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor-templates.cpp

@@ -327,6 +327,7 @@ void UseDerivedClass11(DerivedClass12& obj) { obj.deref(); }
 void deleteBase2(BaseClass2* obj) {
   if (obj->isDerived())
     delete static_cast<DerivedClass12*>(obj);
+    // expected-warning@-1 {{static downcast from 'BaseClass2' to 'DerivedClass12'}}
   else
     delete obj;
 }
@@ -356,6 +357,7 @@ void UseDerivedClass11(DerivedClass13& obj) { obj.deref(); }
 void BaseClass3::destory() {
   if (isDerived())
     delete static_cast<DerivedClass13*>(this);
+    // expected-warning@-1 {{static downcast from 'BaseClass3' to 'DerivedClass13'}}
   else
     delete this;
 }

clang/test/Analysis/cast-to-struct.cpp

@@ -41,20 +41,21 @@ void structToStruct(struct AB *P) {
   Base &B1 = D1;
   D2 = (Derived *)&B1;
   D2 = dynamic_cast<Derived *>(&B1);
-  D2 = static_cast<Derived *>(&B1);
+  D2 = static_cast<Derived *>(&B1); // expected-warning {{static downcast from 'Base' to 'Derived'}}
 
   // True positives when casting from Base to Derived.
   Base B2;
   D2 = (Derived *)&B2;// expected-warning {{Casting data to a larger structure type and accessing a field can lead to memory access errors or data corruption}}
   D2 = dynamic_cast<Derived *>(&B2);// expected-warning {{Casting data to a larger structure type and accessing a field can lead to memory access errors or data corruption}}
-  D2 = static_cast<Derived *>(&B2);// expected-warning {{Casting data to a larger structure type and accessing a field can lead to memory access errors or data corruption}}
+  D2 = static_cast<Derived *>(&B2);// expected-warning {{Casting data to a larger structure type and accessing a field can lead to memory access errors or data corruption}} \
+                                   // expected-warning {{static downcast from 'Base' to 'Derived'}}
 
   // False negatives, cast from Base to Derived. With path sensitive analysis
   // these false negatives could be fixed.
   Base *B3 = &B2;
   D2 = (Derived *)B3;
   D2 = dynamic_cast<Derived *>(B3);
-  D2 = static_cast<Derived *>(B3);
+  D2 = static_cast<Derived *>(B3); // expected-warning {{static downcast from 'Base' to 'Derived'}}
 }
 
 void intToStruct(int *P) {

clang/test/Analysis/ctor.mm

@@ -849,11 +849,11 @@ void test() {
   // constructor could have been called on an initialized piece of memory),
   // so no uninitialized value warning here, and these should be symbols, not
   // undefined values, for later comparison.
-  glob_y = static_cast<D *>(this)->y;
-  glob_z = static_cast<D *>(this)->z;
-  glob_w = static_cast<D *>(this)->w;
-  static_cast<D *>(this)->y = 2;
-  static_cast<D *>(this)->z = 3;
-  static_cast<D *>(this)->w = 4;
+  glob_y = static_cast<D *>(this)->y; // expected-warning {{static downcast}}
+  glob_z = static_cast<D *>(this)->z; // expected-warning {{static downcast}}
+  glob_w = static_cast<D *>(this)->w; // expected-warning {{static downcast}}
+  static_cast<D *>(this)->y = 2; // expected-warning {{static downcast}}
+  static_cast<D *>(this)->z = 3; // expected-warning {{static downcast}}
+  static_cast<D *>(this)->w = 4; // expected-warning {{static downcast}}
 }
 }

clang/test/SemaCXX/address-space-conversion.cpp

@@ -56,9 +56,9 @@ void test_static_cast(void_ptr vp, void_ptr_1 vp1, void_ptr_2 vp2,
   (void)static_cast<A_ptr_2>(bp2);
 
   // Well-formed downcast
-  (void)static_cast<B_ptr>(ap);
-  (void)static_cast<B_ptr_1>(ap1);
-  (void)static_cast<B_ptr_2>(ap2);
+  (void)static_cast<B_ptr>(ap);  // expected-warning {{static downcast from 'A' to 'B'}}
+  (void)static_cast<B_ptr_1>(ap1);  // expected-warning {{static downcast}}
+  (void)static_cast<B_ptr_2>(ap2);  // expected-warning {{static downcast}}
 
   // Well-formed cast to/from void
   (void)static_cast<void_ptr>(ap);