Skip to content

Conversation

smowton
Copy link
Contributor

@smowton smowton commented Jan 6, 2022

This avoids lots of missing-node warnings from codeql bqrs interpret as it discards the nodes that occur in the edges relation but not nodes. The problem arises because subpaths introduced two variants of reach, one of which is more restrictive than simply reach(succ) and succ = pred.getASuccessor(), so it no longer suffices to just check that the successor is reachable.

Suggestions re: the best way to test this welcome.

@smowton smowton requested review from a team as code owners January 6, 2022 11:40
@smowton smowton added the no-change-note-required This PR does not need a change note label Jan 6, 2022
@MathiasVP
Copy link
Contributor

The changes to the .expected files LGTM.

@aschackmull
Copy link
Contributor

Looks like the qltest failures are highlighting a pre-existing bug that I was actually just investigating in a separate context. I don't have a complete overview yet, but I expect to fix it in a separate PR.

@aschackmull
Copy link
Contributor

Looks like the qltest failures are highlighting a pre-existing bug that I was actually just investigating in a separate context. I don't have a complete overview yet, but I expect to fix it in a separate PR.

Actually, the fix is simple and would likely conflict, so I'll just push it here.

smowton and others added 11 commits January 18, 2022 10:30
…ot reachable

This avoids lots of missing-node warnings from `codeql bqrs interpret` as it discards the nodes that occur in the `edges` relation but not `nodes`. The problem arises because subpaths introduced two variants of `reach`, one of which is more restrictive than simply `reach(succ) and succ = pred.getASuccessor()`, so it no longer suffices to just check that the successor is reachable.
@aschackmull aschackmull force-pushed the smowton/fix/restore-nodes-edges-consistency branch from c4e0493 to 9479301 Compare January 18, 2022 09:37
@smowton smowton merged commit 9819752 into github:main Jan 18, 2022
@owen-mc owen-mc changed the title Don't include arg -> param edges in PathGraph::edges where arg is not reachable Dataflow: Don't include arg -> param edges in PathGraph::edges where arg is not reachable Nov 2, 2022
owen-mc added a commit to owen-mc/codeql that referenced this pull request Nov 9, 2022
In the `subpaths` section, the last node is now printed without its type
if it is the sink of the path.

This comes from the commit "Dataflow: Bugfix: include subpaths ending at
a sink. " in github#7526
owen-mc added a commit to owen-mc/codeql that referenced this pull request Nov 9, 2022
In the `subpaths` section, the last node is now printed without its type
if it is the sink of the path.

This comes from the commit "Dataflow: Bugfix: include subpaths ending at
a sink. " in github#7526
owen-mc added a commit to owen-mc/codeql that referenced this pull request Nov 9, 2022
In the `subpaths` section, the last node is now printed without its type
if it is the sink of the path.

This comes from the commit "Dataflow: Bugfix: include subpaths ending at
a sink. " in github#7526
owen-mc added a commit to owen-mc/codeql that referenced this pull request Nov 17, 2022
In the `subpaths` section, the last node is now printed without its type
if it is the sink of the path.

This comes from the commit "Dataflow: Bugfix: include subpaths ending at
a sink. " in github#7526
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.

5 participants