Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.


Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be just 1 newline?

Improvements to Clang's time-trace
----------------------------------

Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -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">,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is more than 'potentially dangerous', yes? Perhaps also we should mention the polymorphic-ness of the types as to why it is dangerous.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you cast to the correct derived class, it works!

Perhaps also we should mention the polymorphic-ness of the types as to why it is dangerous.

Yes

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right, yes... I wish we could explain that better in the brevity we have available here. I kinda hate this is a warning with false-positives, but IMO the goal of trying to make this a deprecated cast is worth the effort.

InGroup<DiagGroup<"static-downcast">>;

// These messages don't adhere to the pattern.
// FIXME: Display the path somehow better.
Expand Down
10 changes: 10 additions & 0 deletions clang/lib/Sema/SemaCast.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there value to some sort of 'note' where we point out where the inheritence happened? Though printing the whole inheritence list is probably pretty awful.

if(Self.LangOpts.RTTI)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if(Self.LangOpts.RTTI)
if (Self.LangOpts.RTTI)

D << FixItHint::CreateReplacement(OpRange.getBegin(), "dynamic_cast");
}

return TC_Success;
}

Expand Down
17 changes: 10 additions & 7 deletions clang/test/Analysis/ArrayDelete.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -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}}

Expand All @@ -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}}
Expand All @@ -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}}
Expand Down
10 changes: 6 additions & 4 deletions clang/test/Analysis/Checkers/WebKit/call-args-safe-functions.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s
// expected-no-diagnostics

class Base {
public:
Expand Down Expand Up @@ -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());
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down
7 changes: 4 additions & 3 deletions clang/test/Analysis/cast-to-struct.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
12 changes: 6 additions & 6 deletions clang/test/Analysis/ctor.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
}
}
6 changes: 3 additions & 3 deletions clang/test/SemaCXX/address-space-conversion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading