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

Add support for unwinding #62

Open
antoyo opened this issue Aug 14, 2021 · 16 comments
Open

Add support for unwinding #62

antoyo opened this issue Aug 14, 2021 · 16 comments
Labels
hard libgccjit requires a change in libgccjit

Comments

@antoyo
Copy link
Contributor

antoyo commented Aug 14, 2021

  • Requires adding support for try/catch in libgccjit.
  • Requires changing rustc_codegen_ssa API.

Some notes to myself to debug the issue in release mode:

  1. Attempt to reproduce by creating an example without std.
  2. Reduce that to a pure libgccjit example (e.g. without using cg_gcc).
  3. Try to reproduce in C++ directly (to see if it's a bug there as well or if the warn about something or if they have a workaround for gotos if that's the issue).
  4. Perhaps the fact that gotos and conditions use C labels instead of the special jump labels is the issue (see how Add support for NonNull function attribute #326 is resolved).
@antoyo antoyo added hard libgccjit requires a change in libgccjit labels Aug 14, 2021
@sapir
Copy link
Contributor

sapir commented Oct 26, 2021

I'd like to try to help with this.

@antoyo
Copy link
Contributor Author

antoyo commented Oct 26, 2021

This issue is very hard so I suggest you start with one of the good first issues.

If you still wanna try, I'll upload my branch of libgccjit that starts adding support for try/catch and I suggest to start looking at the rustc_codegen_ssa API functions related to that (invoke, landing_pad, …) and think of a way to refactor it to make it easier to support a different API based on try/catch (I don't believe it's possible to implement unwinding without changing this API, even with hacks).

@bjorn3
Copy link
Member

bjorn3 commented Oct 26, 2021

Can invoke be codegened as a try + catch with the catch goto'ing to the landingpad?

@antoyo
Copy link
Contributor Author

antoyo commented Oct 26, 2021

Can invoke be codegened as a try + catch with the catch goto'ing to the landingpad?

That's an interesting idea, actually. I didn't think of doing that. Maybe that would work.

@sapir
Copy link
Contributor

sapir commented Oct 26, 2021

Actually I was thinking the entire block should have a try/catch around it, so that the result of the invoke can be saved. I don't understand yet what the result of the llvm landingpad instruction actually is, though, except that it carries some info about the thrown exception?

@bjorn3
Copy link
Member

bjorn3 commented Oct 26, 2021

When the personality function runs it searches for the location of the cleanup code in a compiler generated table (.gcc_except_table I believe for rust). When it find it, it tells the unwinder to use it, but it also tells the unwinder to pass some arguments. In case of rust these are two arguments, which if I recall correctly are pointers to the unwind context and the actual exception object. The landingpad instruction returns all arguments passed by the personality function I believe.

When you jump out of the catch to get to the landingpad does gcc insert a function call to indicate that unwinding is finished? If so, my suggestion won't work. If it doesn't do you know how to run the unwind continue/finish functions? (at machine evel a tail call to _Eh_Resume for continuing unwind I believe, but maybe gcc expects a special instruction?)

@sapir
Copy link
Contributor

sapir commented Oct 27, 2021

When you jump out of the catch to get to the landingpad does gcc insert a function call to indicate that unwinding is finished?

I haven't actually tried it at the GENERIC level, but I tried supposedly equivalent C++ code and I think it does :( It calls __cxa_end_catch:

https://compiler-explorer.com/z/Kadvfz81W

So I guess maybe the proper behavior is to have all the blocks that are only reachable from the catch side of invoke statements, copied into the catch expr?

btw, will the __cxa_* functions work with the panic implementation?

@bjorn3
Copy link
Member

bjorn3 commented Oct 27, 2021

Rust doesn't use the C++ standard library, so it uses _Unwind_* instead of __cxa_*. Resuming with unwinding requires _Unwind_Resume and ending unwinding doesn't seem to generate any implicit function call at all. Only an explicit call to the cleanup function passed to the try intrinsic.

@sapir
Copy link
Contributor

sapir commented Oct 27, 2021

Are unwind tables necessary? Can exceptions panics be implemented just by calling the appropriate functions?

@bjorn3
Copy link
Member

bjorn3 commented Oct 27, 2021

Unwind tables are necessary for DWARF based unwinding. There is also the older SjLj based unwinding which doesn't need unwind tables, but still needs support from the compiler backend AFAIK. Other than that the only option is to change ABI to return a pointer optionally representing the panic and checking this pointer after every call.

@bjorn3
Copy link
Member

bjorn3 commented Oct 27, 2021

Looks like at least for GIMPLE the __cxa_* calls are explicit. I am not sure if it allows jumping out of a cleanup block though, but your suggestion of

So I guess maybe the proper behavior is to have all the blocks that are only reachable from the catch side of invoke statements, copied into the catch expr?

should work I think @antoyo.

@sapir
Copy link
Contributor

sapir commented Oct 27, 2021

I think I'll try it with the goto then and see if it works

@antoyo
Copy link
Contributor Author

antoyo commented Oct 27, 2021

I haven't touched that gcc branch in a while, so it might not work correctly (and there's a bit more stuff to implement) but here it is.

It adds support to set the personality function and try/catch.

@sapir I hope that helps!

@sapir
Copy link
Contributor

sapir commented Nov 7, 2021

I've pushed my work so far to this branch (gcc) and this branch (rustc_codegen_gcc)

@antoyo
Copy link
Contributor Author

antoyo commented Feb 1, 2022

@sapir : Any progress on this? If you're not working on this anymore, I'll start working on this myself soon.

@sapir
Copy link
Contributor

sapir commented Mar 20, 2022

@antoyo sorry! I'm not working on this now, I hope the branches I pushed help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hard libgccjit requires a change in libgccjit
Projects
None yet
Development

No branches or pull requests

3 participants