8000 GH-130396: Broaden definition of "optimized" for gdb tests by markshannon · Pull Request #130550 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

GH-130396: Broaden definition of "optimized" for gdb tests #130550

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Feb 26, 2025
Merged

GH-130396: Broaden definition of "optimized" for gdb tests #130550

merged 2 commits into from
Feb 26, 2025

Conversation

markshannon
Copy link
Member
@markshannon markshannon commented Feb 25, 2025 •
edited by bedevere-app bot
Loading

@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
Open
@markshannon
Copy link
Member Author

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

@bedevere-bot
Copy link

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

@markshannon
Copy link
Member Author

!buildbot Fedora.*Clang

@bedevere-bot
Copy link

🤖 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

@markshannon
Copy link
Member Author

Let's try again

!buildbot Fedora.*Clang

@markshannon markshannon marked this pull request as ready for review February 25, 2025 15:49
@markshannon
Copy link
Member Author

Once more for luck 🙂

!buildbot AMD.*Fedora.*Clang

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

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

@vstinner
Copy link
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>

@markshannon
Copy link
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?

@vstinner
Copy link
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
Copy link
Member Author

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

@bedevere-bot
Copy link

🤖 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

@brandtbucher
Copy link
Member

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

@markshannon
Copy link
Member Author

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

@brandtbucher
Copy link
Member

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

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

@vstinner
Copy link
Member
vstinner commented Feb 25, 2025 •
edited
Loading

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>").

@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
Copy link
Member Author

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

@markshannon markshannon merged commit 129db32 into python:main Feb 26, 2025
42 checks passed
@vstinner vstinner added needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Feb 26, 2025
@miss-islington-app
Copy link

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

@miss-islington-app
Copy link

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

@miss-islington-app
Copy link

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

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 <mark@hotpy.org>
@bedevere-app
Copy link
bedevere-app bot commented Feb 26, 2025

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

6D40
@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Feb 26, 2025
@vstinner vstinner removed the needs backport to 3.12 only security fixes label Feb 26, 2025
@bedevere-app
Copy link
bedevere-app bot commented Feb 26, 2025

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

@vstinner
Copy link
Member

I backported the test_gdb fix to 3.12 and 3.13 branches.

@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 <mark@hotpy.org>
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 <mark@hotpy.org>
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 <vstinner@python.org>
Co-authored-by: Mark Shannon <mark@hotpy.org>
@miss-islington miss-islington mentioned this pull request Feb 26, 2025
Merged
@vstinner vstinner mentioned this pull request Feb 26, 2025
Open
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)
seehwan pushed a commit to seehwan/cpython that referenced this pull request Apr 16, 2025
@markshannon @seehwan80
pythonGH-130396: Treat clang -Og as optimized for gdb tests (pythonGH…
9f26920 
…-130550)
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

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.
4 participants
@markshannon @bedevere-bot @vstinner @brandtbucher
0