dynamic_cast for the price of static_cast

At least for polymorphic types, the cool -fsanitize=undefined feature at runtime flags any broken downcasts, effectively turning all casts into dynamic_casts (which are often avoided in the code due to their cost in production builds):

#include <iostream>
struct S1 {};
struct S2: S1 {};
struct T1 { virtual ~T1() {} };
struct T2: T1 {};
int main() {
    S1 s;
    std::cout << static_cast<S2*>(&s) << '\n';
    T1 t;
    std::cout << static_cast<T2*>(&t) << '\n';

when compiled with -fsanitize=undefined will complain

undef.cc:10:18: runtime error: downcast of address 0x7fffa604fc20 which does not point to an object of type 'T2'

(but still fail to find the broken downcast from S1 to S2).

Letting this loose on the LibreOffice code base results in a plethora of reports about somewhat harmless violations:

For one, many places in the code over-eagerly downcast a pointer to some T3 in a hierarchy where the resulting pointer is only used in a context that merely requires a T2. A trivial example is 14e81a12b0fa6a7bcd9fb29870db8b8bb67b25dd “Not every SwGluePortion is an SwFlyPortion.”

A special subset of this case are trivial derivations in the SfxPoolItem hierarchy that only exist to provide “convenience constructors“ that implicitly specify the right “Which-ID,“ but fail to override Clone (so that client code that downcasts an item to such a convenience class fails for a cloned item). See 68969cc61adecac481ae9656978ef952f435b310 “Consistency around SdrMetricItem” et al.

For another, some base class constructors or destructors (indirectly) call sub-class–specific code, and that code then erroneously finds this to—theoretically, at least—point at an object of merely the base class rather than the whole sub-class type. An example is b75e052d31ec8854cad3bda7d372dcfcd50c9609 “Call SwTxtNode-specific part of DelFrms while SwTxtNode is still SwTxtNode.”

And among all that “noise,” -fsanitize=undefined also finds genuine bugs like a30ce480fa6044b1545145559cd23df140307bd0 “These SwTxtFrm calls shall apparently be done via pOwn, not m_rThis.”


Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s