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

Unwinding support #52

Closed
wants to merge 4 commits into from
Closed

Unwinding support #52

wants to merge 4 commits into from

Conversation

Amanieu
Copy link
Collaborator

@Amanieu Amanieu commented Sep 16, 2016

This is actually required to ensure safety. Currently the following safe code causes a segfault: https://gist.github.com/Amanieu/9f4854c096fa519f046ace876779be60

Based on top of #51

@Amanieu Amanieu force-pushed the unwind branch 3 times, most recently from 224305c to 58a2fb6 Compare September 16, 2016 16:02
@Amanieu Amanieu force-pushed the unwind branch 8 times, most recently from 68df230 to 83030ac Compare September 29, 2016 16:27
@Amanieu Amanieu changed the base branch from master to next-major September 29, 2016 17:37
@Amanieu Amanieu changed the base branch from next-major to master September 29, 2016 17:39
@dpc
Copy link

dpc commented Oct 31, 2016

What's the plan with this in the light of panic aborting by default?

@Amanieu Amanieu force-pushed the unwind branch 3 times, most recently from d07564c to 81fb8ec Compare November 2, 2016 18:33
@Amanieu Amanieu changed the title WIP unwinding support Unwinding support Nov 2, 2016
@Amanieu
Copy link
Collaborator Author

Amanieu commented Nov 2, 2016

This PR is now complete. I implemented unwinding support on all supported architectures, and even threw in ARM support as well.

@Amanieu Amanieu force-pushed the unwind branch 2 times, most recently from 4a74db9 to 5877ec1 Compare November 4, 2016 04:53
@spinda
Copy link

spinda commented Dec 2, 2016

Why hasn't this been merged?

@whitequark
Copy link
Contributor

@spinda Currently this breaks bare-metal usage, which is the reason to use libfringe.

@edef1c edef1c closed this Feb 25, 2017
@dpc
Copy link

dpc commented Mar 22, 2017

What was the problem on bare-metal? From what I understand, seems like a clever solution. I really wish this was implemented already.

@edef1c
Copy link
Owner

edef1c commented Mar 22, 2017

@dpc We don't have a workable unwinder for bare metal. I've gotten started writing one, but I'm short on time.
I miight see if we can set this up as a Cargo feature.

@whitequark
Copy link
Contributor

@edef1c What? Of course we do, libunwind supports bare metal properly and I use it with libfringe.

The problem is that std doesn't export enough hooks into the panic machinery, so we end up with a pretty nasty case where we reimplement half of std::panicking.

@dpc
Copy link

dpc commented Mar 22, 2017

Did you brought it up to rust devs? Any chance it work eventually?

@whitequark
Copy link
Contributor

Did you brought it up to rust devs?

No. Personally I'm quite overloaded on OSS right now.

Any chance it work eventually?

There's no inherent technical reason it can't work.

@dpc
Copy link

dpc commented Mar 22, 2017

Please someone knowledgeable enough open an issue with Rust devs and explain it briefly if that's what is blocking it. :)

@edef1c
Copy link
Owner

edef1c commented Mar 23, 2017

@whitequark oh, huh. I thought that was the issue we had with unwinding on bare metal.

@Amanieu
Copy link
Collaborator Author

Amanieu commented Mar 23, 2017

I rebased my PR and made unwinding an optional feature.

@edef1c
Copy link
Owner

edef1c commented Mar 23, 2017

@Amanieu can you open a new PR for that? I can't reopen because GitHub is dumb about rebasing while a PR is closed

@Amanieu Amanieu mentioned this pull request Mar 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants