Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
gh-132314: fix stack array init warning#132376
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
python-cla-botbot commentedApr 10, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
All commit authors signed the Contributor License Agreement. |
Most changes to Pythonrequire a NEWS entry. Add one using theblurb_it web app or theblurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
There was no warning with the configuration without options, but I reproduced it with the --enable-optimization --with-lto options. The mistake is very simple. |
Python/ceval.c Outdated
| @@ -1814,7 +1814,7 @@ _PyEvalFramePushAndInit_Ex(PyThreadState *tstate, _PyStackRef func, | |||
| PyObject *kwnames = NULL; | |||
| _PyStackRef *newargs; | |||
| PyObject *const *object_array = NULL; | |||
| _PyStackRef stack_array[8]; | |||
| _PyStackRef stack_array[8] = {}; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
IMHO, the compiler warning is a false-positive.
If thestack_array is used
Lines 1832 to 1834 inad3bbe8
| if (nargs <=8) { | |
| newargs=stack_array; | |
| } |
it is initialized
Lines 1844 to 1846 inad3bbe8
| for (Py_ssize_ti=0;i<nargs;i++) { | |
| newargs[i]=PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(callargs,i)); | |
| } |
Having a small array on the stack to avoid thePyMem_Malloc is a common pattern, e.g. further down
Line 1880 inad3bbe8
| _PyStackRefstack_array[8]; |
or like mentioned in the issue#132314
Line 1450 inad3bbe8
| PyObject*small_stack[_PY_FASTCALL_SMALL_STACK]; |
They all follow the same pattern, and if the compiler is not smart enough to detect it, it will issue a warning.
IMHO, initializing those stack variables using= {} is a waste of CPU cycles, and we should silence the warning using adiagnostic pragma.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think hiding warnings using pragma it is not good idea, because we have other compilers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yeah, but not all of them emit the warning, so we only have to silence those that do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yes and we can selectively decide which compilers need to be silenced as well so I think for a falsey positive like that it's better to silence the warning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Fixed. Check please new version.@chris-eibl@picnixz
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Sorry@dura0ok, I totally missed this one - but#134207 rang a bell and so I am here again.
I had a simpler approach in mind just using a pragma for GCC directly in the affected places but I like your approach, too, since it is re-usable.
There is already infrastructure and I'd place the macro next to
Lines 297 to 302 inc740fe3
| #if defined(__clang__) | |
| #define_Py_COMP_DIAG_PUSH_Pragma("clang diagnostic push") | |
| #define_Py_COMP_DIAG_IGNORE_DEPR_DECLS \ | |
| _Pragma("clang diagnostic ignored\"-Wdeprecated-declarations\"") | |
| #define_Py_COMP_DIAG_POP_Pragma("clang diagnostic pop") | |
| #elif defined(__GNUC__) \ |
I'd also go with the naming scheme used there, e.g.
_Py_COMP_DIAG_IGNORE_MAYBE_UNINITIALIZED, and use it likeLines 522 to 530 inc740fe3
| staticvoid | |
| pymain_set_inspect(PyConfig*config,intinspect) | |
| { | |
| config->inspect=inspect; | |
| _Py_COMP_DIAG_PUSH | |
| _Py_COMP_DIAG_IGNORE_DEPR_DECLS | |
| Py_InspectFlag=inspect; | |
| _Py_COMP_DIAG_POP | |
| } |
Add macros to disable and re-enable compiler-specific warnings aboutpossibly uninitialized variables (`-Wmaybe-uninitialized` for GCC,`-Wuninitialized` for Clang, and warning C4700 for MSVC). Use thesemacros in `_PyEvalFramePushAndInit_Ex()` to suppress false positiveson stack-allocated arrays like `stack_array`.This also addresses warnings such as the one in `pycore_call.h` whenusing `small_stack[_PY_FASTCALL_SMALL_STACK]` that may be flagged byGCC's `-Wmaybe-uninitialized`.
Uh oh!
There was an error while loading.Please reload this page.
Dummy warning fix