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-130396: Broaden definition of "optimized" for gdb tests by markshannon · Pull Request #130550 · python/cpython

Conversation

@markshannon

@markshannon markshannon commented Feb 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
  • Issue: Implement stack overflow protection for linux based on actual stack depth #130396

Sorry, something went wrong.

Uh oh!

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

All reactions
@bedevere-app bedevere-app Bot added the tests Tests in the Lib/test dir label Feb 25, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Feb 25, 2025
Implement stack overflow protection for linux based on actual stack depth #130396
Closed
@markshannon

markshannon commented Feb 25, 2025

Copy link
Copy Markdown
Member Author

!buildbot AMD64.Fedora.Stable.Clang.3.x

All reactions

Sorry, something went wrong.

Uh oh!

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

@bedevere-bot

bedevere-bot commented Feb 25, 2025

Copy link
Copy Markdown

The regex 'AMD64.Fedora.Stable.Clang.3.x' did not match any buildbot builder. Is the requested builder in the list of stable builders?

All reactions

Sorry, something went wrong.

Uh oh!

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

@markshannon markshannon added the skip news label Feb 25, 2025
@markshannon

markshannon commented Feb 25, 2025

Copy link
Copy Markdown
Member Author

!buildbot Fedora.*Clang

All reactions

Sorry, something went wrong.

Uh oh!

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

@bedevere-bot

bedevere-bot commented Feb 25, 2025

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @markshannon for commit 3a3817e 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F130550%2Fmerge

The command will test the builders whose names match following regular expression: Fedora.*Clang

The builders matched are:

  • aarch64 Fedora Rawhide Clang PR
  • AMD64 Fedora Rawhide Clang Installed PR
  • PPC64LE Fedora Stable Clang Installed PR
  • PPC64LE Fedora Rawhide Clang Installed PR
  • aarch64 Fedora Stable Clang PR
  • AMD64 Fedora Stable Clang Installed PR
  • aarch64 Fedora Rawhide Clang Installed PR
  • AMD64 Fedora Stable Clang PR
  • aarch64 Fedora Stable Clang Installed PR
  • PPC64LE Fedora Stable Clang PR
  • PPC64LE Fedora Rawhide Clang PR
  • AMD64 Fedora Rawhide Clang PR
All reactions

Sorry, something went wrong.

Uh oh!

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

@markshannon

markshannon commented Feb 25, 2025

Copy link
Copy Markdown
Member Author

Let's try again

!buildbot Fedora.*Clang

All reactions

Sorry, something went wrong.

Uh oh!

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

@markshannon
markshannon marked this pull request as ready for review February 25, 2025 15:49
@bedevere-app bedevere-app Bot added the awaiting core review label Feb 25, 2025
@markshannon

markshannon commented Feb 25, 2025

Copy link
Copy Markdown
Member Author

Once more for luck 🙂

!buildbot AMD.*Fedora.*Clang

All reactions

Sorry, something went wrong.

Uh oh!

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

@python python deleted a comment from bedevere-bot Feb 25, 2025
@vstinner

vstinner commented Feb 25, 2025

Copy link
Copy Markdown
Member

The buildbot workers used on GitHub are called "... PR", not "... 3.x".

All reactions

Sorry, something went wrong.

Uh oh!

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

@vstinner

vstinner commented Feb 25, 2025

Copy link
Copy Markdown
Member

Python built with gcc -Og has a cfunction_vectorcall_O() function in the call stack, and so py-bt is able to find the <built-in method id call:

(gdb) where
#0  builtin_id (self=0x7ffff7c7d8b0, v=v@entry=42) at Python/bltinmodule.c:1310
#1  0x00000000004d9ce1 in cfunction_vectorcall_O (func=<built-in method id of module object at remote 0x7ffff7c7d8b0>, args=<optimized out>, 
    nargsf=<optimized out>, kwnames=<optimized out>) at Objects/methodobject.c:523
#2  0x000000000048b12a in _PyObject_VectorcallTstate (tstate=0x9a9950 <_PyRuntime+329808>, 
    callable=callable@entry=<built-in method id of module object at remote 0x7ffff7c7d8b0>, args=args@entry=0x7ffff7fb3228, nargsf=9223372036854775809, 
    kwnames=kwnames@entry=0x0) at ./Include/internal/pycore_call.h:167
#3  0x000000000048b1f2 in PyObject_Vectorcall (callable=callable@entry=<built-in method id of module object at remote 0x7ffff7c7d8b0>, 
    args=args@entry=0x7ffff7fb3228, nargsf=<optimized out>, kwnames=kwnames@entry=0x0) at Objects/call.c:327
#4  0x00000000005a8cfc in _PyEval_EvalFrameDefault (tstate=0x9a9950 <_PyRuntime+329808>, frame=0x7ffff7fb31c0, throwflag=0)
    at Python/generated_cases.c.h:1371

