Skip to content

gh-91048: Prevent optimizing away the asyncio debug offsets structure on Windows #132963

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

Merged
merged 3 commits into from
Apr 25, 2025

Conversation

pablogsal
Copy link
Member

@pablogsal pablogsal commented Apr 25, 2025

To avoid having the debug sections being optimised away by the compiler we use attribute((used)) on gcc and clang but in Windows this is not supported by the Microsoft compiler and there is no equivalent flag. Unfortunately Windows offers almost no alternative other than exporting the symbol in the dynamic table or using it somehow.

…ucture on Windows

To avoid having the debug sections being optimised away by the compiler
we use  __attribute__((used)) on gcc and clang but in Windows this is
not supported by the Microsoft compiler and there is no equivalent flag.
Unfortunately Windows offers almost no alternative other than exporting
the symbol in the dynamic table or using it somehow.
@Fidget-Spinner
Copy link
Member

Sorry can't test this out right now, but does dllexport not prevent it from being optimized away? Or are there other reasons we can't use it?

@pablogsal
Copy link
Member Author

pablogsal commented Apr 25, 2025

Sorry can't test this out right now, but does dllexport not prevent it from being optimized away?

Check the commit message: we don't want to export the symbol in the dynamic table. This is basically the less intrusive option Windows offers.

@pablogsal
Copy link
Member Author

Just for completeness, we also get an error if we try to add __declspec(dllexport) to the macro:

C:\Users\pablogsal\GitHub\cpython\Python\pylifecycle.c(113,1): error C2370: '_PyRuntime': redefinition; different storage class [C:\Users\pablogsal\GitHub\cpython\PCbuild\_freeze_module.vcxproj]

we could have it as an option only for shared modules but then we need to deal with this other error:

C:\Users\pablogsal\GitHub\cpython\PCbuild\win32\_freeze_module.exe : fatal error LNK1120: 1 unresolved externals [C:\Users\pablogsal\GitHub\cpython\PCbuild\_freeze_module.vcxproj]

@pablogsal pablogsal disabled auto-merge April 25, 2025 17:08
@pablogsal
Copy link
Member Author

I also tried doing this but doesn't work:

#if defined(MS_WINDOWS)
#define _GENERATE_DEBUG_SECTION_WINDOWS(name)                       \
   _Pragma("optimize(\"g\", off)")                                 \
   _Pragma(Py_STRINGIFY(section(Py_STRINGIFY(name), read, write))) \
   __declspec(allocate(Py_STRINGIFY(name)))                        \
   _Pragma("optimize(\"g\", on)")
#else
#define _GENERATE_DEBUG_SECTION_WINDOWS(name)
#endif

@pablogsal pablogsal enabled auto-merge (squash) April 25, 2025 17:18
@pablogsal pablogsal merged commit a5e628b into python:main Apr 25, 2025
41 checks passed
@pablogsal pablogsal deleted the gh-91048-win branch April 25, 2025 17:43
@@ -185,6 +185,10 @@ typedef struct {
/* Counter for autogenerated Task names */
uint64_t task_name_counter;

/* Pointer to the asyncio debug offset to avoid it to be optimized away
by the compiler */
void *debug_offsets;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then we'd even do not need the __attribute__((used)) any longer in the GENERATE_DEBUG_SECTION macros.
But it doesn't hurt either - so let's keep it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I still prefer to keep it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants