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

Track upstreaming llvm-libunwind patches #72655

Open
am11 opened this issue Jul 22, 2022 · 5 comments
Open

Track upstreaming llvm-libunwind patches #72655

am11 opened this issue Jul 22, 2022 · 5 comments
Labels
area-Infrastructure-coreclr tracking-external-issue The issue is caused by external problem (e.g. OS) - nothing we can do to fix it directly
Milestone

Comments

@am11
Copy link
Member

am11 commented Jul 22, 2022

We have made several llvm libunwind patches for NativeAOT, since its initial bring up: dotnet/corert#2293. The collective patches are in the following two commits on top of upstream's latest version (as of this moment) 14.0.6 release:

Apply https://github.com/dotnet/runtime/commit/e57194552327bccaf2b45a9190f0e6e1289472e4
Apply https://github.com/dotnet/runtime/commit/db7ed089a9e20a391a53e0b547f95dc8bf916765

It would be great (on multiple fronts) if we upstream those patches via LLVM code review platform of choice; Phabricator (https://llvm.org/docs/Phabricator.html). It will require some additional work for other architectures which llvm-libunwind supports (such as s390x and RISCV), to make them ready to upstream. At the moment, it is relatively easier to upstream patches since we are on the latest release.


unw_get_save_loc API

One of the key additions in our local patches is the implementation of unw_get_save_loc, which was scoped to memory locations, but does not cover register locations. The original commit was dotnet/corert@c6571c6 and v14.0.6 state can be extracted from e571945.

Moreover, unw_get_save_loc is the API which we test in coreclr PAL introspection code (src/coreclr/pal/src/configure.cmake), and based on which we decide whether to use the pinning fallback in GC. If the memory-location-only patch gets upstreamed (without first implementing register locations support), we would probably need to improve our introspection to detect the exact capability we need out of this API for GC; currently HP libunwind (the recommended variant for coreclr PAL) has support for both memory and registers location, which eventually gives us granular control over object lifetime in the GC.

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jul 22, 2022
@am11 am11 added tracking-external-issue The issue is caused by external problem (e.g. OS) - nothing we can do to fix it directly area-Infrastructure-coreclr labels Jul 22, 2022
@ghost
Copy link

ghost commented Jul 22, 2022

Tagging subscribers to this area: @hoyosjs
See info in area-owners.md if you want to be subscribed.

Issue Details

We have made several llvm libunwind patches for NativeAOT, since its initial bring up: dotnet/corert#2293. The collective patches are in the following two commits on top of upstream's latest version (as of this moment) 14.0.6 release:

Apply https://github.com/dotnet/runtime/commit/e57194552327bccaf2b45a9190f0e6e1289472e4
Apply https://github.com/dotnet/runtime/commit/db7ed089a9e20a391a53e0b547f95dc8bf916765

It would be great (on multiple fronts) if we upstream those patches via LLVM code review platform of choice; Phabricator (https://llvm.org/docs/Phabricator.html). It will require some additional work for other architectures which llvm-libunwind supports (such as s390x and RISCV), to make them ready to upstream. At the moment, it is relatively easier to upstream patches since we are on the latest release.


unw_get_save_loc API

One of the key additions in our local patches is the implementation of unw_get_save_loc, which was scoped to memory locations, but does not cover register locations. The original commit was dotnet/corert@c6571c6 and v14.0.6 state can be extracted from e571945.

Moreover, unw_get_save_loc is the API which we test in coreclr PAL introspection code (src/coreclr/pal/src/configure.cmake), and based on which we decide whether to use the pinning fallback in GC. If the memory-location-only patch gets upstreamed (without first implementing register locations support), we would probably need to improve our introspection to detect the exact capability we need out of this API for GC; currently HP libunwind (the recommended variant for coreclr PAL) has support for both memory and registers location, which eventually gives us granular control over object lifetime in the GC.

Author: am11
Assignees: -
Labels:

tracking-external-issue, area-Infrastructure-coreclr, untriaged

Milestone: -

@am11 am11 changed the title Track upstreaming llvm-libunwind patch Track upstreaming llvm-libunwind patches Jul 22, 2022
@am11
Copy link
Member Author

am11 commented Jul 22, 2022

cc @janvorli

@janvorli
Copy link
Member

@am11 thank you for creating this tracking issue. Regarding the unw_get_save_loc and register locations, I believe we don't generate any unwind instructions that would save / restore managed references in registers to / from other registers and I don't see us needing such functionality. So adding that would be mostly just for completeness of the implementation.

@hoyosjs hoyosjs removed the untriaged New issue has not been triaged by the area owner label Aug 15, 2022
@hoyosjs hoyosjs added this to the Future milestone Aug 15, 2022
@am11
Copy link
Member Author

am11 commented Sep 6, 2023

An update; LLVM has recently switched from Phabricator to GitHub Pull Requests model, which makes it much convenient to upstream the local patches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Infrastructure-coreclr tracking-external-issue The issue is caused by external problem (e.g. OS) - nothing we can do to fix it directly
Projects
Status: No status
Development

No branches or pull requests

3 participants