Skip to content

Python: Fix bad join order in adjacentUseUseSameVar #4544

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
RasmusWL merged 1 commit into from
Oct 23, 2020
Merged

Python: Fix bad join order in adjacentUseUseSameVar #4544

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 27 additions & 3 deletions python/ql/src/semmle/python/essa/SsaCompute.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,16 +431,40 @@ private module SsaComputeImpl {
defSourceUseRank(v, b2, 1, i2)
}

/**
* Holds if `use1` is a use of the variable `v`, and there exists an adjacent reference to `v`
* in basic block `b1` at index `i1`.
*
* A helper predicate for `adjacentUseUseSameVar`, to prevent the first join from being between
* the two instances of `variableSourceUse` in
* ```ql
* exists(SsaSourceVariable v, BasicBlock b1, int i1, BasicBlock b2, int i2 |
* adjacentVarRefs(v, b1, i1, b2, i2) and
* variableSourceUse(v, use1, b1, i1) and
* variableSourceUse(v, use2, b2, i2)
* )
* ```
*/
pragma[nomagic]
cached
Copy link
Contributor

@hvitved hvitved Oct 26, 2020

Choose a reason for hiding this comment

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

Why cached when this is only a helper predicate used by predicate that is already cached?

Copy link
Contributor Author

@tausbn tausbn Oct 26, 2020

Choose a reason for hiding this comment

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

Ah, good point. I think what happened here is I started out with a non-private helper predicate, which must be cached because the module is. For some reason I decided to make it cached rather than private (and then later on I made it private but forgot to remove the cached annotation).

Thanks!

Copy link
Member

@RasmusWL RasmusWL Nov 4, 2020

Choose a reason for hiding this comment

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

Made PR to fix this in #4608

private predicate adjacentRefUse(
SsaSourceVariable v, BasicBlock b2, int i2, ControlFlowNode use1
) {
exists(BasicBlock b1, int i1 |
adjacentVarRefs(v, b1, i1, b2, i2) and
variableSourceUse(v, use1, b1, i1)
)
}

/**
* Holds if `use1` and `use2` form an adjacent use-use-pair of the same SSA
* variable, that is, the value read in `use1` can reach `use2` without passing
* through any other use or any SSA definition of the variable.
*/
cached
predicate adjacentUseUseSameVar(ControlFlowNode use1, ControlFlowNode use2) {
exists(SsaSourceVariable v, BasicBlock b1, int i1, BasicBlock b2, int i2 |
adjacentVarRefs(v, b1, i1, b2, i2) and
variableSourceUse(v, use1, b1, i1) and
exists(SsaSourceVariable v, BasicBlock b2, int i2 |
adjacentRefUse(v, b2, i2, use1) and
variableSourceUse(v, use2, b2, i2)
)
}
Expand Down