1

Following up a similar question in previous post about downcasting and type safety , I wonder if the following example creates undefined behavior.

An instance of Base class has been created. No dynamic binding take place.
However, in the Base::interface function the instance of the Base class is casted to an instance of the Derived class . Is this safe? If yes , why ? Please find the piece of code below.

#include <iostream>
template <typename Derived>
struct Base{
  void interface(){
    static_cast<Derived*>(this)->implementation();
  }
};

struct Derived1: Base<Derived1>{
  void implementation(){
    std::cout << "Implementation Derived1" << std::endl;
  }
};
        
int main(){
  
  std::cout << std::endl;
  Base<Derived1> d1;
  d1.interface();
  std::cout << std::endl;
}
getsoubl
  • 808
  • 10
  • 25

1 Answers1

5

There is no Derived that this new Derived * could point to, so it is definitely not safe: the cast itself has undefined behaviour, as described in [expr.static.cast]§11 (emphasis mine):

A prvalue of type “pointer to cv1 B”, where B is a class type, can be converted to a prvalue of type “pointer to cv2 D”, where D is a complete class derived from B, if cv2 is the same cv-qualification as, or greater cv-qualification than, cv1. [...] If the prvalue of type “pointer to cv1 B” points to a B that is actually a subobject of an object of type D, the resulting pointer points to the enclosing object of type D. Otherwise, the behavior is undefined.

You can mitigate this risk by restricting access to Base's constructor:

template <typename Derived>
struct Base{
    // Same as before...

protected:
    Base() = default;
};

This is better, but still allows for the same issue if someone were to accidentally define struct Derived2 : Base<AnotherDerived> { };. Preventing that can be done with a specific friend declaration, but has the downside of giving full access to Base's private members:

template <typename Derived>
struct Base{
    // Same as before...

private:
    friend Derived;
    Base() = default;
};

Note that this still lets Derived construct naked Base<Derived> objects in its member functions, but that's where I generally stop whacking moles.

Quentin
  • 62,093
  • 7
  • 131
  • 191
  • `Derived` is `Derived1`. This is the curiosity behind CRTP. Could you elaborate why you think it's UB? – Patrick Roberts Feb 09 '21 at 20:58
  • 1
    @PatrickRoberts I know, I'm actually quite fond of this pattern myself. However, OP's situation is that they instantiated a naked `Base`, *not* a `Derived1`. – Quentin Feb 09 '21 at 21:00
  • Thank you, I think my brain just needed a little kick and a minute to process the relationship between the classes. Now I'm conscious of another potential pitfall to avoid, (especially seeing as everything [_looks_](https://godbolt.org/z/hrWdoW) okay, at least until it isn't). – Patrick Roberts Feb 09 '21 at 21:05
  • 1
    @PatrickRoberts, I think that is UB because in runtime the type of d1 is Base.But inside the interface this is casted to a pointer of Derived1 class – getsoubl Feb 09 '21 at 21:06
  • If one should speculate, since `implementation()` doesn't try to access member variables in `Derived1` the vtable lookup _seems_ to work. Had it tried accessing member variables I think a crash (or some other odd behavior) would be more likely (like [this](https://godbolt.org/z/zvhzd4)). – Ted Lyngmo Feb 09 '21 at 21:06
  • 3
    Most of the time with CRTP the constructors should be `protected` unless it is abstract so only derived types can be constructed. In my experience, it is rare that you can construct an instance of a CRTP base type directly. – François Andrieux Feb 09 '21 at 21:08
  • 1
    @TedLyngmo Have a look in this [example](https://godbolt.org/z/azKdvb) . In this example a variable in Derived class is accessed. Using sanitizer , memory violation was detected – getsoubl Feb 09 '21 at 21:16
  • @getsoubl It seems clang++ and g++ react differently. I used clang++ in my example. Switching to g++ made the sanitizer react. – Ted Lyngmo Feb 09 '21 at 21:23
  • You might want to mention you should make the `Base` ctor/dtor protected to avoid this problem. – Yakk - Adam Nevraumont Feb 09 '21 at 21:27
  • @Yakk-AdamNevraumont good point, I did just that. – Quentin Feb 09 '21 at 21:36
  • Another prevention is to add a `static_assert(std::is_base_of_v);` in a method which should be instantiated (as destructor). – Jarod42 Feb 10 '21 at 08:28