classification
Title: symtable_handle_namedexpr does not adjust correctly the recursion level
Type: crash Stage: resolved
Components: Interpreter Core Versions: Python 3.9, Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: miss-islington, ncoghlan, pablogsal
Priority: normal Keywords: patch

Created on 2019-08-26 03:09 by pablogsal, last changed 2019-09-03 07:49 by ncoghlan. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 15499 merged pablogsal, 2019-08-26 03:10
PR 15515 merged pablogsal, 2019-08-26 14:56
PR 15593 merged ncoghlan, 2019-08-29 12:20
PR 15594 merged miss-islington, 2019-08-29 13:27
Messages (9)
msg350478 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2019-08-26 03:09
The symtable_handle_namedexpr function does not adjust correctly the recursion level when exiting (also, is actually not returning anything in a function defined as non-void but the return value is not used so is not technically undefined behavior).
msg350535 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2019-08-26 14:52
New changeset 0e4ea16336685cf3fa353d8c54af59b45b2d5c33 by Pablo Galindo in branch 'master':
bpo-37947: Adjust correctly the recursion level in symtable for named expressions (GH-15499)
https://github.com/python/cpython/commit/0e4ea16336685cf3fa353d8c54af59b45b2d5c33
msg350540 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2019-08-26 15:27
New changeset 3769425abd8da9a59b9645baf90ef49b9c69c140 by Pablo Galindo in branch '3.8':
[3.8] bpo-37947: Adjust correctly the recursion level in symtable for named expressions (GH-15499) (GH-15515)
https://github.com/python/cpython/commit/3769425abd8da9a59b9645baf90ef49b9c69c140
msg350766 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2019-08-29 12:22
Reviewing the PR post-merge, I'm pretty sure this has introduced a double-decrement bug due to the original code being hard to read (the error cases did the decrement inside the helper function, while the success case did it in the calling function).

https://github.com/python/cpython/pull/15593 builds on your PR by removing the recursion counter adjustments from inside the helper function, leaving only the ones in the calling function.
msg350767 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2019-08-29 12:31
Thank you very much Nick for correcting this!
msg350770 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2019-08-29 12:43
Reviewing all the code that touches recursion_depth (both in the symtable and in the thread state), I'm not seeing any sanity checks that ensure our increments and decrements *balance*.

So I'm going to add one to PySymTable_BuildObject.
msg350776 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2019-08-29 13:27
New changeset 06145230c833c3db5dab8858e11bcd550a37c57f by Nick Coghlan in branch 'master':
bpo-37947: Avoid double-decrement in symtable recursion counting (GH-15593)
https://github.com/python/cpython/commit/06145230c833c3db5dab8858e11bcd550a37c57f
msg350779 - (view) Author: miss-islington (miss-islington) Date: 2019-08-29 13:46
New changeset 384c6d72d9a12764282ccc2d3935232a34a7cfbe by Miss Islington (bot) in branch '3.8':
bpo-37947: Avoid double-decrement in symtable recursion counting (GH-15593)
https://github.com/python/cpython/commit/384c6d72d9a12764282ccc2d3935232a34a7cfbe
msg351071 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2019-09-03 07:49
We never actually coded a reproducer for this, but if we had, it would have been a crash bug.
History
Date User Action Args
2019-09-03 07:49:39ncoghlansettype: crash
messages: + msg351071
2019-08-29 13:46:25miss-islingtonsetnosy: + miss-islington
messages: + msg350779
2019-08-29 13:27:16miss-islingtonsetpull_requests: + pull_request15270
2019-08-29 13:27:01ncoghlansetmessages: + msg350776
2019-08-29 12:43:33ncoghlansetmessages: + msg350770
2019-08-29 12:31:35pablogsalsetmessages: + msg350767
2019-08-29 12:22:43ncoghlansetnosy: + ncoghlan
messages: + msg350766
2019-08-29 12:20:16ncoghlansetpull_requests: + pull_request15269
2019-08-26 15:28:36pablogsalsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2019-08-26 15:27:34pablogsalsetmessages: + msg350540
2019-08-26 14:56:22pablogsalsetpull_requests: + pull_request15202
2019-08-26 14:52:46pablogsalsetmessages: + msg350535
2019-08-26 03:10:55pablogsalsetkeywords: + patch
stage: patch review
pull_requests: + pull_request15188
2019-08-26 03:09:10pablogsalcreate