UB: QirTupleHeader::Release()

[kuzminrobin]

The instruction return this->refCount; can access the dangling pointer.

int QirTupleHeader::Release()
{
    . . .
    if (this->refCount == 0)
    {
        char* buffer = reinterpret_cast<char*>(this);
        delete[] buffer;   // Here the data pointed to by `this` gets deallocated. The `this` becomes a dangling pointer.
    }
    return this->refCount;   // This has a risk of accessing the dangling pointer. Undefined Behavior.
}

Consider changing to

int QirTupleHeader::Release()
{
    . . .
    int32_t retVal = this->refCount;   // Memorize in the local variable the `refCount`.
    if (this->refCount == 0)
    {
        char* buffer = reinterpret_cast<char*>(this);
        delete[] buffer;   // Here the data pointed to by `this` is deallocated. The `this` becomes a dangling pointer.
    }
    return retVal;   // Access the local variable.
}

Consider also syncing the data type returned by int QirTupleHeader::Release() and type of this->refCount - int32_t.

[bettinaheim]

@kuzminrobin Was there a specific reason you didn't make that change yet, or was it merely to split it out into its own PR and you haven't gotten around to it yet?

[kuzminrobin]

I opened this bug and forgot about that ;-) I apologize. I also didn't know the priority of this.
I'll be more proactive and resolve such bugs together with the current work to minimize the review cycle.
Assigned to myself and added to my plan.