C++: PostUpdateNodes for const-pointer arguments
#11743
Conversation
PostUpdateNodes for pointer-to-const argumentsPostUpdateNodes for <s>pointer-to-const</s>const-pointer arguments
dec0d85 to
76cd5cc
Compare
PostUpdateNodes for <s>pointer-to-const</s>const-pointer argumentsPostUpdateNodes for const-pointer arguments
|
Edit: Fixed! |
76cd5cc to
3571341
Compare
…guments of pointer to const-type) has an outgoing argument node.
3571341 to
cc03716
Compare
jketema
left a comment
There was a problem hiding this comment.
Mostly looks plausible. One question.
cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll
Outdated
Show resolved
Hide resolved
| exists(PointerWrapper pw, Type t | | ||
| cppType.hasType(t, _) and | ||
| t.stripType() = pw and | ||
| not pw.pointsToConst() |
There was a problem hiding this comment.
Is there a reason we do not have to drill into the type being wrapped here?
There was a problem hiding this comment.
Nah. This was really just me trying to minimize the amount of code I touched in a single PR 😂. I think a similar careful type inspection could be done for smart pointers (which is mostly what a PointerWrapper type is).
There was a problem hiding this comment.
Should we keep track of this somehow?
There was a problem hiding this comment.
I mean: issue, comment, ...
There was a problem hiding this comment.
Yeah, good point. I'll create an issue for it 👍.
…mmon.qll Co-authored-by: Jeroen Ketema <93738568+jketema@users.noreply.github.com>
|
I'm merging this since the CI failures aren't caused by this PR (it seems to happen on all PRs currently 😭). |
2aace0d
into
github:mathiasvp/replace-ast-with-ir-use-usedataflow
That's fine, It was ok before, and we only changed one comment. |
Previously, we failed to identify that flow can come out of functions with a signature like readv. This PR fixes this.
Slightly more generally, this PR fixes problems related to getting flow out of a function like:
since it's perfectly possible for
footo write to*p.Luckily, everything in dataflow was already setup to handle the flow. We just didn't didn't add dataflow nodes representing the outgoing flow for such arguments.
There are a lot of new results (and no lost results 🎉!). I've checked all the SAMATE once, and they all seem to be in tests that we're expected to flag. Similarly, the results on real code also look like things the queries are expected to find (but which we didn't find previously because we had assumed flow couldn't exit through some
constparameter).