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

Update gimli to 0.17.0 #25

Merged
merged 3 commits into from
Feb 25, 2019
Merged

Update gimli to 0.17.0 #25

merged 3 commits into from
Feb 25, 2019

Conversation

philipc
Copy link
Contributor

@philipc philipc commented Feb 24, 2019

And fix #22

@philipc philipc requested a review from main-- February 24, 2019 05:19
// part of its operation, but we should ensure:
// - it never needs any cleanup
// - it never panics (this applies to all these extern functions, but especially here
// now that we are specifying `unwind(allowed)`)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a sidenote, I would argue that "needing cleanup" is not a problem in principle, it's just terrible when the actual unwinder object lives on the stack (thus should actually not be cleaned up).

The thing about panicking is that the entire point of this file is to bridge to rust's unwinding code - written in Rust. So again I think panicking is not a problem in principle (although it may still be strictly UB and thus a footgun, this unwind(allowed) mess being the prime example). The much bigger issue I see is that we have to find a general solution for error handling in this crate that does not involve panicking at all, in any place, so we don't run into infinite loops.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. I've changed the comment.

Also, inline asm no longer works (segmentation fault).
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.

Demo currently doesn't work.
2 participants