gh-118518: Allow perf to work without frame pointers #112254
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I need to clean this up a bit after the latest changes :( Also this is missing docs. |
997b082 to
e2c0d7f
Compare
pablogsal
commented
Nov 19, 2023
9a00d1f to
b2ab074
Compare
|
!buildbot perf |
|
🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 7cbd394 🤖 The command will test the builders whose names match following regular expression: The builders matched are:
|
|
🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 7cbd394 🤖 If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
|
Hi, Linux perf supports DWARF unwinding using either libunwind or libdw. And they might have different quality of supports. Do you know which one you used (and worked)? And please share the problems or symptoms you are seeing in more detail, thanks! |
|
Thanks a lot for the help @namhyung
Do you know how can I check this? The closest I got is to check symbols in thee binary distributed with the distro: from that looks like is using
To illustrate the problems I am going to execute the following steps in an Consider this Python file: Running perf with this integration activated with a Python compiled with no frame pointers (the default) over the file: This produces a bunch of elf files that look sane. For example The symbols are correctly placed in the symbol table: The eh_frame is there and empty as expeected: But unfortunately perf doesn't connect the trampolines. For example: Notice that there are a bunch of but if you inspect those shared objects hey look ok: Other things to notice:
|
Try |
|
Let's do this one by one. So on Ubuntu, it basically gets the stack trace correctly but no symbols in the python trampolines, right? One thing I noticed is warnings in the readelf output for the symbol table. I don't know what .symtab.sh_info means. One thing I can find so far is the Solaris Linker and Libraries Guide:
But I'm not sure if it's the case for Linux as my test binary has mixed global and local symbols. |
|
Also, I'm curious how exactly the JIT code maps to the address. Can you please run this? |
|
Ok, here is the information for Ubuntu: |
|
Here is the data for Arch Linux (which is the distro where everything works as expected): And here is one of the ELF objects being created: |
|
!buildbot perf |
|
🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 1de613e 🤖 The command will test the builders whose names match following regular expression: The builders matched are:
|
|
I'll take a look and let you know. |
|
The fixes were added to v6.8 originally and then backported to v6.7.2. So and kernel versions later would be fine. But the more important ones are long-term stable (LTS) versions which are v6.6 and v6.1. I've checked v6.6.14 has the fixes but v6.1.x doesn't. I asked the stable team to add the fixes to v6.1 already. I'll let you know once it's done and I got the version number. Again, this is only perf tool issue and nothing related to the kernel. But the perf tools just use the same version numbers because it's maintained in the same source tree. |
|
|
|
|
|
|
|
|
|
|
|
On it |
|
#118592 for the builedbot failure |
|
@namhyung I found another problem. If that shows: with libunwind-enabled perf and with perf without libunwind support. Any idea of what may be going on? |
|
Hummmm, wait, this may be something with the versions. I tried to compile the current version on main from the kernel git repo (commit dd5a440a31fae6e459c0d6271dddd62825505361) and that works with libunwind, but the perf version shipped in Arch Linux (perf version 6.7-3) doesn't. Can you confirm that this version should have the fix? |
|
Ah, nevermind, all good. Seems that the version in arch linux it's using the v6.7 tag: which I suppose it's before 6.7.2 (I was confused by the weird output of |
|
Yeah, the version number after |
SonicField
pushed a commit
to SonicField/cpython
that referenced
this pull request
May 8, 2024
|
v6.1.91 added the fix. |
That's fantastic! |
|
make[2]: Entering directory '/home/abuild/rpmbuild/BUILD/Python-3.13.0b3' |
| DWRF_U8(DWRF_CFA_def_cfa_offset); DWRF_UV(8); | ||
| /* Extra registers saved for JIT-compiled code. */ | ||
| #elif defined(__aarch64__) && defined(__AARCH64EL__) && !defined(__ILP32__) | ||
| DWRF_U8(DWRF_CFA_advance_loc | 1); |
There was a problem hiding this comment.
That can't be correct since on aarch64 every insn is 4-byte aligned.
There was a problem hiding this comment.
It's correct as the code alignment factor on aarch64 is 4 instead of 1 such as on x86_64.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Uh oh!
There was an error while loading. Please reload this page.