https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87697
--- Comment #2 from jynelson at email dot sc.edu --- (In reply to Jonathan Wakely from comment #1) > (In reply to jynelson from comment #0) > > Casting a base class to a derived class gives no warning, even with -Wall > > -Werror enabled. I've been told on IRC that this sort of cast is undefined > > behaviour according to spec if neither class is virtual. > > It has nothing to do with whether anything is virtual or not. The relevant > standard wording is: > > If the object of type “cv1 B” is actually a base class subobject of an > object of type D, the result refers to the enclosing object of type D. > Otherwise, the behavior is undefined. Found this at http://www.open-std.org/jtc1/sc22/open/n2356/expr.html, good to know it's undefined for sure. > > > If that is the > > case, it would be nice to have a warning. > > It's only possible to warn when the compiler can statically prove that the > pointer really isn't pointing to a derived. That's possible in your example > because it was created right there by the 'new c1' expression, but that is a > very silly thing to do and surely nobody does it in real code. Generally a > function just gets passed a pointer to a base class, and can't see its > construction, so can't tell if the cast is valid or not. I would argue the compiler is in the best place to statically prove an invalid downcast, since it knows the types of every object. I'm not sure how functions relate to this, but if it's easier to go by a function's type signature, I think it would be better to warn on *any* downcast, and make the developer specifically override that warning. Part of the reason I opened the bug is I didn't know this was illegal at first. > > > If that isn't the case, there are several more questions that arise, like > > `why does c1 have access to c2` and `why can the const value c2::x be > > accessed without be initialized`? > > It doesn't, because there's no c2 object constructed. Obviously the code is > silly and broken. Of course it's silly and broken, it's an example. In a larger project, where there's an intermediary object defined many lines away from this assignment, it would be much easier for this to slip by even with code review on the assumption that the original was the correct class. something like this: ``` c1 original; // ... a bunch of code ... // looks reasonable derived *obj = static_cast<derived*>( &original ); ``` This is exacerbated by static cast never giving a warning. This is OK for integers and floats where downcasts are well-defined, but for undefined behaviour there should at least be a warning. I would actually argue it should be an error, the same way casting completely different types is an error (https://stackoverflow.com/questions/14380275). I'll attach some code that compiles (except for the error) as an example. > > I don't think a compiler warning would be worthwhile, because it would only > warn about code which is obviously wrong by inspection i.e. which should be > caught by the most cursory code review. I disagree that that it would have to be cursory. > The general case can be diagnosed through runtime instrumentation but > -fsanitize=undefined only gives a runtime error for an invalid static_cast > involving polymorphic types (because the type identification is linked to > the vtable). > > -fsanitize=address does diagnose that accessing members of c2 performs an > out-of-bounds read: > This is useful, but a little obscure, I doubt it's widely used.
