Skip to content

Python: Add tests for reachability when using nonlocal. #2074

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.

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
merged 1 commit into from
Oct 7, 2019

Conversation

taus-semmle
Copy link
Contributor

Variables marked as nonlocal can give rise to false negatives for reachability when they are mutated in a different scope. Thus, a variable that is set to False, and where a local function (with said variable nonlocal mutates it to be True, is considered to be False throughout, and in particular this may lead the extrator to prune the True branch of an if-statement depending on this variable.

On an internal PR, I propose that we do not prune these edges when the variable is also mentioned in a nonlocal statement. This will inevitably lead to some false negatives, and I have added tests to this effect.

I expect these tests to fail until we've merged the internal PR. (But once this is done, we can re-run the tests, and they should then go through. I have of course verified that they work locally.)

@taus-semmle
Copy link
Contributor Author

taus-semmle commented Oct 3, 2019

As expected, the tests failed. For the record, here is the relevant test output:

Expected results:

Actual results:
| nonlocal.py:9:9:9:30 | Raise | Unreachable statement. |
| nonlocal.py:21:9:21:30 | Raise | Unreachable statement. |
| nonlocal.py:31:9:31:30 | Raise | Unreachable statement. |
| nonlocal.py:40:9:40:30 | Raise | Unreachable statement. |
| nonlocal.py:51:9:51:30 | Raise | Unreachable statement. |

@taus-semmle taus-semmle added the depends on internal PR This PR should only be merged in sync with an internal Semmle PR label Oct 4, 2019
@taus-semmle taus-semmle marked this pull request as ready for review October 7, 2019 10:38
Copy link
Member

@RasmusWL RasmusWL left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@semmle-qlci semmle-qlci merged commit ff5a98b into github:master Oct 7, 2019
@tausbn tausbn deleted the python-unreachable-nonlocal branch February 12, 2021 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
depends on internal PR This PR should only be merged in sync with an internal Semmle PR Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants