Calling methods on C++ classes that are null

fcarney

Yes, this is a very bad idea. But I observed some interesting things about QObject and C++ classes in general.
If I call deleteLater on a null QObject pointer it does not crash. It spews errors in the console. So I was wondering what deleteLater was actually doing:
void QObject::deleteLater()
{
QCoreApplication::postEvent(this, new QDeferredDeleteEvent());
}

As long as the call doesn't access data members or other functions that do it seems to work fine. This may in fact be compiler dependent however. Calling methods on null objects is undefined from what I have read.

So I wondered can we detect if a method is being called on a null object and prevent a crash?

class NoCrashOnNull
{
public:
    NoCrashOnNull()
        : value(42)
    {

    }

    int someMethod(){
        qDebug() << this;
        auto ptr = this;
        qDebug() << ptr;
        if(ptr){
            qDebug() << this << "Pointer was null";
            return -1;
        }

        return value;
    }

protected:
    int value;
};

This will not work. The compiler optimizes out the logic branch because "this" is "always" not null. But I found if I call a method to check for null and return a boolean I can detect null and it will branch:

class NoCrashOnNull
{
public:
    NoCrashOnNull()
        : value(42)
    {

    }

    int someMethod(){
        qDebug() << this;
        auto ptr = this;
        qDebug() << ptr;
        if(checkNull(this)){
            qDebug() << this << "Pointer was null";
            return -1;
        }

        return value;
    }

    static bool checkNull(void* vptr){
        return vptr ? true : false;
    }
protected:
    int value;
};

This is gcc in Linux. It may not be the same story for other versions and other compilers. I just wanted to play a bit.

JonB

@fcarney
Till the compiler decides to inline/optimize/analyze your method call, see you're calling it with this, and puts you back in the first example situation :) But yes in practice (not "aggressive" optimization) passing a parameter to a function tends to do what you say. When debugging and having hassle from the optimizer such that I can't correctly view certain variables' values in Watch window etc., passing them to a dummy function seems to let you see their values correctly.

What I don't get is that when I was still C and a work colleague was C++ I distinctly remember him showing me that you could call a method on a null instance, and check for this == nullptr/0 and write code for that, even though it seemed strange. Has C++ always forbidden calling member methods on null instances, did it used to allow it or not enforce it, or what?

fcarney

@JonB What is weird is the qDebug() << this and qDebug << ptr both print 0x0. So I can view that this is null, but when it was making decisions based upon this and ptr it assumed it cannot be null and didn't branch. Somehow the function call sidesteps this. Yeah, I have not tried a higher level of optimization.

I think in the early C++ days the compilers were a lot less smart. So the scenario you describe was probably a thing at one time.

JonB

@fcarney
The thing about compilers optimizing away this == nullptr was mentioned several times in SO posts, so obviously a known optimization. I don't know what level of optimization/which compilers they were on though.

fcarney

Huh, my project for testing this has -O3 set: QMAKE_CXXFLAGS += -O3
I also tried in release and the function call wins out.

JonB

@fcarney
The forthcoming quantum version of gcc analyzes all possible paths in other universes and hence realizes this can't* [* 99.9999999999% certain] be null, and will optimize even your function call away....

SimonSchroeder

I guess that calling a member method on a nullptr will not crash when you are not accessing any members (and the member function is not virtual). This is probably the reason why deleteLater() does not crash. This supposes, however, that QDeferredDeleteEvent() either checks for nullptr or just calls delete without accessing any members before.

I guess you can just rewrite your checkNull to: static bool checkNull(void* vptr) { return vptr; }. This brings up the idea if if(checkNull(this)) ... can be replaced by if(static_cast<bool>(this)) ... to prevent the compiler from optimizing it away.

Fun fact: Did you know that it is legal to have delete this; as the last statement in a member function?

J.Hilk

@fcarney said in Calling methods on C++ classes that are null:

postEvent

the first thing postEvent does, is checking for nullptr, if it's null, it returns immediately.

        void QCoreApplication::postEvent(QObject *receiver, QEvent *event, int priority)
	{
	    Q_TRACE_SCOPE(QCoreApplication_postEvent, receiver, event, event->type());
	
	    if (receiver == 0) {
	        qWarning("QCoreApplication::postEvent: Unexpected null receiver");
	        delete event;
	        return;
	    }

ok I now get, what you're actually asking :D

I would actually say, that the compiler properly inlines this

void QObject::deleteLater()
{
    QCoreApplication::postEvent(this, new QDeferredDeleteEvent());
}

and therefore it's a pseudo static function call ? :D 🤷‍♂️

JonB

@SimonSchroeder said in Calling methods on C++ classes that are null:

I guess that calling a member method on a nullptr will not crash

Maybe, maybe not, but it is "undefined behaviour" according to C++ standard.

if(checkNull(this)) ... can be replaced by if(static_cast<bool>(this)) ... to prevent the compiler from optimizing it away.

How so? Since compiler knows/assumes this != nullptr it should know static_cast<bool>(this) == true, so why not same optimization as already observed?