Skip to content

Conversation

tausbn
Copy link
Contributor

@tausbn tausbn commented Mar 3, 2020

I could not find an easy way to avoid the cartesian product in #2700, so I have simply disabled the code responsible for it.

@tausbn tausbn added Python Priority PR that should be reviewed and merged as a matter of priority. labels Mar 3, 2020
@tausbn tausbn requested a review from BekaValentine March 3, 2020 17:21
@tausbn tausbn requested a review from a team as a code owner March 3, 2020 17:21
Copy link
Contributor

@BekaValentine BekaValentine left a comment

Choose a reason for hiding this comment

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

In reviewing this, do you want me to merely look for noticeable errors, and make constructive suggestions on the changes as done, or should I also try to understand the cause of the CP and see if there might be some kind of alternative solution?

@tausbn
Copy link
Contributor Author

tausbn commented Mar 3, 2020

This is mostly a hotfix, so that we can progress with the on-going dist upgrade, and so an in-depth review is probably not needed. Just a quick look to see that I haven't done something completely ridiculous.

Of course, if you do figure out a solution for the CP that I just removed by brute force (by essentially reverting to the previous behaviour), that would be great.

@BekaValentine
Copy link
Contributor

Ok sounds good.

@BekaValentine BekaValentine self-requested a review March 4, 2020 00:07
@BekaValentine BekaValentine self-requested a review March 4, 2020 00:31
@semmle-qlci semmle-qlci merged commit c4b961c into github:master Mar 4, 2020
Comment on lines -230 to 240
private predicate clears_taint(ControlFlowNode final_test, ControlFlowNode tainted, ControlFlowNode test, boolean sense) {
test_equality_with_const(final_test, tainted, sense)
private predicate clears_taint(ControlFlowNode tainted, ControlFlowNode test, boolean sense) {
test_equality_with_const(test, tainted, sense)
or
test_in_const_seq(final_test, tainted, sense)
test_in_const_seq(test, tainted, sense)
or
test.(UnaryExprNode).getNode().getOp() instanceof Not and
exists(ControlFlowNode nested_test |
nested_test = test.(UnaryExprNode).getOperand() and
clears_taint(final_test, tainted, nested_test, sense.booleanNot())
clears_taint(tainted, nested_test, sense.booleanNot())
)
}
Copy link
Member

Choose a reason for hiding this comment

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

Looking over this code again, I'm so confused about final_test and what it's supposed to do. Thanks for fixing this up 👍

@tausbn tausbn deleted the python-fix-or-disable-cps branch February 12, 2021 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority PR that should be reviewed and merged as a matter of priority. Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants