symtable_handle_namedexpr does not adjust correctly the recursion level

[pablogsal]

BPO	37947
Nosy	@ncoghlan, @pablogsal, @miss-islington
PRs	bpo-37947: Adjust correctly the recursion level in symtable for named expressions #15499 [3.8] bpo-37947: Adjust correctly the recursion level in symtable for named expressions (GH-15499) #15515 bpo-37947: Avoid double-decrement in symtable recursion counting #15593 [3.8] bpo-37947: Avoid double-decrement in symtable recursion counting (GH-15593) #15594

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2019-08-26.15:28:36.283>
created_at = <Date 2019-08-26.03:09:10.895>
labels = ['interpreter-core', '3.8', '3.9', 'type-crash']
title = 'symtable_handle_namedexpr does not adjust correctly the recursion level'
updated_at = <Date 2019-09-03.07:49:39.607>
user = 'https://github.com/pablogsal'

bugs.python.org fields:

activity = <Date 2019-09-03.07:49:39.607>
actor = 'ncoghlan'
assignee = 'none'
closed = True
closed_date = <Date 2019-08-26.15:28:36.283>
closer = 'pablogsal'
components = ['Interpreter Core']
creation = <Date 2019-08-26.03:09:10.895>
creator = 'pablogsal'
dependencies = []
files = []
hgrepos = []
issue_num = 37947
keywords = ['patch']
message_count = 9.0
messages = ['350478', '350535', '350540', '350766', '350767', '350770', '350776', '350779', '351071']
nosy_count = 3.0
nosy_names = ['ncoghlan', 'pablogsal', 'miss-islington']
pr_nums = ['15499', '15515', '15593', '15594']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue37947'
versions = ['Python 3.8', 'Python 3.9']

Linked PRs

• gh-82129: Provide __annotate__ method for dataclasses from make_dataclass #122262

[pablogsal]

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).

[pablogsal]

New changeset 0e4ea16 by Pablo Galindo in branch 'master':
bpo-37947: Adjust correctly the recursion level in symtable for named expressions (GH-15499)
0e4ea16

[pablogsal]

New changeset 3769425 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)
3769425

[ncoghlan]

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).

#15593 builds on your PR by removing the recursion counter adjustments from inside the helper function, leaving only the ones in the calling function.

[pablogsal]

Thank you very much Nick for correcting this!

[ncoghlan]

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.

[ncoghlan]

New changeset 0614523 by Nick Coghlan in branch 'master':
bpo-37947: Avoid double-decrement in symtable recursion counting (GH-15593)
0614523

[miss-islington]

New changeset 384c6d7 by Miss Islington (bot) in branch '3.8':
bpo-37947: Avoid double-decrement in symtable recursion counting (GH-15593)
384c6d7

[ncoghlan]

We never actually coded a reproducer for this, but if we had, it would have been a crash bug.