gh-151515: Fix incorrect function pointer cast in asdl_c.py generator#151514
gh-151515: Fix incorrect function pointer cast in asdl_c.py generator#151514CoderSilicon wants to merge 6 commits into
Conversation
Updated formatting and added emphasis to copyright and license information. Improved clarity in several sections.
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
picnixz
left a comment
There was a problem hiding this comment.
Plsease revert all cosmetic changes in this PR. Only change the part that may need to change.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
| struct ast_state *state = &interp->ast; | ||
| assert(!state->finalized); | ||
| if (_PyOnceFlag_CallOnce(&state->once, (_Py_once_fn_t *)&init_types, state) < 0) { | ||
| if (_PyOnceFlag_CallOnce(&state->once, (_Py_once_fn_t)init_types, state) < 0) { |
There was a problem hiding this comment.
The symbol init_types is declared a few lines up as a function, not a variable, so regardless of whether you take its address (&init_types) or not, you always get a pointer to the function.
From Include/internal/pycore_lock.h, _Py_once_fn_t is a function (signature) type, not a function pointer type:
typedef int _Py_once_fn_t(void *arg);
I don’t think it is valid to cast to a function type.
I don’t see how the original code produces undefined behaviour. However, why not just remove the cast?
| if (_PyOnceFlag_CallOnce(&state->once, (_Py_once_fn_t)init_types, state) < 0) { | |
| if (_PyOnceFlag_CallOnce(&state->once, &init_types, state) < 0) { |
Summary
This PR fixes an undefined behavior bug in the C code generated by
Parser/asdl_c.pyforget_ast_state().The Problem
In the generated code,
_PyOnceFlag_CallOnceis invoked like this:_PyOnceFlag_CallOnce(&state->once, (_Py_once_fn_t *)&init_types, state)Using
&init_typespasses a pointer to a function pointer (effectively a double pointer), rather than the function pointer itself. by forcing this with an explicit cast(_Py_once_fn_t *)silences the compiler but triggers a strict aliasing violation and undefined behavior at runtime when the pointer is dereferenced. This can lead to compiler-optimization-driven segmentation faults, particularly in free-threaded/GIL-disabled builds where one-time initialization flags are heavily relied upon.The Fix
Updated
Parser/asdl_c.pyto passinit_typesdirectly with a standard function pointer cast(_Py_once_fn_t)init_types(removing the extra&and*). Ranmake regen-astto cleanly update the generated files.