CPP: Fix function pointer/lambda related false positives in 'Resource not released in destructor'#672
Conversation
|
This is going to conflict with #580, when that PR reaches master. I'm happy to fix that when it comes up. |
…s used to be called the 'release' or 'releaseName').
… inside lambda expression.
…arget of a function pointer.
jbj
left a comment
jbj
left a comment
There was a problem hiding this comment.
This appears to be a strict improvement. LGTM except for one comment.
| releaseExpr = r.getAReleaseExpr(kind) and | ||
| releaseExpr.getEnclosingFunction().getEnclosingAccessHolder*() = acquire.getEnclosingFunction() | ||
| // here, `getEnclosingAccessHolder*` allows us to go from a nested function or lambda | ||
| // expression to the class method enclosing it. |
There was a problem hiding this comment.
It seems a bit coincidental that getEnclosingAccessHolder does what you want here. I'm not even sure that predicate should be public. Would getEnclosingElement not work?
There was a problem hiding this comment.
Yes getEnclosingElement works - but we pay a substantial performance cost (~20%), according to my tests.
There was a problem hiding this comment.
I don't see why that would be more expensive. As getEnclosingElement is cached, it's essentially free to use in a query that's part of a suite.
The join you have there is of course risky since it's of the form restrictedExpr1.getEnclosingSomething() = restrictedExpr2.getEnclosingSomething(), and there's never a good way to join-order that. The slightest change can affect whether you get the magic and join order you need.
There was a problem hiding this comment.
You're persuading me... I'll try testing again in the context of a warmed-up cache.
|
@geoffw0 let's try to get this in before the next dist upgrade |
jbj
left a comment
jbj
left a comment
There was a problem hiding this comment.
I'll merge this to make sure it gets into today's dist upgrade.
Post-release preparation for codeql-cli-2.7.6
2 participants
Add some tests and fix several types of false positives in this query related to function pointers and lambdas. I've also renamed some things in the query as the terminology used had become inconsistent.