gh-132314: fix stack array init warning #132376
Conversation
|
All commit authors signed the Contributor License Agreement. |
|
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 |
|
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.
IMHO, the compiler warning is a false-positive.
If the stack_array is used
Lines 1832 to 1834 in ad3bbe8
it is initialized
Lines 1844 to 1846 in ad3bbe8
Having a small array on the stack to avoid the PyMem_Malloc is a common pattern, e.g. further down
Line 1880 in ad3bbe8
or like mentioned in the issue #132314
Line 1450 in ad3bbe8
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 a diagnostic pragma.
There was a problem hiding this comment.
I think hiding warnings using pragma it is not good idea, because we have other compilers.
There was a problem hiding this comment.
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.
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.
Fixed. Check please new version. @chris-eibl @picnixz
There was a problem hiding this comment.
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 in c740fe3
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 in c740fe3
Add macros to disable and re-enable compiler-specific warnings about possibly uninitialized variables (`-Wmaybe-uninitialized` for GCC, `-Wuninitialized` for Clang, and warning C4700 for MSVC). Use these macros in `_PyEvalFramePushAndInit_Ex()` to suppress false positives on stack-allocated arrays like `stack_array`. This also addresses warnings such as the one in `pycore_call.h` when using `small_stack[_PY_FASTCALL_SMALL_STACK]` that may be flagged by GCC's `-Wmaybe-uninitialized`.
Dummy warning fix