Skip to content
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.

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

Fix misunwind during Profile test under rr #39553

Merged
merged 1 commit into from
Feb 9, 2021
Merged

Fix misunwind during Profile test under rr #39553

merged 1 commit into from
Feb 9, 2021

Conversation

Keno
Copy link
Member

@Keno Keno commented Feb 6, 2021

See the included patch for details. In short, libunwind has a bug that causes Profile test failures. Unfortunately, since I'm not seeing the issue locally, I cannot say whether this actually fixes it, so I added a second commit that just builds libunwind locally on the buildbot and hopefully if we run this a couple of times, we'll find out if it works or not. If it does, we can submit this upstream and spin a Yggdrasil release.

@Keno Keno assigned vtjnash and unassigned vtjnash Feb 6, 2021
@Keno Keno requested a review from vtjnash February 6, 2021 23:34
@Keno
Copy link
Member Author

Keno commented Feb 6, 2021

Actually, I was able to reproduce the original problem in a docker container, and this does seem to fix it, so I'll go ahead and go work on the Yggdrasil bump.

Keno added a commit to JuliaPackaging/Yggdrasil that referenced this pull request Feb 6, 2021
@Keno Keno force-pushed the kf/unwindcfarsp branch 2 times, most recently from 44aa9ef to 0fef25f Compare February 7, 2021 00:38
@Keno
Copy link
Member Author

Keno commented Feb 7, 2021

Updated the patch. Hopefully that should fix the backtrace quality regression of the previous patch while keeping the Profile fix.

@Keno Keno force-pushed the kf/unwindcfarsp branch 2 times, most recently from a7ce2de to b2ad240 Compare February 7, 2021 00:47
@Keno
Copy link
Member Author

Keno commented Feb 8, 2021

I'm gonna go with this method of CI'ing doesn't work. Works fine locally with the patch applied as well as with a BB-built version of the binary, but not with the one from the buildbot - I assume the corruption has something to do with it. I'll just bump BB and let's try that.

Keno added a commit to JuliaPackaging/Yggdrasil that referenced this pull request Feb 8, 2021
Keno added a commit to JuliaPackaging/Yggdrasil that referenced this pull request Feb 8, 2021
@Keno Keno changed the title [DO NOT MERGE] Fix misunwind during Profile test under rr Fix misunwind during Profile test under rr Feb 8, 2021
@Keno
Copy link
Member Author

Keno commented Feb 8, 2021

Freebsd issue turned out to be the fallback unwinder (I guess some library on freebsd doesn't have DWARF info), which needs to be updated to this change. Aarch64 issue, I didn't debug, but I'll just drop the non-x86_64 changes here since we don't (currently) run under rr on aarch64.

Keno added a commit to JuliaPackaging/Yggdrasil that referenced this pull request Feb 8, 2021
Also drop changes for other architectures (c.f. JuliaLang/julia#39553).
Keno added a commit to JuliaPackaging/Yggdrasil that referenced this pull request Feb 8, 2021
Also drop changes for other architectures (c.f. JuliaLang/julia#39553).
Keno added a commit to JuliaPackaging/Yggdrasil that referenced this pull request Feb 8, 2021
Also drop changes for other architectures (c.f. JuliaLang/julia#39553).
Keno added a commit to JuliaPackaging/Yggdrasil that referenced this pull request Feb 8, 2021
Also drop changes for other architectures (c.f. JuliaLang/julia#39553).
@kshyatt kshyatt added the test This change adds or pertains to unit tests label Feb 8, 2021
Keno added a commit that referenced this pull request Feb 9, 2021
I noticed in #39553 that JIT unwind info didn't actually
work on FreeBSD. As a general policy, we should register our frame info to make sure
that our unwind is accurate. Libunwind has some heuristics to fall back to, but they
can break e.g. for large stack frames.
Libunwind improperly aliases RSP and CFA, which are separate concepts.
Fix that.
@Keno Keno added the ci Continuous integration label Feb 9, 2021
@Keno Keno merged commit bbf7f97 into master Feb 9, 2021
@Keno Keno deleted the kf/unwindcfarsp branch February 9, 2021 13:57
vtjnash pushed a commit that referenced this pull request Feb 9, 2021
I noticed in #39553 that JIT unwind info didn't actually
work on FreeBSD. As a general policy, we should register our frame info to make sure
that our unwind is accurate. Libunwind has some heuristics to fall back to, but they
can break e.g. for large stack frames.
@Keno Keno added the backport 1.6 Change should be backported to release-1.6 label Feb 11, 2021
@vtjnash
Copy link
Member

vtjnash commented Feb 16, 2021

This is broken on i686. Looks like the failures appeared on earlier runs of the PR, but, awkwardly, tester_linux32 never ran on the final update, so it has 17 green marks without showing the 1 red.

KristofferC pushed a commit that referenced this pull request Feb 17, 2021
Libunwind improperly aliases RSP and CFA, which are separate concepts.
Fix that.

(cherry picked from commit bbf7f97)
@KristofferC KristofferC mentioned this pull request Feb 17, 2021
52 tasks
@KristofferC KristofferC removed the backport 1.6 Change should be backported to release-1.6 label Mar 14, 2021
ElOceanografo pushed a commit to ElOceanografo/julia that referenced this pull request May 4, 2021
Libunwind improperly aliases RSP and CFA, which are separate concepts.
Fix that.
ElOceanografo pushed a commit to ElOceanografo/julia that referenced this pull request May 4, 2021
I noticed in JuliaLang#39553 that JIT unwind info didn't actually
work on FreeBSD. As a general policy, we should register our frame info to make sure
that our unwind is accurate. Libunwind has some heuristics to fall back to, but they
can break e.g. for large stack frames.
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this pull request May 9, 2021
Libunwind improperly aliases RSP and CFA, which are separate concepts.
Fix that.
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this pull request May 9, 2021
I noticed in JuliaLang#39553 that JIT unwind info didn't actually
work on FreeBSD. As a general policy, we should register our frame info to make sure
that our unwind is accurate. Libunwind has some heuristics to fall back to, but they
can break e.g. for large stack frames.
staticfloat pushed a commit that referenced this pull request Dec 23, 2022
Libunwind improperly aliases RSP and CFA, which are separate concepts.
Fix that.

(cherry picked from commit bbf7f97)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Continuous integration test This change adds or pertains to unit tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants