X Tutup
The Wayback Machine - https://web.archive.org/web/20240324153523/https://github.com/github/codeql/issues/10568
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CPP: Some guard questions about control #10568

Open
icy17 opened this issue Sep 24, 2022 · 4 comments
Open

CPP: Some guard questions about control #10568

icy17 opened this issue Sep 24, 2022 · 4 comments
Labels
question Further information is requested

Comments

@icy17
Copy link

icy17 commented Sep 24, 2022

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 icy17 added the question Further information is requested label Sep 24, 2022
@icy17 icy17 changed the title CPP: Some guard questions CPP: Some guard questions about control Sep 24, 2022
@icy17
Copy link
Author

icy17 commented Sep 24, 2022

...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....

@MathiasVP
Copy link
Contributor

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
Copy link
Contributor

...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)

?

@icy17
Copy link
Author

icy17 commented Oct 10, 2022

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants
X Tutup