CodeQL queries 1.23

Skip to end of metadata
Go to start of metadata

Name: Virtual call from constructor or destructor

Description: Virtual functions should not be invoked from a constructor or destructor of the same class. Confusingly, virtual functions are resolved statically (not dynamically) in constructors and destructors for the same class. The call should be made explicitly static by qualifying it using the scope resolution operator.

ID: cpp/virtual-call-in-constructor

Kind: problem

Severity: warning

Precision: high

Query: AV Rule 71.1.ql
/**
 * @name Virtual call from constructor or destructor
 * @description Virtual functions should not be invoked from a constructor or destructor of the same class. Confusingly, virtual functions are resolved statically (not dynamically) in constructors and destructors for the same class. The call should be made explicitly static by qualifying it using the scope resolution operator.
 * @kind problem
 * @problem.severity warning
 * @precision high
 * @id cpp/virtual-call-in-constructor
 * @tags reliability
 *       readability
 *       language-features
 *       external/jsf
 */

import cpp

predicate thisCall(FunctionCall c) {
  c.getQualifier() instanceof ThisExpr or
  c.getQualifier().(PointerDereferenceExpr).getChild(0) instanceof ThisExpr
}

predicate virtualThisCall(FunctionCall c, Function overridingFunction) {
  c.isVirtual() and
  thisCall(c) and
  overridingFunction = c.getTarget().(VirtualFunction).getAnOverridingFunction()
}

/*
 * Catch most cases: go into functions in the same class, but only catch direct
 * references to "this".
 */

predicate nonVirtualMember(MemberFunction mf, Class c) {
  mf = c.getAMemberFunction() and
  not mf instanceof Constructor and
  not mf instanceof Destructor and
  not mf.isVirtual()
}

predicate callFromNonVirtual(MemberFunction source, Class c, MemberFunction targ) {
  exists(FunctionCall fc |
    fc.getEnclosingFunction() = source and fc.getTarget() = targ and thisCall(fc)
  ) and
  targ = c.getAMemberFunction() and
  nonVirtualMember(source, c)
}

pragma[noopt]
predicate indirectlyCallsVirtualFunction(MemberFunction caller, Function target, Class c) {
  exists(FunctionCall fc |
    virtualThisCall(fc, _) and
    fc.getEnclosingFunction() = caller and
    fc.getTarget() = target and
    nonVirtualMember(caller, c)
  )
  or
  exists(MemberFunction mid |
    indirectlyCallsVirtualFunction(mid, target, c) and
    callFromNonVirtual(caller, c, mid)
  )
}

from FunctionCall call, string explanation, Function virtFunction, Function overridingFunction
where
  (
    call.getEnclosingFunction() instanceof Constructor or
    call.getEnclosingFunction() instanceof Destructor
  ) and
  (
    (
      virtualThisCall(call, overridingFunction) and
      explanation = "Call to virtual function $@ which is overridden in $@. If you intend to statically call this virtual function, it should be qualified with "
          + virtFunction.getDeclaringType().toString() + "::."
    ) and
    virtFunction = call.getTarget() and
    overridingFunction.getDeclaringType().getABaseClass+() = call
          .getEnclosingFunction()
          .getDeclaringType()
    or
    exists(VirtualFunction target |
      thisCall(call) and indirectlyCallsVirtualFunction(call.getTarget(), target, _)
    |
      explanation = "Call to function " + call.getTarget().getName() +
          " that calls virtual function $@ (overridden in $@)." and
      virtFunction = target and
      overridingFunction = target.getAnOverridingFunction() and
      overridingFunction.getDeclaringType().getABaseClass+() = call
            .getEnclosingFunction()
            .getDeclaringType()
    )
  )
select call, explanation, virtFunction, virtFunction.getName(), overridingFunction,
  overridingFunction.getDeclaringType().getName()

This rule finds calls to virtual functions from a constructor or destructor that may resolve to a different function than was intended. When instantiating a derived class, the resolution of a virtual function call depends on the type that defines the constructor/destructor that is currently running, not the class that is being instantiated. This is to prevent the calling of functions in the derived class that rely on fields declared in the derived class. The values of such fields are undefined until the constructor of the derived class is invoked after the constructor of the base class. Values declared in the derived class are likewise destructed prior to invocation of the destructor of the base class.

The indicated function call is a call to a virtual function in a constructor or destructor, which will most likely not call the intended function, or if correct would be difficult to interpret without knowledge of the class' inheritance graph.

Recommendation

Do not call virtual functions from the constructor or destructor. Change the virtual function in the base into a non-virtual function and pass any required parameters from the derived classes, or simply perform initialization that requires a virtual function after construction/before destruction.

Example

class Base {
protected:
    Resource* resource;
public:
    virtual void init() {
        resource = createResource();
    }
    virtual void release() {
        freeResource(resource);
    }
};

class Derived: public Base {
    virtual void init() {
        resource = createResourceV2();
    }
    virtual void release() {
        freeResourceV2(resource);
    }
};

Base::Base() {
    this->init();
}
Base::~Base() {
    this->release();
}

int f() {
    // this will call Base::Base() and then Derived::Derived(), but this->init()
    // inBase::Base() will resolve to Base::init(), not Derived::init()
    // The reason for this is that when Base::Base is called, the object being
    // created is still of type Base (including the vtable)
    Derived* d = new Derived();
}

References