Skip to content
This repository has been archived by the owner on Nov 1, 2020. It is now read-only.

Use LLVM libunwind for stack unwinding on Unix #2293

Merged
merged 2 commits into from
Dec 1, 2016

Conversation

janvorli
Copy link
Member

@janvorli janvorli commented Dec 1, 2016

This change replaces external libunwind usage on Unix (including OSX) by a copy of the LLVM libunwind.
It also modifies libunwind to support reporting register location and implements
the unw_get_save_loc API to get this location.
For now, only memory locations are supported (register locations are not).
The support for register locations was added to x86, x64, arm and arm64. The OpenRISC and
PPC code was not modified.

@janvorli
Copy link
Member Author

janvorli commented Dec 1, 2016

@jkotas, @sergiy-k can you take a look please?
Please note there are two commits - first one just brings in the unmodified libunwind sources, the other contains all the changes in our stuff and the libunwind stuff. So they should be merged in separately to preserve the history.

@janvorli
Copy link
Member Author

janvorli commented Dec 1, 2016

There is something strange going on. I can build it on my OSX / Linux devices, but the CI build has failed. I have to investigate.

@janvorli janvorli force-pushed the add-llvm-libunwind-as-sources branch from 7cc3eaa to 60f4ebf Compare December 1, 2016 01:33
@jkotas
Copy link
Member

jkotas commented Dec 1, 2016

I would be nice to have the libunwind sources in a commit that has nothing but the libunwind sources in it, set the author of the commit to dotnet-bot <[email protected]> and include link to the exact stamp from where it was forked in the commit description.

@janvorli
Copy link
Member Author

janvorli commented Dec 1, 2016

The first commit is just that - only the author and stamp are not that way. Let me see if I can modify it that way.

@janvorli janvorli force-pushed the add-llvm-libunwind-as-sources branch from 60f4ebf to 407f544 Compare December 1, 2016 01:52
@janvorli
Copy link
Member Author

janvorli commented Dec 1, 2016

@jkotas I've modified the first commit that way.

@jkotas
Copy link
Member

jkotas commented Dec 1, 2016

The first commit is just

It also has changes to integrate it into the corert build system and PAL. It would be nice to separate those out in a different commit.

@janvorli
Copy link
Member Author

janvorli commented Dec 1, 2016

Hmm, I was sure I have separated that. Sigh.

@jkotas
Copy link
Member

jkotas commented Dec 1, 2016

We should be able to remove libunwind8-dev from Documentation\prerequisites-for-building.md.

@janvorli
Copy link
Member Author

janvorli commented Dec 1, 2016

Good point

@janvorli janvorli force-pushed the add-llvm-libunwind-as-sources branch from 407f544 to ca2ab24 Compare December 1, 2016 02:15
dotnet-bot and others added 2 commits December 1, 2016 03:24
Fetched from https://github.com/llvm-mirror/libunwind
master branch as of commit 1041783b9f43ede983fbca7e41aaf3300286fcd7
This change replaces external libunwind usage on Unix by a copy of the LLVM libunwind.
It also modifies libunwind to support reporting register location and implements
the unw_get_save_loc API to get this location.
For now, only memory locations are supported (register locations are not).
@janvorli janvorli force-pushed the add-llvm-libunwind-as-sources branch from ca2ab24 to c6571c6 Compare December 1, 2016 02:33
@janvorli
Copy link
Member Author

janvorli commented Dec 1, 2016

@jkotas I have moved stuff between commits using some git magic, so now the first commit should be pure import of the LLVM libunwind.

@janvorli
Copy link
Member Author

janvorli commented Dec 1, 2016

I've also updated the prereq doc.

@jkotas
Copy link
Member

jkotas commented Dec 1, 2016

Could you please also get in touch with llvm libunwind maintainers whether they would be open to upstreaming the change you made?

@jkotas
Copy link
Member

jkotas commented Dec 1, 2016

#2166 can be reverted with this change.

@jkotas jkotas merged commit 49a052f into dotnet:master Dec 1, 2016
@janvorli
Copy link
Member Author

janvorli commented Dec 1, 2016

@jkotas I have already removed the #2166 as part of this change.

@jkotas
Copy link
Member

jkotas commented Dec 1, 2016

Actually, I have done in in #2294. It is taken care of either way.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants