codeql icon indicating copy to clipboard operation
codeql copied to clipboard

CPP: Some guard questions about control

Open icy17 opened this issue 3 years ago • 3 comments

Hi! I want to detect if there is a check for NULL return value of a function.And I want to get the block which is controlled by ret == NULL So Do I have to consider the following four cases?

if(ret == NULL)     //comparesEq(ret, null, 0, true, true))
if(ret != NULL)      //comparesEq(ret, null, 0, false, true)
if(ret)                    // I don't know yet how to write this
if(!ret)                   //I don't know yet how to write this

Or Is there a method that includes the above four cases?

icy17 avatar Sep 24 '22 10:09 icy17

...and now I check if a basicblock is controlled by ret == NULL in two cases:

if(ret == NULL) 
if(ret != NULL)

by code like that:

 gc.controls(leakbb, true)
  and null.getValue().toInt() = 0
  and null = gc.getAChild*()
gc.comparesEq(ret, null, 0, true, true)

and

null.getValue().toInt() = 0
and null = gc.getAChild*()
and gc.comparesEq(ret, null, 0, false, true)
and not gc.controls(leakbb, true)

Is that right? I run this query for a long time....

icy17 avatar Sep 24 '22 11:09 icy17

Regarding your first question:

So Do I have to consider the following four cases?

if(ret == NULL)     //comparesEq(ret, null, 0, true, true))
if(ret != NULL)      //comparesEq(ret, null, 0, false, true)
if(ret)                    // I don't know yet how to write this
if(!ret)                   //I don't know yet how to write this

Or Is there a method that includes the above four cases?

I think the answer is 'no'. As you've discovered yourself, comparesEq only handles the first two cases. We can't easily add support for those last two cases with the current comparesEq interface since there's no null expression in the database to compare against.

So to handle the third and fourth case, the GuardCondition will be the top-level expression of a conditional statement. The global value numbering library is often useful in this scenario. There's a predicate in a query here that handles all 4 cases, and I think you can take inspiration from that.

MathiasVP avatar Sep 28 '22 15:09 MathiasVP

...and now I check if a basicblock is controlled by ret == NULL in two cases:

if(ret == NULL) 
if(ret != NULL)

by code like that:

 gc.controls(leakbb, true)
  and null.getValue().toInt() = 0
  and null = gc.getAChild*()
gc.comparesEq(ret, null, 0, true, true)

and

null.getValue().toInt() = 0
and null = gc.getAChild*()
and gc.comparesEq(ret, null, 0, false, true)
and not gc.controls(leakbb, true)

Is that right? I run this query for a long time....

A couple of comments:

  • Depending on what the type of null is you might not need the null.getValue().toInt() = 0 check. For instance, if null is a value of type NullValue then that condition is unnecessary.
  • For the last snippet (the one containing not gc.controls(leakbb, true)), that might not do what you think it's doing. In particular, it's not restricting leakbb, so there's a chance that you're predicate is including too many tuples. Maybe you wanted to say that the guard condition controls the 'false' branch instead? That is, maybe you wanted:
// ...
and gc.controls(leakbb, false)

?

MathiasVP avatar Sep 28 '22 15:09 MathiasVP

@MathiasVP Thank you for your answer! But if I use gc.controls(leakbb, false), it can't detect that bb:

if (ret != NULL)
{
...
}
....     //in that bb, ret maybe null, but that not controlled by gc

And, I found if I use GuardCondition and globalValueNumber, it costs long time to run query. Is there any way to reduce the query time? Or Is there any documentation on how to reduce query time?I've been looking for ways to shorten the time.

icy17 avatar Oct 10 '22 08:10 icy17