Add UninitializedNode::getIndirectionIndex/0#21458
Add UninitializedNode::getIndirectionIndex/0#21458jeongsoolee09 wants to merge 3 commits intomainfrom
UninitializedNode::getIndirectionIndex/0#21458Conversation
There was a problem hiding this comment.
Pull request overview
Adds an indirection-index dimension to Public::UninitializedNode so queries can distinguish uninitialized values at different indirection levels (e.g., arrays-of-arrays).
Changes:
- Extend
UninitializedNodeto trackindirectionIndexrather than hard-coding index0. - Expose the indirection level via
UninitializedNode::getIndirectionIndex/0.
Comments suppressed due to low confidence (1)
cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowNodes.qll:785
- The doc comment for
getIndirectionIndex()uses different terminology (“level of indirection to get to this node”) than the rest of this file (which consistently says “indirection index”). For consistency and clarity, consider rephrasing to “Gets the indirection index of this node” and (optionally) define what 0/1/... correspond to.
/** Gets the level of indirection to get to this node. */
int getIndirectionIndex() { result = indirectionIndex }
| @@ -767,10 +767,11 @@ module Public { | |||
| */ | |||
| class UninitializedNode extends Node { | |||
| LocalVariable v; | |||
| int indirectionIndex; | |||
There was a problem hiding this comment.
The class-level doc comment for UninitializedNode now understates the behavior: the implementation no longer represents only indirection level 0. Please update the doc comment to mention that the node can represent different indirection indices (and briefly what 0/1/... mean) so query authors don’t misinterpret it.
This issue also appears on line 784 of the same file.
There was a problem hiding this comment.
Copilot is right here. The changes here are actually a semantically breaking change (which would need a full deprecation cycle).
Instead of doing what you have here, I suggest you do what I hinted at on Slack:
- Keep the
UninitializedNodeas it currently is onmain - Create a new class
IndirectUninitializedNodewhich basically the the charpred you have here (andindirectionIndex > 1to avoid overlap withUninitializedNode) - Add a new predicate
LocalVariable asIndirectUninitializedNode(int indirectionIndex)onDataFlow::Nodeand an convenience predicate with 0 parametersLocalVariable asIndirectUninitializedNode()implemented asresult = this.asIndirectUninitializedNode(_).
This avoids a breaking change to the UninitializedNode class and the asUninitializedNode predicate.
This reverts commit 6c792e6.
…Variable` This way the changes do not alter the meaning of `UninitializedNode`. In the meantime, the code still provides a specialized `Node` type `IndirectUninitializedNode` to access the nodes behind levels of indirection.
Add a member predicate
getIndirectionIndextoUninitializedNode. The existing node only took indirection level 0 to account, clouding the existence of uninitialized nodes behind any level of indirection.For example, consider this code:
Previously,
UninitializedNodeonly capturedarray, in its uninitialized state, of typeint[2][3]. However, it is also uninitialized at one level down, being of typeint[3].any(UninitializedNode node | node.getIndirectionIndex() = 1 | node)will now get theint[3]of the above.