(gdb) py-bt
Traceback (most recent call first):
  <built-in method id of module object at remote 0x7ffff7c7d8b0>
  File "/home/vstinner/python/main/Lib/test/test_gdb/gdb_sample.py", line 10, in baz
    id(42)
  File "/home/vstinner/python/main/Lib/test/test_gdb/gdb_sample.py", line 7, in bar
    baz(a, b, c)
  File "/home/vstinner/python/main/Lib/test/test_gdb/gdb_sample.py", line 4, in foo
    bar(a=a, b=b, c=c)
  File "/home/vstinner/python/main/Lib/test/test_gdb/gdb_sample.py", line 12, in <module>
    foo(1, 2, 3)

Python built with clang -Og has no cfunction_vectorcall_O() function in the call stack, and py-bt fails to find the id() call:

#0  builtin_id (self=<optimized out>, v=42) at Python/bltinmodule.c:1311
#1  0x000000000048f894 in _PyObject_VectorcallTstate (tstate=0xa07840 <_PyRuntime+329808>, 
    callable=<built-in method id of module object at remote 0x7ffff7c7d8b0>, args=0x9beaa8 <_PyRuntime+31416>, 
    nargsf=0, kwnames=0x0) at ./Include/internal/pycore_call.h:167
#2  0x00000000006002a5 in _PyEval_EvalFrameDefault (tstate=0xa07840 <_PyRuntime+329808>, frame=0x7ffff7fb31c0, 
    throwflag=0) at Python/generated_cases.c.h:1371

(gdb) py-bt
Traceback (most recent call first):
  File "/home/vstinner/python/main/Lib/test/test_gdb/gdb_sample.py", line 10, in baz
    id(42)
  File "/home/vstinner/python/main/Lib/test/test_gdb/gdb_sample.py", line 7, in bar
    baz(a, b, c)
  File "/home/vstinner/python/main/Lib/test/test_gdb/gdb_sample.py", line 4, in foo
    bar(a=a, b=b, c=c)
  File "/home/vstinner/python/main/Lib/test/test_gdb/gdb_sample.py", line 12, in <module>
    foo(1, 2, 3)

_PyObject_VectorcallTstate() is built differently:

  • GCC: call 0x48aed2 <_PyObject_MakeTpCall>
  • clang: jmp 0x48fcc0 <_PyObject_MakeTpCall>
All reactions

Sorry, something went wrong.

Uh oh!

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

@markshannon

markshannon commented Feb 25, 2025

Copy link
Copy Markdown
Member Author

Yes. Clang is tailcalling from cfunction_vectorcall_O so it isn't left on the stack.

But what do we do about the gdb tests?

All reactions

Sorry, something went wrong.

Uh oh!

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

@vstinner

vstinner commented Feb 25, 2025

Copy link
Copy Markdown
Member

But what do we do about the gdb tests?

support.python_is_optimized() should return True if Python was built with clang -Og.

In my experience, python-gdb.py is not reliable with any optimization level higher than -O0 (disable all optimizations). On Fedora, python3-debug is built with gcc -O0 to work well in gdb.

markshannon 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
Treat clang -Og as optimized for gdb tests
909fe79
@markshannon
markshannon force-pushed the fix-gdb-test branch from c9ab78d to 909fe79 Compare February 25, 2025 17:22
@markshannon

markshannon commented Feb 25, 2025

Copy link
Copy Markdown
Member Author

!buildbot AMD64.*Fedora.*Stable.*Clang

All reactions

Sorry, something went wrong.

Uh oh!

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

@bedevere-bot

bedevere-bot commented Feb 25, 2025

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @markshannon for commit 909fe79 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F130550%2Fmerge

The command will test the builders whose names match following regular expression: AMD64.*Fedora.*Stable.*Clang

The builders matched are:

  • AMD64 Fedora Stable Clang Installed PR
  • AMD64 Fedora Stable Clang PR
All reactions

Sorry, something went wrong.

Uh oh!

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

@brandtbucher

brandtbucher commented Feb 25, 2025

Copy link
Copy Markdown
Member

I'm confused. Does Clang still optimize even with -Og?

All reactions

Sorry, something went wrong.

Uh oh!

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

@markshannon

markshannon commented Feb 25, 2025

Copy link
Copy Markdown
Member Author

It depends what you mean by "optimize".
For good performance, no.
Enough to confuse gdb, yes.

All reactions

Sorry, something went wrong.

Uh oh!

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

@markshannon
Merge branch 'main' into fix-gdb-test
4df96fc
@brandtbucher

brandtbucher commented Feb 25, 2025

Copy link
Copy Markdown
Member

Weird, I thought the whole point of -Og was "don't confuse gdb".

Maybe for Clang it just means "don't confuse lldb".

All reactions

Sorry, something went wrong.

Uh oh!

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

brandtbucher
brandtbucher approved these changes Feb 25, 2025
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Feb 25, 2025
@vstinner

