Skip to content

Conversation

@MathiasVP
Copy link
Contributor

@MathiasVP MathiasVP commented Jan 23, 2026

For a dataflow node n one can ask if n is the node corresponding to a StoreInstruction. This is useful when one wants to deal uniformly with all kinds of assignments/initializers/increments/decrements/etc.

The predicate also tells you whether an assignment overwrites an entire buffer or whether it may be a partial write (i.e., node.asDefinition(true) means the write may not overwrite the entire buffer). In order to do that it uses some of the SSA predicates (in particular, SsaImpl::defToNode) to find the corresponding Ssa::Definition belonging to the StoreInstruction.

However, if there's no corresponding SSA information for the StoreInstruction (for instance, if it's a store to a field or to a variable that is not live) then SsaImpl::defToNode won't hold. And so the entire node.asDefinition predicate did not have a result.

This PR fixes this so that, instead of the predicate not having a result, it will have a result with uncertain = true. This means that node.asDefinition can now be used to grab all assignments/initializers/increments/decrements/etc regardless of whether we have SSA information for it.

cc @bdrodes

@MathiasVP MathiasVP requested a review from a team as a code owner January 23, 2026 16:32
Copilot AI review requested due to automatic review settings January 23, 2026 16:32
@github-actions github-actions bot added the C++ label Jan 23, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes DataFlow::Node.asDefinition so it returns a result (marked uncertain = true) even when the underlying StoreInstruction has no corresponding SSA definition, preventing “missing result” cases.

Changes:

  • Adjusted Node.asDefinition/1 implementation to avoid depending on SsaImpl::defToNode for producing any result.
  • Added a new InlineExpectations-based regression test for asDefinition over different store forms (including a field store / dead variable store).

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.

File Description
cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll Updates asDefinition uncertainty determination so stores without SSA still yield a result.
cpp/ql/test/library-tests/dataflow/asDefinition/test.ql Adds an InlineExpectations test query for Node.asDefinition.
cpp/ql/test/library-tests/dataflow/asDefinition/test.cpp Adds C++ test cases exercising assignments/initializers including a field store.
cpp/ql/test/library-tests/dataflow/asDefinition/test.expected Empty InlineExpectations expected-output file for the new test.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant