gh-101525: Skip test_gdb if the binary is relocated by BOLT. #118572

corona10 · 2024-05-04T13:57:35Z

Issue: Make BOLT as stable feature #101525

corona10 · 2024-05-04T13:58:05Z

Now the all tests are fully passed with BOLTed binary.

aaupov · 2024-05-05T02:25:38Z

Lib/test/support/__init__.py

+    binary_path = sys.executable
+    with open(binary_path, 'rb') as f:
+        binary = f.read()
+        if b'.note.bolt_info' in binary:


Can you call readelf -e <binary_path> and grep .note.bolt_info? Checking headers is efficient and robust.

I also think it will be the best way to handle this issue. But I am not sure it will be okay to call external binary for this. I may need to get opinions from @vstinner and @erlend-aasland WDYT?

Why not checking sysconfig? This check looks slow and not reliable.

Why not checking sysconfig? This check looks slow and not reliable.

Do you have any recommendation points? CFLAG does not guarantee the build is BOLTed.

Maybe test:

'--enable-bolt' in sysconfig.get_config_var('CONFIG_ARGS')

Only put the code in test_gdb if you're worried that the the test is not reliable. And add a comment to explain it.

Well then there is no way to display the optimization information but okay got it.

You cannot expect "readelf" to be available, it would be a new dependency. I dislike looking for a string in a binary without parsing the ELF binary.

We call external binaries in test.support to determine macOS features, so there is precedent for doing that sort of thing; is readelf a standalone package, or is it part of the compiler toolchain?

; is readelf a standalone package, or is it part of the compiler toolchain?

AFAIK, it's part of binutils: https://sourceware.org/binutils/docs/binutils/readelf.html

AFAIK, it's part of binutils: https://sourceware.org/binutils/docs/binutils/readelf.html

Thanks. binutils is preinstalled on the Ubuntu runners GitHub provides, so there is no more dependencies for the CI there. Also, anyone who wants to build their own CPython probably already did apt install build-essential, which includes binutils. Looks to me like there are no more deps introduced. What about macOS?

Lib/test/libregrtest/utils.py

aaupov · 2024-05-06T15:11:47Z

By the way, since -update-debug-sections is used (https://github.com/python/cpython/blob/main/configure.ac#L2185), do you really need to skip gdb tests?

corona10 · 2024-05-06T15:16:14Z

By the way, since -update-debug-sections is used (https://github.com/python/cpython/blob/main/configure.ac#L2185), do you really need to skip gdb tests?

Well, when I run a test_gdb with BOLTed binary, sometimes it fails, well let me try with other systems too.

vstinner

LGTM

vstinner · 2024-05-07T06:40:22Z

I suggest to backport the change to 3.12, and maybe also to 3.11.

erlend-aasland · 2024-05-07T07:39:25Z

Lib/test/support/__init__.py

@@ -856,6 +856,14 @@ def check_cflags_pgo():
    return any(option in cflags_nodist for option in pgo_options)


+def check_bolt_optimized():
+    # Always return false, if the platform is WASI.


This comment only says what the code does, but not why. Could you rewrite it to state why this is happening? I can easily see by the code that we always return false if we're on WASI; there is no need to manifest that info into a comment ;)

corona10 · 2024-05-08T00:53:09Z

I suggest to backport the change to 3.12, and maybe also to 3.11.

@vstinner @erlend-aasland
BOLT is the experimental feature from 3.12 :)
Anyway, I will finalize this PR during the PyCon US. This week, I am too busy with all my tasks at my full-time job before the conference.

corona10 · 2024-08-31T16:55:45Z

@vstinner @erlend-aasland I've updated the staled PR, Can you please take a look again?

vstinner

LGTM

Misc/NEWS.d/next/Tests/2024-05-04-22-56-41.gh-issue-101525.LHK166.rst

vstinner

LGTM

miss-islington-app · 2024-09-02T13:24:56Z

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

…ythongh-118572) (cherry picked from commit f95fc4d) Co-authored-by: Donghee Na <donghee.na@python.org>

miss-islington-app · 2024-09-02T13:25:12Z

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

cherry_picker f95fc4de115ae03d7aa6dece678240df085cb4f6 3.12

bedevere-app · 2024-09-02T13:25:14Z

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

…h-118572) (#123601) gh-101525: Skip test_gdb if the binary is relocated by BOLT. (gh-118572) (cherry picked from commit f95fc4d) Co-authored-by: Donghee Na <donghee.na@python.org>

…OLT. (pythongh-118572) (cherry picked from commit f95fc4d) Co-authored-by: Donghee Na <donghee.na@python.org>

bedevere-app · 2024-09-02T14:19:19Z

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

#123603) [3.12] gh-101525: Skip test_gdb if the binary is relocated by BOLT. (gh-118572) (cherry picked from commit f95fc4d)

corona10 requested review from vstinner and erlend-aasland May 4, 2024 13:57

bedevere-app bot added the awaiting core review label May 4, 2024

bedevere-app bot mentioned this pull request May 4, 2024

Make BOLT as stable feature #101525

Open

7 tasks

aaupov reviewed May 5, 2024

View reviewed changes

vstinner reviewed May 6, 2024

View reviewed changes

Lib/test/libregrtest/utils.py Show resolved Hide resolved

corona10 requested a review from vstinner May 6, 2024 13:33

corona10 changed the title ~~gh-101525: Skip test_gdb if the binary is BOLTed.~~ [WIP] gh-101525: Skip test_gdb if the binary is BOLTed. May 6, 2024

corona10 changed the title ~~[WIP] gh-101525: Skip test_gdb if the binary is BOLTed.~~ gh-101525: Skip test_gdb if the binary is BOLTed. May 6, 2024

vstinner approved these changes May 7, 2024

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting core review labels May 7, 2024

erlend-aasland reviewed May 7, 2024

View reviewed changes

corona10 added 9 commits September 1, 2024 01:48

pythongh-101525: Skip test_gdb if the binary is BOLTed.

8755316

Address code review

973abd6

nit

83d879c

Address code review

af9c391

Address code review

e9ec9b9

fix

2d6d05a

nit

b854706

fix

c0bd894

Address code review

ea13917

corona10 force-pushed the gh-101525-test branch from d77cc23 to ea13917 Compare August 31, 2024 16:55

corona10 added the needs backport to 3.12 only security fixes label Aug 31, 2024

corona10 added the needs backport to 3.13 bugs and security fixes label Aug 31, 2024

vstinner approved these changes Sep 2, 2024

View reviewed changes

Misc/NEWS.d/next/Tests/2024-05-04-22-56-41.gh-issue-101525.LHK166.rst Outdated Show resolved Hide resolved

Address code review

0e9d019

corona10 commented Sep 2, 2024

View reviewed changes

Misc/NEWS.d/next/Tests/2024-05-04-22-56-41.gh-issue-101525.LHK166.rst Show resolved Hide resolved

corona10 enabled auto-merge (squash) September 2, 2024 13:01

corona10 changed the title ~~gh-101525: Skip test_gdb if the binary is BOLTed.~~ gh-101525: Skip test_gdb if the binary is relocated by BOLT. Sep 2, 2024

corona10 disabled auto-merge September 2, 2024 13:02

corona10 enabled auto-merge (squash) September 2, 2024 13:02

vstinner approved these changes Sep 2, 2024

View reviewed changes

corona10 merged commit f95fc4d into python:main Sep 2, 2024
42 checks passed

bedevere-app bot removed the awaiting merge label Sep 2, 2024

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 2, 2024

pythongh-101525: Skip test_gdb if the binary is relocated by BOLT. (p…

da1b359

…ythongh-118572) (cherry picked from commit f95fc4d) Co-authored-by: Donghee Na <donghee.na@python.org>

miss-islington-app bot assigned corona10 Sep 2, 2024

bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Sep 2, 2024

corona10 added a commit to corona10/cpython that referenced this pull request Sep 2, 2024

[3.12] pythongh-101525: Skip test_gdb if the binary is relocated by B…

1ba223c

…OLT. (pythongh-118572) (cherry picked from commit f95fc4d) Co-authored-by: Donghee Na <donghee.na@python.org>

corona10 mentioned this pull request Sep 2, 2024

[3.12] gh-101525: Skip test_gdb if the binary is relocated by BOLT. (… #123603

Merged

bedevere-app bot removed the needs backport to 3.12 only security fixes label Sep 2, 2024

corona10 added a commit that referenced this pull request Sep 2, 2024

[3.12] gh-101525: Skip test_gdb if the binary is relocated by BOLT. (… (

4514998

#123603) [3.12] gh-101525: Skip test_gdb if the binary is relocated by BOLT. (gh-118572) (cherry picked from commit f95fc4d)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-101525: Skip test_gdb if the binary is relocated by BOLT. #118572

gh-101525: Skip test_gdb if the binary is relocated by BOLT. #118572

corona10 commented May 4, 2024 •

edited by bedevere-app bot

Loading

corona10 commented May 4, 2024

aaupov May 5, 2024

corona10 May 5, 2024

vstinner May 5, 2024

corona10 May 5, 2024

vstinner May 5, 2024

vstinner May 5, 2024

corona10 May 5, 2024 •

edited

Loading

erlend-aasland May 6, 2024

corona10 May 6, 2024 •

edited

Loading

erlend-aasland May 6, 2024

aaupov commented May 6, 2024

corona10 commented May 6, 2024

vstinner left a comment

vstinner commented May 7, 2024

erlend-aasland May 7, 2024

corona10 commented May 8, 2024 •

edited

Loading

corona10 commented Aug 31, 2024 •

edited

Loading

vstinner left a comment

vstinner left a comment

miss-islington-app bot commented Sep 2, 2024

miss-islington-app bot commented Sep 2, 2024

bedevere-app bot commented Sep 2, 2024

bedevere-app bot commented Sep 2, 2024

gh-101525: Skip test_gdb if the binary is relocated by BOLT. #118572

gh-101525: Skip test_gdb if the binary is relocated by BOLT. #118572

Conversation

corona10 commented May 4, 2024 • edited by bedevere-app bot Loading

corona10 commented May 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

corona10 May 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

corona10 May 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aaupov commented May 6, 2024

corona10 commented May 6, 2024

vstinner left a comment

Choose a reason for hiding this comment

vstinner commented May 7, 2024

Choose a reason for hiding this comment

corona10 commented May 8, 2024 • edited Loading

corona10 commented Aug 31, 2024 • edited Loading

vstinner left a comment

Choose a reason for hiding this comment

vstinner left a comment

Choose a reason for hiding this comment

miss-islington-app bot commented Sep 2, 2024

miss-islington-app bot commented Sep 2, 2024

bedevere-app bot commented Sep 2, 2024

bedevere-app bot commented Sep 2, 2024

corona10 commented May 4, 2024 •

edited by bedevere-app bot

Loading

corona10 May 5, 2024 •

edited

Loading

corona10 May 6, 2024 •

edited

Loading

corona10 commented May 8, 2024 •

edited

Loading

corona10 commented Aug 31, 2024 •

edited

Loading