-
-
Notifications
You must be signed in to change notification settings - Fork 33.4k
Description
Since #130398, the below warning is emitted several times in case of MSVC release (or PGO) builds:
Include\internal\pycore_ceval.h(209): warning C4172: returning address of local variable or temporary : here
So the comment in the code
cpython/Include/internal/pycore_ceval.h
Lines 202 to 211 in 55815a6
| static inline uintptr_t | |
| _Py_get_machine_stack_pointer(void) { | |
| #if _Py__has_builtin(__builtin_frame_address) | |
| return (uintptr_t)__builtin_frame_address(0); | |
| #else | |
| char here; | |
| /* Avoid compiler warning about returning stack address */ | |
| return return_pointer_as_int(&here); | |
| #endif | |
| } |
only works for debug builds. This is because in case of optimizing, MSVC is inlining
return_pointer_as_int
cpython/Include/internal/pycore_ceval.h
Lines 196 to 200 in 55815a6
| #if !_Py__has_builtin(__builtin_frame_address) | |
| static uintptr_t return_pointer_as_int(char* p) { | |
| return (uintptr_t)p; | |
| } | |
| #endif |
and then is "smart" enough to realize and warn about it.
See here https://godbolt.org/z/ooK5Poxo3 (btw gcc or clang won't do that https://godbolt.org/z/459f6hj9G).
__declspec(noinline) would help, but IMHO is an unneeded performance penalty.
I suggest to use #pragma warning(disable:4172) to silence the warning.
Since this is a code generation warning (4700-4999), we have to guard the whole
_Py_get_machine_stack_pointer and cannot just place it around line 209.
@colesbury had a much better idea: use _AddressOfReturnAddress which even gets rid of UB: see https://godbolt.org/z/7c9Gq9jzM and https://github.com/llvm/llvm-project/blob/llvmorg-10.0.0-rc1/clang/lib/Basic/Stack.cpp#L24.