C++ Initializer Inconsistencies

[bdrodes]

Below is a simple C++ file illustrating two uses of codeql Initializer .

#include <iostream>
#include <cstdlib>

using namespace std;
int main()
{
    char* test1 = "hello world";
    if(char* test2 = (char*)malloc(10))
    {
        cout<<"test"<<endl;
    }
}

This example illustrates a few inconsistencies in how Initializer logic functions.

The first issue is that the initializer expression does not appear to be a child of the if condition expression. The following query will yield no result.

/**
 * @kind problem
 */
import cpp

from Initializer i, IfStmt ifs
where ifs.getControllingExpr().getAChild*() = i.getExpr()
select i, "TEST"

The second issue, which is more minor and potentially not an issue is the query below will find that test2 is an access of itself, whereas test1 is not an access. This may be expected however, since test2 is first assigned, then result evaluated by the if conditional. I'm not entirely sure if that's what is going on here though.

/**
 * @kind problem
 */
import cpp

from Variable v, Expr e
where 
e = v.getAnAccess()
select e, "TEST"

[jketema]

Thanks for your question. I think the behaviour is all as expected. Going backwards through your issues:

The second issue, which is more minor and potentially not an issue is the query below will find that test2 is an access of itself, whereas test1 is not an access

Correct. This is on purpose. In the case of test1, we only have a definition. However, in the case of test2, two things need to happen when we would execute the code: (1) test2 is defined, (2) the controlling expression is evaluated, which uses test2, i.e., there's a variable access. These two things are both modelled.

The first issue is that the initializer expression does not appear to be a child of the if condition expression.

In light of my answer above, this makes some sense in this case, as the initialisation is part of the definition of test2 and not of its use. So, it in some way falls outside of the controlling expression. Note that this behaviour is also consistent with the behaviour of DeclStmts, which also doesn't have its initializer as a child. I do agree that this behaviour might be somewhat unexpected.

What I would do to get the initialisers out in this case is the following:

from IfStmt ifs, ConditionDeclExpr cde, Initializer i
where ifs.getControllingExpr().getAChild*() = cde and cde.getVariable().getInitializer() = i
select i

Or, if you're interested in the initialising expression:

from IfStmt ifs, ConditionDeclExpr cde, Expr ie
where ifs.getControllingExpr().getAChild*() = cde and cde.getInitializingExpr() = ie
select ie

Of course, we could always have a look at changing this behaviour if there are really interesting use cases for this.

[bdrodes]

@jketema Thanks for the suggestion. I had not used ConditionDeclExpr, I'll need to look into that for my current workaround.

That said, it still strikes me as odd that the expression for the initialization is not within the conditional expression. It seems error prone to me unless you are really looking for it (as I was). Since this is a case where what I would naively say is the "more intuitive approach" yields no results, it appears to me that it would be a common case for false negatives. I'm not sure if that is sufficient enough of an argument to motivate a change though.