vstinner commented Feb 25, 2025 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Weird, I thought the whole point of -Og was "don't confuse gdb".

That's an old debate. In my experience with GCC and clang, -Og does optimize way too much for gdb which often fails to read function local variables and arguments ("<optimized out>").

All reactions

Sorry, something went wrong.

Uh oh!

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

@markshannon markshannon changed the title GH-130396: Relax regex for gdb test GH-130396: Broaden definition of "optimized" for gdb tests Feb 26, 2025
@markshannon

markshannon commented Feb 26, 2025

Copy link
Copy Markdown
Member Author

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78685

vstinner 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 merged commit 129db32 into python:main Feb 26, 2025
@bedevere-app bedevere-app Bot removed the awaiting merge label Feb 26, 2025
@vstinner vstinner added needs backport to 3.12 only security fixes needs backport to 3.13 only security fixes labels Feb 26, 2025
@miss-islington-app

miss-islington-app Bot commented Feb 26, 2025

Copy link
Copy Markdown

Thanks @markshannon for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

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

@miss-islington-app

miss-islington-app Bot commented Feb 26, 2025

Copy link
Copy Markdown

Thanks @markshannon for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

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

@miss-islington-app

miss-islington-app Bot commented Feb 26, 2025

Copy link
Copy Markdown

Sorry, @markshannon, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 129db32d6f2d7f450d2741da6a222c18e458c61b 3.12
All reactions

Sorry, something went wrong.

Uh oh!

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

@miss-islington-app miss-islington-app Bot assigned markshannon Feb 26, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 26, 2025
@markshannon @miss-islington
pythonGH-130396: Treat clang -Og as optimized for gdb tests (pythonGH…
e85905c
…-130550)

(cherry picked from commit 129db32)

Co-authored-by: Mark Shannon <[email protected]>
@bedevere-app

bedevere-app Bot commented Feb 26, 2025

Copy link
Copy Markdown

GH-130572 is a backport of this pull request to the 3.13 branch.

All reactions

Sorry, something went wrong.

Uh oh!

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

@bedevere-app bedevere-app Bot removed the needs backport to 3.13 only security fixes label Feb 26, 2025
@vstinner vstinner removed the needs backport to 3.12 only security fixes label Feb 26, 2025
@bedevere-app

bedevere-app Bot commented Feb 26, 2025

Copy link
Copy Markdown

GH-130573 is a backport of this pull request to the 3.12 branch.

All reactions

Sorry, something went wrong.

Uh oh!

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

@vstinner

vstinner commented Feb 26, 2025

Copy link
Copy Markdown
Member

I backported the test_gdb fix to 3.12 and 3.13 branches.

All reactions

Sorry, something went wrong.

Uh oh!

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

@markshannon
markshannon deleted the fix-gdb-test branch February 26, 2025 11:20
vstinner pushed a commit that referenced this pull request Feb 26, 2025
@miss-islington @markshannon
[3.13] GH-130396: Treat clang -Og as optimized for gdb tests (GH-130550…
43a2a37
…) (#130572)

GH-130396: Treat clang -Og as optimized for gdb tests (GH-130550)
(cherry picked from commit 129db32)

Co-authored-by: Mark Shannon <[email protected]>
vstinner added a commit that referenced this pull request Feb 26, 2025
@vstinner @markshannon
[3.12] GH-130396: Treat clang -Og as optimized for gdb tests (GH-130550…
c4aeb4c
…) (#130573)

GH-130396: Treat clang -Og as optimized for gdb tests (GH-130550)

(cherry picked from commit 129db32)

Co-authored-by: Mark Shannon <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 26, 2025
@vstinner @markshannon @miss-islington
[3.12] pythonGH-130396: Treat clang -Og as optimized for gdb tests (p…
40b2e6b
…ythonGH-130550) (pythonGH-130573)

pythonGH-130396: Treat clang -Og as optimized for gdb tests (pythonGH-130550)

(cherry picked from commit 129db32)
(cherry picked from commit c4aeb4c)

Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: Mark Shannon <[email protected]>
@miss-islington miss-islington mentioned this pull request Feb 26, 2025
[3.11] GH-130396: Treat clang -Og as optimized for gdb tests (GH-130550) (GH-130573) #130593
Merged
@vstinner vstinner mentioned this pull request Feb 26, 2025
PPC64LE Fedora Stable Clang 3.x fails test_gdb #104736
Closed
pablogsal pushed a commit that referenced this pull request Feb 27, 2025
@miss-islington
[3.11] GH-130396: Treat clang -Og as optimized for gdb tests (GH-130550…
f1897d3
…) (GH-130573) (#130593)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brandtbucher brandtbucher brandtbucher approved these changes

Uh oh!

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

Assignees

@markshannon markshannon

Labels

skip news tests Tests in the Lib/test dir

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

@markshannon @bedevere-bot @vstinner @brandtbucher
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 01402520 — 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.