Is calling pure virtual functions indirectly from a constructor always undefined behaviour?

Question

I'm working on building Cppcheck on AIX with the xlC compiler (see previous question). Checker classes all derive from a Check class, whose constructor registers each object in a global list:

check.h

class Check {
public:
    Check() {
        instances().push_back(this);
        instances().sort();
    }
    static std::list<Check *> &instances();
    virtual std::string name() const = 0;
private:
    bool operator<(const Check *other) const {
        return (name() < other->name());
    }
};

checkbufferoverrun.h

class CheckBufferOverrun: public Check {
public:
    // ...
    std::string name() const {
        return "Bounds checking";
    }
};

The problem I appear to be having is with the instances().sort() call. sort() will call Check::operator<() which calls Check::name() on each pointer in the static instances() list, but the Check instance that was just added to the list has not yet had its constructor fully run (because it's still inside Check::Check()). Therefore, it should be undefined behaviour to call ->name() on such a pointer before the CheckBufferOverrun constructor has completed.

Is this really undefined behaviour, or am I missing a subtlety here?

Note that I don't think the call to sort() is strictly required, but the effect is that Cppcheck runs all its checkers in a deterministic order. This only affects the output in the order in which errors are detected, which causes causes some test cases to fail because they're expecting the output in a particular order.

Update: The question as above still (mostly) stands. However, I think the real reason why the call to sort() in the constructor wasn't causing problems (ie. crashing by calling a pure virtual function) is that the Check::operator<(const Check *) is never actually called by sort()! Rather, sort() appears to compare the pointers instead. This happens in both g++ and xlC, indicating a problem with the Cppcheck code itself.

Answer 1

Yes, it's undefined. The standard specifically says so in 10.4/6

Member functions can be called from a constructor (or destructor) of an abstract class; the effect of making a virtual call (10.3) to a pure virtual function directly or indirectly for the object being created (or destroyed) from such a constructor (or destructor) is undefined.

Answer 2

It is true that calling a pure virtual function from a constructor is always an undefined behaviour.

The virtual pointer can not be assumed to be set until the constructor has run completely (closing "}"), and hence any call to a virtual function (or pure virtual function) has to be setup at the time of compilation itself (statically bound call).

Now, if the virtual function is pure virtual function, the compiler will generally insert its own implementation for such pure virtual function, the default behavior of which is to generate a segmentation fault. The Standard does not dictate what should be the implementation of a pure virtual function, but most of C++ compilers adopt aforesaid style.

If your code is not causing any runtime mischief demeanour, then it is not getting called in the said call sequence. If you could post the implementation code for below 2 functions

instances().push_back(this);
instances().sort();

then maybe it will help to see what's going on.

Answer 3

As long as object construction isn't finished, a pure virtual function may not be called. However, if it's declared pure virtual in a base class A, then defined in B (derived from A), the constructor of C (derived from B) may call it, since B's construction is complete.

In your case, use a static constructor instead:

class check {
private Check () { ... }
public:
    static Check* createInstance() {
        Check* check = new Check();
        instances().push_back(check);
        instances().sort();
    }
...
}

Answer 4

I think your real problem is that you've conflated two things: the Checker base class, and some mechanism for registering (derived) instances of Check.

Among other things, this isn't particularly robust: I may want to use your Checker classes, but I may want to register them differently.

Maybe you could do something like this: Checker get a protected ctor (it's abstract anyway, and so only derived classes ought to be calling the Checker ctor).

Derived classes also have protected ctors, and a public static method (the "named constructor pattern") to create instances. That creating method news up a Checker subclass, and them passes it (fully created at this point) to a CheckerRegister class (which is also abstract, so users can implemented their own if need be).

You use whatever singleton pattern, or dependency injection mechanism, that you prefer, to instantiate a Checkerregister and make it available to Checker subclasses.

One simple way to do this would be to have a getCheckerRegister static method on Checker.

So a Checker subclass might look like this:

class CheckBufferOverrun: public Check { protected: CheckBufferOverrun : Check("Bounds checking") { // since every derived has a name, why not just pass it as an arg? } public: CheckBufferOverrun makeCheckBufferOverrun() { CheckBufferOverrun that = new CheckBufferOverrun();

   // get the singleton, pass it something fully constructed
   Checker.getCheckerRegister.register(that) ;
   return that;
}

If it looks like this will end up being a lot of boilerplate code, write a template. If you worry that because each template instance in C++ is a real and unique class, write a non-templated base class that will register any Checker-derived.