Skip to content

Conversation

@markshannon
Copy link
Contributor

No description provided.

@markshannon markshannon added the WIP This is a work-in-progress, do not merge yet! label Jun 18, 2019
@markshannon markshannon force-pushed the python-points-to-more-unknowns branch 2 times, most recently from 5ab7117 to fdf033a Compare June 19, 2019 10:10
@markshannon markshannon changed the title Python: Reduce unreachable nodes for unknowns. WORK IN PROGRESS. Python: Fix getOperand for 'not' node and make sure it can only point-to a boolean. Jun 19, 2019
@markshannon
Copy link
Contributor Author

The particular case this PR fixes is for

def invert_after_or(x, y):
    if not (x or y):
        b1
    else:
        b2

test

@markshannon markshannon force-pushed the python-points-to-more-unknowns branch from fdf033a to 5b145ed Compare June 19, 2019 10:23
@markshannon markshannon removed the WIP This is a work-in-progress, do not merge yet! label Jun 20, 2019
Copy link
Contributor

@taus-semmle taus-semmle left a comment

Choose a reason for hiding this comment

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

One question, otherwise LGTM.

op instanceof USub and value = ObjectInternal::fromInt(-opvalue.intValue())
or
opvalue = ObjectInternal::unknown() and value = opvalue
not op instanceof Not and opvalue = ObjectInternal::unknown() and value = opvalue
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to double-check, this extra condition is needed because ObjectInternal::unknown() has a booleanValue (in fact, it has both such values), and therefore without this instanceof check, we would have that a not of an unknown value was both True, False, and unknown.

I gather USub doesn't present a similar problem, because ObjectInternal::unknown() doesn't have an intValue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, exactly. We want -unknown and ~unknown to be unknown, but not unknown to be True or False

@taus-semmle taus-semmle merged commit 832abc7 into github:master Jun 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants