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 segfaults with debug mode code on 64-bit Arm #3875

Merged
merged 1 commit into from
Oct 2, 2021
Merged

Conversation

SeanTAllen
Copy link
Member

While getting pony working on 64-bit Arm, we found a bug where the
test-stdlib-debug tests would segfault. Everything worked fine with release
builds, but debug builds would crash.

After investigation, we've found that in codegen_machine in host.cc if the
opt_level is get to either CodeGenOpt::Default or CodeGenOpt::Aggressive
instead of CodeGenOpt::None the problem goes away.

It seems likely that this is an LLVM bug, but that hasn't been established.
We've committed a fix in host.cc that when the target is arm (which is
64-bit arm) that we use CodeGenOpt::Default. This is far from ideal, but gets
us working compilations until we can investigate further.

The end result of this should either be a patch that we upstream to LLVM or
possibly, the discovery that we are doing something wrong with our Arm code
generation that optimization manages to fix.

@SeanTAllen SeanTAllen requested review from ergl and a team October 2, 2021 20:39
@SeanTAllen SeanTAllen added the changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge label Oct 2, 2021
@ponylang-main
Copy link
Contributor

Hi @SeanTAllen,

The changelog - fixed label was added to this pull request; all PRs with a changelog label need to have release notes included as part of the PR. If you haven't added release notes already, please do.

Release notes are added by creating a uniquely named file in the .release-notes directory. We suggest you call the file 3875.md to match the number of this pull request.

The basic format of the release notes (using markdown) should be:

## Title

End user description of changes, why it's important,
problems it solves etc.

If a breaking change, make sure to include 1 or more
examples what code would look like prior to this change
and how to update it to work after this change.

Thanks.

While getting pony working on 64-bit Arm, we found a bug where the
`test-stdlib-debug` tests would segfault. Everything worked fine with `release`
builds, but `debug` builds would crash.

After investigation, we've found that in `codegen_machine` in `host.cc` if the
`opt_level` is get to either `CodeGenOpt::Default` or `CodeGenOpt::Aggressive`
 instead of `CodeGenOpt::None` the problem goes away.

It seems likely that this is an LLVM bug, but that hasn't been established.
We've committed a fix in `host.cc` that when the target is `arm` (which is
64-bit arm) that we use `CodeGenOpt::Default`. This is far from ideal, but gets
us working compilations until we can investigate further.

The end result of this should either be a patch that we upstream to LLVM or
possibly, the discovery that we are doing something wrong with our Arm code
generation that optimization manages to fix.
@ergl ergl merged commit 6ca10a3 into main Oct 2, 2021
@ergl ergl deleted the arm64-debug-fix branch October 2, 2021 22:18
github-actions bot pushed a commit that referenced this pull request Oct 2, 2021
github-actions bot pushed a commit that referenced this pull request Oct 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants