c++ CRTP stack corruption

Question

when running the following code on MSVC 2013, x64 Debug config, it will show up a message box with this famous error message when quitting the main() function

"Run-Time Check Failure #2 - Stack around the variable 'tmp' was corrupted.". 

The question I can't answer is: Why?

note that no error message will occur when running on Release config. (why?)

disclaimer: this is just a sample code, meaning that I'm trying to use this same design on other classes (one base and several derived) with much more methods and template arguments and with a much more complex data type than the basic int*.

#include <iostream>

template <class T>
class base {
public:
    base() {
        static_cast<T*>(this)->setData();
    }
    ~base() {
        static_cast<T*>(this)->destroyData();
    }

    void print() {
        static_cast<T*>(this)->print_int();
    }
};

---

class derived : public base<derived> {
public:

    void setData() {
        x = new int();
    }

    void destroyData() {
        delete x;
        x = nullptr;
    }

    void print_int() {
        std::cout << "x = " << *x << std::endl;
    }

private:

    derived() {}
    derived(const derived& other) {}
    inline derived& operator= (derived copy) {}

    int *x;
};

---

int main() {
    base<derived> tmp;

    tmp.print();

    return 0;
}

EDIT: @Yakk if I understand correctly, you propose as solution this code:

#include <iostream>

template <class T>
class mix_in : public T
{
public:
    mix_in() { (this)->setData(); }
    ~mix_in() { (this)->destroyData(); }
    void print() { (this)->print_int(); }
};

---

class foo
{
public:

    void setData()
    {
        x = new int();
    }

    void destroyData()
    {
        delete x;
        x = nullptr;
    }

    void print_int()
    {
        std::cout << "x = " << *x << std::endl;
    }

    foo() {}

private:

    int *x;
};

---

int main()
{
    mix_in<foo> tmp;

    tmp.print();

    return 0;
}

It works just fine, thank you. I'll probably use this pattern since it seems that there's no solution to use CRTP for what I'm trying to do.

But I still would like to understand why the stack corruption happens. In spite of all the discussion around use or not use CRTP for all things, I'd like to understand very precisely why it happens.

Thank you again.

Answer 1

tmp is a base<derived>, not a derived. The point of CRTP is that the base class "knows" the object's actual type because it's being passed as the template parameter, but if you manually create a base<derived>, then the base class functions would think the object is a derived, but it isn't - it's just a base<derived>. Weird things (undefined behavior) happen as a result. (As noted in the other answer, you are also printing out something without setting it...)

As to your second question, the checks generated by MSVC to detect those sorts of programmer errors appears to be turned off in Release mode, presumably for performance reasons.

Answer 2

template <class T> class mix_in:public T {
  public:
    base() { (this)->setData(); }
    ~base() { (this)->destroyData(); }
    void print() { (this)->print_int(); }
};

Rename derived to foo. Do not inherit from mix_in or base in foo.

Swap base for mix_in in main.

CRTP is not the solution to all problems.

Answer 3

You say in comments,

the derived class will never be instantiated, its constructor is never called

Others have already pointed out the undefined use of an unconstructed object. To be specific, consider that int *x is a derived member, and your base<derived> usage generates calls, in sequence, to derived::set_data(), derived::print_int(), and on exit derived::destroy_data(), all member functions of an object that doesn't exist, referring to a member for which you've never allocated space¹.

But I think what you're after is wholly legitimate. Factoring code is the whole point of templates and inheritance, i.e. to make important code easier to even identify let alone comprehend, and any later refactoring easier to do, by abstracting out boilerplate.

So:

template < class T >
struct preconstruction_access_subset {};  // often left empty

template < class T, class S = preconstruction_access_subset<T> >
struct base : S {
        base() { S::setData(); }
       ~base() { S::destroyData(); }
        void print() { S::print_int(); }
};

// ... later ...

#include <iostream>

struct derived;
template<> struct preconstruction_access_subset<derived> {
        // everything structors in `base<derived>` need
        int *x;
        void setData()     { x = new int; }
        void destroyData() { delete x; }
        void print_int()   { std::cout << x << '\n'; }
};
struct derived : base<derived> {
        // everything no structors in any base class need to access
};

int main() {
        base<derived> tmp;
        tmp.print();
}

In this example print_int() isn't actually accessed during construction, so you could drop it back into the derived class and access it from `base as for "normal" CRTP, it's just a matter of what's clearest and best for your actual code.

---

^{1 That isn't the only problem, just the only one that's apparent without considering any reliance the compiler itself might need to place on the construction sequence. Think independent-name binding and potential inlining and the ways object identity morphs during {de,con}struction and compiler code factoring for all the potential template/inheritance/member-function/etc. combinations. Don't just hoist the int *x; to solve the current symptom, that would just kick the trouble farther down the road even if yours doesn't get that far yet.}

Answer 4

As for your code shown in main():

int main() {
    base<derived> tmp;

    tmp.print();

    return 0;
}

base<derived> tmp; that's wrong usage of this pattern, you wanted derived tmp;.

The point of the CRTP is that the derived class is injected to the base class and provides implementations for certain features that are resolved at compile time.
The base class is inherently intended, to be instantiated exclusively via inheritance. Thus you should ensure not to have any instantiations outside an inheritance context.

To avoid users of your implementation trapped by this misconception, make base's constructor(s) protected:

template <class T>
class base {
public:
    ~base() {
        static_cast<T*>(this)->destroyData();
    }

// ...
protected:
    T* thisPtr;
    base() : thisPtr(static_cast<T*>(this)) {
    }
};

NOTE: The base class shouldn't call any methods dependent on the fully initialized derived class methods from within your base class constructor though, as you have shown with your sample (static_cast<T*>(this)->setData();)!

You may store the reference to T* for later use though, as shown in the above sample.

---

class derived : public base<derived> {
public:
    derived() {
        setData();
    }

    void destroyData() {
        delete x;
        x = nullptr;
    }

    void print_int() {
        std::cout << "x = " << *x << std::endl;
    }

private: // ????
   derived(const derived& other) {} 
   inline derived& operator= (derived copy) {}

   int *x;
};

Also it's a bit unclear, why you want to hide copy constructor and assignment operator for your class?