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-137573: Check C stack depth before stack allocating JIT optimizer struct by markshannon · Pull Request #137676 · python/cpython

Conversation

@markshannon

@markshannon markshannon commented Aug 12, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member
  • Issue: Stack overflow error from recursion with 3.14.0rc1 on MacOS #137573

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
markshannon added 2 commits August 12, 2025 15:01
@markshannon
Check C stack depth before stack allocating JIT optimizer struct
a2c2a78
@markshannon
Add news
ef27395
@markshannon
markshannon requested review from Fidget-Spinner and tomasr8 as code owners August 12, 2025 14:02
@bedevere-app bedevere-app Bot added the awaiting core review label Aug 12, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Aug 12, 2025
Stack overflow error from recursion with 3.14.0rc1 on MacOS #137573
Closed
@markshannon markshannon assigned Fidget-Spinner Aug 12, 2025
AA-Turner
AA-Turner reviewed Aug 12, 2025
View reviewed changes
Comment thread Lib/test/test_call.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@Fidget-Spinner

Fidget-Spinner commented Aug 12, 2025

Copy link
Copy Markdown
Member

Just so I'm clear: you plan to move this to the thread state later but only on main right? So that we don't change the struct layout in an rc for 3.14?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
markshannon and others added 2 commits August 13, 2025 11:44
@markshannon @AA-Turner
Update Lib/test/test_call.py
0a889df 
Co-authored-by: Adam Turner <[email protected]>
@markshannon
Account for trace buffer as well
be99bd4
@Fidget-Spinner

Fidget-Spinner commented Aug 13, 2025

Copy link
Copy Markdown
Member

Can you please add a Py_NO_INLINE to the _PyOptimizer_Optimize function too? I don't think this will fix it as if it's inlined, the alloca might be hoisted out to the main interpreter loop by the compiler.
All reactions

Sorry, something went wrong.

Uh oh!

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

markshannon commented Aug 13, 2025

Copy link
Copy Markdown
Member Author

Can you please add a Py_NO_INLINE to the _PyOptimizer_Optimize function too? I don't think this will fix it as if it's inlined, the alloca might be hoisted out to the main interpreter loop by the compiler.

Yes, but not in this PR as it seems unrelated
Fidget-Spinner reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

markshannon commented Aug 13, 2025

Copy link
Copy Markdown
Member Author

Just so I'm clear: you plan to move this to the thread state later but only on main right? So that we don't change the struct layout in an rc for 3.14?

Yes, that's the plan.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@markshannon markshannon added the needs backport to 3.14 bugs and security fixes label Aug 13, 2025
Fidget-Spinner
Fidget-Spinner reviewed Aug 14, 2025
View reviewed changes
Comment thread Python/optimizer_analysis.c
OPT_STAT_INC(optimizer_attempts);
/* Make sure we have enough C stack space for the optimizer */
int margin = 1 + sizeof(JitOptContext)/_PyOS_STACK_MARGIN_BYTES;
if (_Py_ReachedRecursionLimitWithMargin(_PyThreadState_GET(), margin)) {

@Fidget-Spinner Fidget-Spinner Aug 14, 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.

To work properly, the optimize_uops function or this function must be marked Py_NO_INLINE as well.

Otherwise, the compiler can just inline the functions and hoist the alloca above the check, making the check useless.

Sorry, something went wrong.

Uh oh!

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

markshannon commented Sep 18, 2025

Copy link
Copy Markdown
Member Author

We've already started moving the buffers to the heap, so this won't be needed.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@markshannon markshannon closed this Sep 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AA-Turner AA-Turner AA-Turner left review comments
@Fidget-Spinner Fidget-Spinner Fidget-Spinner left review comments
@tomasr8 tomasr8 Awaiting requested review from tomasr8 tomasr8 is a code owner

Uh oh!

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

Assignees

@Fidget-Spinner Fidget-Spinner

Labels

awaiting core review needs backport to 3.14 bugs and security fixes

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.

3 participants

@markshannon @Fidget-Spinner @AA-Turner
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.