CPP: Allocation and Deallocation libraries#2463
Conversation
…yExpr.getExtent().
…l to that made recently in NoSpaceForZeroTerminator.ql.
jbj
left a comment
jbj
left a comment
There was a problem hiding this comment.
Looks great! It would have been easier to review if more of the Library commits had been squashed together.
|
Added |
Co-Authored-By: Jonas Jensen <jbj@github.com>
|
qlformat failed |
|
Formatting fixed. |
jbj
left a comment
jbj
left a comment
There was a problem hiding this comment.
There's now a test failure and merge conflicts.
The merge conflicts should now be fixed. I'll have to wait for the tests to finish again before I can see why it failed. |
|
I've just fixed the failing test - the model needed to account for I've also modernized these two queries as it's something I'd been meaning to do and doing so makes the exception cleaner. (latest four commits are new and should be reviewed) |
| where | ||
| alloc.requiresDealloc() and | ||
| not exists(alloc.(NewOrNewArrayExpr).getPlacementPointer()) and | ||
| not allocMayBeFreed(alloc) |
There was a problem hiding this comment.
Shouldn't requiresDealloc be false for a placement-new expression? It's hard to use this library if the caller must know about the special cases of alloc that are possible.
There was a problem hiding this comment.
I considered that, but wasn't sure because we also don't know that a placement new expression doesn't require a dealloc either. I'd imagine it often does, but sometimes the entire pool is expected to be destroyed instead.
In the interest of avoiding false positives I think I'll make this change...
2 participants
There's quite a lot going on here, but the core of it is taking the functionality from
commons/Alloc.qll, cleaning it up a lot, and repackaging it asmodels/interfaces/Allocation.qllandmodels/interfaces/Deallocation.qll. This allows us to reason about allocations and their sizes consistently regardless of whether they aremalloc,realloc,new,new []etc.Alloc.qllis now a legacy wrapper for the new library.I've updated the NoSpaceForZeroTerminator.ql and OverflowCalculated.ql queries to use the new library, which improves their results since they were previously looking for calls to
malloconly.There is quite a lot of existing query test coverage, I have extended it in a few places.