Skip to content
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

Probable extra semicolon in Py_LeaveRecursiveCall macro #45936

Closed
amauryfa opened this issue Dec 11, 2007 · 3 comments
Closed

Probable extra semicolon in Py_LeaveRecursiveCall macro #45936

amauryfa opened this issue Dec 11, 2007 · 3 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs)

Comments

@amauryfa
Copy link
Member

BPO 1595
Nosy @loewis, @amauryfa, @orivej

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 = 'https://github.com/loewis'
closed_at = <Date 2008-02-12.19:30:56.470>
created_at = <Date 2007-12-11.21:40:48.420>
labels = ['interpreter-core']
title = 'Probable extra semicolon in Py_LeaveRecursiveCall macro'
updated_at = <Date 2008-02-12.19:30:56.469>
user = 'https://github.com/amauryfa'

bugs.python.org fields:

activity = <Date 2008-02-12.19:30:56.469>
actor = 'loewis'
assignee = 'loewis'
closed = True
closed_date = <Date 2008-02-12.19:30:56.470>
closer = 'loewis'
components = ['Interpreter Core']
creation = <Date 2007-12-11.21:40:48.420>
creator = 'amaury.forgeotdarc'
dependencies = []
files = []
hgrepos = []
issue_num = 1595
keywords = []
message_count = 3.0
messages = ['58468', '58471', '62330']
nosy_count = 3.0
nosy_names = ['loewis', 'amaury.forgeotdarc', 'orivej']
pr_nums = []
priority = 'high'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue1595'
versions = ['Python 3.0']

@amauryfa
Copy link
Member Author

In file ceval.h, the macro Py_LeaveRecursiveCall is defined like this:

#define Py_LeaveRecursiveCall()				\
    do{ if((--PyThreadState_GET()->recursion_depth) <   \
	   _Py_CheckRecursionLimit - 50);               \
	  PyThreadState_GET()->overflowed = 0;          \
    } while(0)

The semicolon on the third line seems very suspicious to me: the if()
statement has no side effect, and "overflowed" is always reset to zero.

I don't really understand the consequences, though. The variable seems
to be used as an additional protection against C code that does not
correctly unwind when the recursion limit is hit.

@amauryfa amauryfa added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Dec 11, 2007
@loewis
Copy link
Mannequin

loewis mannequin commented Dec 11, 2007

I think you are right - that's a bug. As a consequence, overflowed would
be cleared less often than it is now, which in turn may cause a fatal
abort in case it fails to recover from a stack overflow (i.e. if it
consumes another 50 stack frames, instead of unwinding).

This entire machinery is there to guard against hard-to-analyze crashes
resulting from stack overflows, in particular when a dictionary lookup
happened to cause a stack overflow. The overflow would raise the Python
exception, which would then be cleared in the dictionary lookup, as if
nothing happened.

@loewis
Copy link
Mannequin

loewis mannequin commented Feb 12, 2008

Thanks again for pointing that out. Fixed in r60750.

@loewis loewis mannequin closed this as completed Feb 12, 2008
@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs)
Projects
None yet
Development

No branches or pull requests

1 participant