Downcast to derived class in CRTP base class constructor: UB or not?

Question

Consider the following classes:

template <class Derived>
class BaseCRTP {
  private:
    friend class LinkedList<Derived>;
    Derived *next = nullptr;

  public:
    static LinkedList<Derived> instances;

    BaseCRTP() {
        instances.insert(static_cast<Derived *>(this));
    }
    virtual ~BaseCRTP() {
        instances.remove(static_cast<Derived *>(this));
    }
};

struct Derived : BaseCRTP<Derived> {
    int i;
    Derived(int i) : i(i) {}
};

int main() {
    Derived d[] = {1, 2, 3, 4};
    for (const Derived &el : Derived::instances) 
        std::cout << el.i << std::endl;
}

I know that it is undefined behavior to access the members of Derived in the BaseCRTP<Derived> constructor (or destructor), since the Derived constructor is executed after the BaseCRTP<Derived> constructor (and the other way around for the destructors).

My question is: is it undefined behavior to cast the this pointer to Derived * to store it in the linked list, without accessing any of Derived's members?

LinkedList::insert only accesses BaseCRTP::next.

When using -fsanitize=undefined, I do get a runtime error for the static_casts, but I don't know if it's valid or not:


    instances.insert(static_cast<Derived *>(this));

crt-downcast.cpp:14:26: runtime error: downcast of address 0x7ffe03417970 which does not point to an object of type 'Derived'
0x7ffe03417970: note: object is of type 'BaseCRTP<Derived>'
 82 7f 00 00  00 2d 93 29 f3 55 00 00  00 00 00 00 00 00 00 00  e8 7a 41 03 fe 7f 00 00  01 00 00 00
              ^~~~~~~~~~~~~~~~~~~~~~~
              vptr for 'BaseCRTP<Derived>'
4
3
2
1

    instances.remove(static_cast<Derived *>(this));

crt-downcast.cpp:17:26: runtime error: downcast of address 0x7ffe034179b8 which does not point to an object of type 'Derived'
0x7ffe034179b8: note: object is of type 'BaseCRTP<Derived>'
 fe 7f 00 00  00 2d 93 29 f3 55 00 00  a0 79 41 03 fe 7f 00 00  04 00 00 00 f3 55 00 00  08 c0 eb 51
              ^~~~~~~~~~~~~~~~~~~~~~~
              vptr for 'BaseCRTP<Derived>'

Additionally, here's a simplified version of the LinkedList class:

template <class Node>
class LinkedList {
  private:
    Node *first = nullptr;

  public:
    void insert(Node *node) {
        node->next = this->first;
        this->first = node;
    }

    void remove(Node *node) {
        for (Node **it = &first; *it != nullptr; it = &(*it)->next) {
            if (*it == node) {
                *it = node->next;
                break;
            }
        }
    }
}

Doesn't your `LinkedList` access members of `Derived` (e.g.: by using a copy or move constructor)? — UnholySheep, Apr 06 '20 at 13:38
@UnholySheep no, it just saves a pointer, it doesn't copy or access anything else apart from `BaseCRTP::next`. — tttapa, Apr 06 '20 at 13:39
According to CWG1517, `Derived` is not under construction until its base class constructor finishes, but... how does it affect that one can't `static_cast` to it? — Language Lawyer, Nov 28 '20 at 18:35
Could you please add your "real" Linked list. It misses `;` at the end and it does not support `begin()` / `end()` — Bernd, Nov 28 '20 at 20:00
@LanguageLawyer, it does not affect the static_cast but if you compile with -fsanitize=undefined then only it will throw a runtime error. — rahul.deshmukhpatil, Nov 28 '20 at 21:41

Bernd · Answer 1 · 2020-11-28T21:01:42.647

1

Up to now I didn't found a rule which says it is or is not UB, but I can explain why you see the observed behaviour.

In the C++ standard we have the following rule:

Member functions, including virtual functions (11.7.2), can be called during construction or destruction (11.10.2). When a virtual function is called directly or indirectly from a constructor or from a destructor, including during the construction or destruction of the class’s non-static data members, and the object to which the call applies is the object (call it x) under construction or destruction, the function called is the final overrider in the constructor’s or destructor’s class and not one overriding it in a more-derived class.

Which basically says that calls to virtuals functions are allowed during construction. But those calls must not resolved to more derived classes.

This is achieved in clang by simply exchanging the VMT Pointer during construction.

This can be observed here: https://godbolt.org/z/3Gebvv (Or simply by analizing the generated assembly)

-fsanitize=undefined adds now some logic to find bugs / undefined behaviour. As an example they check if such a cast is valid. To do so they use the VMT which get's changed by the compiler.

In a non-constructor context an inappropriate vmt-pointer is a very good indication for undefined behaviour. But here could it be a bug in the sanitizer, too.

edited Nov 28 '20 at 21:01

answered Nov 28 '20 at 20:44

Bernd

2,113
8
22

I strongly suspect that it is happening because the implementation of the fsanitize is trying to act smartly. instead of doing static_cast, it's trying to perform sanitize check the way a dynamic cast would have done and since casting happening in constructor vptr is pointing to VTable with type info of the Base. If you remove the virtual method, this static_cast does not cause problem with the -fsanitize=undefined. Simple example https://godbolt.org/z/c4rvMK – rahul.deshmukhpatil Nov 28 '20 at 22:11
1

Yes, fsanitize uses the runtime type info provided via VMT to validate the cast. Without a VMT there is simply no runtime type info and fsanitize can't check to much... – Bernd Nov 28 '20 at 22:23
fsanitize is not really "smart", but it does a "better" check for polymorphic types - which causes the issue here. – Bernd Nov 28 '20 at 22:31
Thank you for this answer, even though it doesn't quite explain if the code is UB or not. I wonder what you think about this: I claim that downcasting with `static_cast` is just an unchecked (unsafe) version of `dynamic_cast`. In this code, `dynamic_cast` would return null because the derived class is not constructed yet. If you believe the logic that a valid `static_cast` should also be a successful `dynamic_cast`, then the `static_cast` here is not valid. Do you agree with this interpretation? In any case I'm still hoping for a definitive answer about UB or not. – John Zwinck Nov 29 '20 at 01:25
@JohnZwinck, I agree with the interpretation that ```static_cast``` used for downcasting is an invalid and unchecked(unsafe) version of ```dynamic_cast```. I think this is happening just because of the implementation of the ```-fsanitize``` check. It catches the invalid downcasting via ```static_cast``` correctly in functions other than the polymorphic base class constructor because of obvious use of the ```typeinfo``` via current class vptr. And ```-fsanitize``` should specialize handling of downcasting via ```static_cast``` in polymorphic base class. – rahul.deshmukhpatil Nov 29 '20 at 08:18

Downcast to derived class in CRTP base class constructor: UB or not?

1 Answers1