diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index b1b8f3bfa33b1..70b0269d8cd75 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -593,6 +593,9 @@ 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 ---------------------------------- diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 1d777f670097b..2de5de50d8699 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/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, DefaultIgnore; +def warn_static_downcast : Warning< + "static downcast from %0 to %1 is potentially dangerous%select{|; did you mean 'dynamic_cast'?}2">, + InGroup>; // These messages don't adhere to the pattern. // FIXME: Display the path somehow better. diff --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp index f98857f852b5a..e3159f968abc2 100644 --- a/clang/lib/Sema/SemaCast.cpp +++ b/clang/lib/Sema/SemaCast.cpp @@ -19,6 +19,7 @@ #include "clang/AST/ExprCXX.h" #include "clang/AST/ExprObjC.h" #include "clang/AST/RecordLayout.h" +#include "clang/Basic/Diagnostic.h" #include "clang/Basic/PartialDiagnostic.h" #include "clang/Basic/TargetInfo.h" #include "clang/Lex/Preprocessor.h" @@ -1759,6 +1760,17 @@ 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; } diff --git a/clang/test/Analysis/ArrayDelete.cpp b/clang/test/Analysis/ArrayDelete.cpp index 6887e0a35fb8b..94a17746414eb 100644 --- a/clang/test/Analysis/ArrayDelete.cpp +++ b/clang/test/Analysis/ArrayDelete.cpp @@ -21,7 +21,7 @@ void sink(Base *b) { } void sink_cast(Base *b) { - delete[] static_cast(b); // no-warning + delete[] static_cast(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(b); // no-warning + delete[] static_cast(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(b2); // expected-note{{Casting from 'Base' to 'Derived' here}} + Derived *d2 = static_cast(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(b4); - DoubleDerived *dd4 = static_cast(d4); + Derived *d4 = static_cast(b4); // expected-warning {{static downcast from 'Base' to 'Derived'}} + DoubleDerived *dd4 = static_cast(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(b5); // expected-note{{Casting from 'Base' to 'DoubleDerived' here}} + DoubleDerived *dd5 = static_cast(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(b2); // no-note: See the FIXME. + Derived &d2 = static_cast(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}} diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-safe-functions.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-safe-functions.cpp index a87446564870c..322aa223db057 100644 --- a/clang/test/Analysis/Checkers/WebKit/call-args-safe-functions.cpp +++ b/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,18 +29,21 @@ template inline Target* dynamicDowncast(Source* source) { return static_cast(source); + // expected-warning@-1 {{static downcast from 'Derived' to 'SubDerived'}} } template inline Target* checkedDowncast(Source* source) { return static_cast(source); + // expected-warning@-1 {{static downcast from 'Derived' to 'SubDerived'}} } template inline Target* uncheckedDowncast(Source* source) { return static_cast(source); + // expected-warning@-1 {{static downcast from 'Derived' to 'SubDerived'}} } template @@ -49,8 +51,8 @@ String toString(const Types&... values); void foo(OtherObject* other) { - dynamicDowncast(other->obj()); - checkedDowncast(other->obj()); - uncheckedDowncast(other->obj()); + dynamicDowncast(other->obj()); // expected-note {{in instantiation}} + checkedDowncast(other->obj()); // expected-note {{in instantiation}} + uncheckedDowncast(other->obj()); // expected-note {{in instantiation}} toString(other->obj()); } diff --git a/clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor-templates.cpp b/clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor-templates.cpp index 4fc1624d7a154..c49e1e07ccfb5 100644 --- a/clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor-templates.cpp +++ b/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(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(this); + // expected-warning@-1 {{static downcast from 'BaseClass3' to 'DerivedClass13'}} else delete this; } diff --git a/clang/test/Analysis/cast-to-struct.cpp b/clang/test/Analysis/cast-to-struct.cpp index c3aba023c56e4..b994000aca789 100644 --- a/clang/test/Analysis/cast-to-struct.cpp +++ b/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(&B1); - D2 = static_cast(&B1); + D2 = static_cast(&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(&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(&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(&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(B3); - D2 = static_cast(B3); + D2 = static_cast(B3); // expected-warning {{static downcast from 'Base' to 'Derived'}} } void intToStruct(int *P) { diff --git a/clang/test/Analysis/ctor.mm b/clang/test/Analysis/ctor.mm index 6ac9050fc29f7..2ab0aa59b1924 100644 --- a/clang/test/Analysis/ctor.mm +++ b/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(this)->y; - glob_z = static_cast(this)->z; - glob_w = static_cast(this)->w; - static_cast(this)->y = 2; - static_cast(this)->z = 3; - static_cast(this)->w = 4; + glob_y = static_cast(this)->y; // expected-warning {{static downcast}} + glob_z = static_cast(this)->z; // expected-warning {{static downcast}} + glob_w = static_cast(this)->w; // expected-warning {{static downcast}} + static_cast(this)->y = 2; // expected-warning {{static downcast}} + static_cast(this)->z = 3; // expected-warning {{static downcast}} + static_cast(this)->w = 4; // expected-warning {{static downcast}} } } diff --git a/clang/test/SemaCXX/address-space-conversion.cpp b/clang/test/SemaCXX/address-space-conversion.cpp index b1fb69816334d..3620d0997d5e8 100644 --- a/clang/test/SemaCXX/address-space-conversion.cpp +++ b/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(bp2); // Well-formed downcast - (void)static_cast(ap); - (void)static_cast(ap1); - (void)static_cast(ap2); + (void)static_cast(ap); // expected-warning {{static downcast from 'A' to 'B'}} + (void)static_cast(ap1); // expected-warning {{static downcast}} + (void)static_cast(ap2); // expected-warning {{static downcast}} // Well-formed cast to/from void (void)static_cast(ap);