GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-91048: Prevent optimizing away the asyncio debug offsets structure on Windows by pablogsal · Pull Request #132963 · python/cpython

Conversation

@pablogsal

@pablogsal pablogsal commented Apr 25, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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.

  • Issue: Async Call-Stack Reconstruction #91048

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@pablogsal
pythongh-91048: Prevent optimizing away the asyncio debug offsets str…
4e99743 
…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.
@pablogsal
pablogsal requested review from 1st1, asvetlov, kumaraditya303 and willingc as code owners April 25, 2025 16:57
@bedevere-app bedevere-app Bot added the awaiting core review label Apr 25, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Apr 25, 2025
Async Call-Stack Reconstruction #91048
Closed
@pablogsal pablogsal mentioned this pull request Apr 25, 2025
gh-91048: Refactor _testexternalinspection and add Windows support #132852
Merged
@pablogsal
pablogsal requested review from Fidget-Spinner and chris-eibl April 25, 2025 16:57
@pablogsal pablogsal added the skip news label Apr 25, 2025
@pablogsal
pablogsal force-pushed the gh-91048-win branch from 00d2604 to 4e99743 Compare April 25, 2025 16:59
@Fidget-Spinner

Fidget-Spinner commented Apr 25, 2025

Copy link
Copy Markdown
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?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@pablogsal

pablogsal commented Apr 25, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Fidget-Spinner
Fidget-Spinner approved these changes Apr 25, 2025
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Apr 25, 2025
@pablogsal
pablogsal enabled auto-merge (squash) April 25, 2025 17:02
chris-eibl
chris-eibl reviewed Apr 25, 2025
View reviewed changes
Comment thread Modules/_asynciomodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@pablogsal

pablogsal commented Apr 25, 2025

Copy link
Copy Markdown
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]
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@pablogsal
pablogsal disabled auto-merge April 25, 2025 17:08
@pablogsal

pablogsal commented Apr 25, 2025

Copy link
Copy Markdown
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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
pablogsal
pablogsal commented Apr 25, 2025
View reviewed changes
Comment thread Modules/_asynciomodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
pablogsal
pablogsal commented Apr 25, 2025
View reviewed changes
Comment thread Modules/_asynciomodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@pablogsal
Apply suggestions from code review
6f6bde5
pablogsal
pablogsal commented Apr 25, 2025
View reviewed changes
Comment thread Modules/_asynciomodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@pablogsal
Update Modules/_asynciomodule.c
d327223
@pablogsal
pablogsal enabled auto-merge (squash) April 25, 2025 17:18
@pablogsal
pablogsal merged commit a5e628b into python:main Apr 25, 2025
@pablogsal
pablogsal deleted the gh-91048-win branch April 25, 2025 17:43
@bedevere-app bedevere-app Bot removed the awaiting merge label Apr 25, 2025
chris-eibl
chris-eibl reviewed Apr 25, 2025
View reviewed changes
Comment thread Modules/_asynciomodule.c

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

@chris-eibl chris-eibl Apr 25, 2025

Copy link
Copy Markdown
Member

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.

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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@pablogsal pablogsal Apr 25, 2025

Copy link
Copy Markdown
Member Author

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 I still prefer to keep it

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
chris-eibl reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zooba zooba zooba left review comments
@chris-eibl chris-eibl chris-eibl left review comments
@Fidget-Spinner Fidget-Spinner Fidget-Spinner approved these changes
@1st1 1st1 Awaiting requested review from 1st1 1st1 is a code owner
@asvetlov asvetlov Awaiting requested review from asvetlov asvetlov is a code owner
@kumaraditya303 kumaraditya303 Awaiting requested review from kumaraditya303 kumaraditya303 is a code owner
@willingc willingc Awaiting requested review from willingc willingc is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@pablogsal @Fidget-Spinner @zooba @chris-eibl
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.