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

Pony can catch C++ exceptions and vice-versa #2455

Closed
Praetonus opened this issue Dec 28, 2017 · 10 comments · Fixed by #3907
Closed

Pony can catch C++ exceptions and vice-versa #2455

Praetonus opened this issue Dec 28, 2017 · 10 comments · Fixed by #3907
Labels
bug Something isn't working help wanted Extra attention is needed needs investigation This needs to be looked into before its "ready for work"

Comments

@Praetonus
Copy link
Member

Praetonus commented Dec 28, 2017

A try block in Pony can catch a C++ exception, and a try block in C++ with a catch-all clause can catch a Pony exception. This is problematic on both sides. On the C++ exception/Pony catch side, the C++ runtime needs to clean up its exceptions in order to behave correctly. On the Pony exception/C++ catch side, the C++ runtime reclaiming the static memory used for Pony exceptions will result in a lot of bad things.

This happens with C++ code compiled with both GCC and Clang. I've only tested this with the POSIX-specific exception handling. I do not know if it also happens on Windows.

It would also be interesting to look at how exceptions from other languages than Pony and C++ behave in those cases. Rust exceptions, for example.

@Praetonus
Copy link
Member Author

Praetonus commented Dec 28, 2017

According to the Itanium exception handling ABI manual section 1.6.4, the C++ catch-all clause being able to catch foreign exceptions is intended behaviour. The C++ program is then allowed to either swallow the exception (deleting it with _Unwind_DeleteException) or to rethrow it without examining it. In C++ code, this translates to the catch-all clause having a rethrow expression or not. So it turns out that the C++ side is normal "you better know what you're doing or everything will blow up" C++.

Regarding the Pony side, I think we should always rethrow foreign exceptions. It looks like the most straightforward way to do it is to check the exception class inside of the personality function and if it is a foreign exception, to tell the unwinding API that we can't handle it.

I'm marking this as needing discussion during sync since I'd like to have other people's opinion on the matter.

@bates64
Copy link

bates64 commented Jan 2, 2018

Rust exceptions

Rust panics kill the process. Most possibly-erroring functions will return a Result - should a Result::Err be counted as an exception when connected to Pony?

@Praetonus
Copy link
Member Author

@heyitsmeuralex Thank you for the input. I'm not extremely familiar with Rust, so what I'm going to say might be wrong.

From what I can see from the Rust docs, Result seems to be a sum type, which would be akin to Pony's union types. By "exceptions" I meant anything resulting in stack unwinding. In Rust it seems like panics are the only constructs able to do that.

In the current state of things, Pony would swallow Rust's panics that result in unwinding. I haven't looked at Rust's implementation of EH so I don't know what it would do with a Pony exception.


To sum up my thoughts about all of this more precisely than in my previous comment, I think the responsibility of defining what happens to a foreign exception belongs to the language traversed by the exception (or languages, if the exception traverses frames from multiple languages in addition to its source language).

However, I think what's done when handling a foreign exception should be clearly defined by each language. I can see three reasonable possibilities

  • swallow the exception and clean it up correctly
  • rethrow the exception
  • invoke undefined behaviour

Basically the Itanium ABI rules with UB allowed. I've added that because no language should be forced to handle foreign stuff, and since it would be explicitly defined as undefined behaviour it would act as a big "don't do that" sign. So I think it would be fine for languages that don't want to bother with foreign exceptions.

Of course we can't go around and ask every language out there to use one of these rules so this is just my opinion on what I think "basic courtesy" foreign exception interfaces can look like.

As I said in my previous comment, I think Pony should always rethrow foreign exceptions. This seems like the less constraining and intrusive method to me, given that we probably don't want to allow programmers to handle foreign exceptions in detail since it would be unsafe and would require new syntax.

There's something I overlooked in my previous comment with regards to this solution, which is the treatment of the then part of try blocks, which is always executed regardless of whether an exception was thrown. If we rethrow foreign exceptions, this means that they might traverse multiple try/else/then blocks, and we'll have to interrupt unwinding in those cases in order to execute then blocks.

This means that the implementation wouldn't be as simple as a conditional in the personality function, but it still wouldn't be very complex for anybody familiar with exception handling in the runtime and code generator.

This also has a language impact, as it means that then block shouldn't assume that either the try block or the else block ran to completion. Though I don't think it would have a big impact in real code, as depending on that sounds a bit like an antipattern.

@jemc
Copy link
Member

jemc commented Jan 2, 2018

we probably don't want to allow programmers to handle foreign exceptions in detail since it would be unsafe and would require new syntax

I agree that we probably don't want to do this, but not sure I agree with your reason. If they're making foreign function calls, those calls are already unsafe.

This also has a language impact, as it means that then block shouldn't assume that either the try block or the else block ran to completion

I think the only thing one could reasonably assume from a then block currently is: "either the try block ran to completion, or it was incomplete, and the else block ran to completion". Would that no longer be a valid assumption from a then block?

@Praetonus
Copy link
Member Author

If they're making foreign function calls, those calls are already unsafe.

That's a good point.

I think the only thing one could reasonably assume from a then block currently is: "either the try block ran to completion, or it was incomplete, and the else block ran to completion". Would that no longer be a valid assumption from a then block?

Yes, it wouldn't be a valid assumption anymore. However, it would only impact cases where a foreign exception can traverse the try/else/then block.

@Praetonus
Copy link
Member Author

We discussed this during the sync call. Consensus is that when catching a foreign exception, we should rethrow it immediately without running any part of the try/else/then block.

@Praetonus Praetonus self-assigned this Jan 3, 2018
Praetonus pushed a commit to Praetonus/ponyc that referenced this issue Jan 5, 2018
Praetonus pushed a commit to Praetonus/ponyc that referenced this issue Jan 5, 2018
Praetonus pushed a commit to Praetonus/ponyc that referenced this issue Jan 9, 2018
Praetonus pushed a commit to Praetonus/ponyc that referenced this issue Jan 9, 2018
@Praetonus
Copy link
Member Author

#2466 fixes the issue on *nix systems. There still are issues on windows, foreign code doesn't catch foreign exceptions if they've traversed a Pony frame. During the sync call last wednesday, @sylvanc mentioned that this might be related to the way SEH and LLVM exception code generation interact.

If there are SEH experts out there, feel free to weigh in.

@Praetonus Praetonus added windows help wanted Extra attention is needed labels Jan 26, 2018
@Praetonus
Copy link
Member Author

Unassigning myself since #2466 has been merged.

@Praetonus
Copy link
Member Author

Looks like the windows problem might be related to this LLVM bug.

@SeanTAllen SeanTAllen added needs investigation This needs to be looked into before its "ready for work" bug Something isn't working and removed windows labels May 12, 2020
@chalcolith
Copy link
Member

This seems to be a JIT problem, given that the test works with the new non-jit test runner.

SeanTAllen pushed a commit that referenced this issue Dec 10, 2021
Previously there were a number of tests in `tests/libponyc` that relied on the LLVM ORC JIT to test Pony compilation. Several of these never worked right on Windows or ARM.

This PR (a collaboration between @SeanTAllen and @kulibali) removes all tests that depend on the JIT from `tests/libponyc` and moves them to individual directories in `tests/libponyc-run`.

Updates the build system to build individual libraries from any C or C++ code found in subdirectories of `tests/libponyc-run` when running `make build` (`make.ps1 build` on Windows). These libraries end up in `build/{release,debug}`.

Adds a program in `tests/libponyc-run/runner` that, during `make test`, does the following for each subdirectory in `tests/libponyc-run` that contains Pony code:

- Runs `ponyc` on that directory, linking with the libraries built by `make build` as necessary.
- Runs the resulting program, comparing the exit code with the value in `expected-exit-code.txt` (or `0` if that file does not exist).

Fixes #3857
Fixes #2455 (the exception-catching test now works)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed needs investigation This needs to be looked into before its "ready for work"
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants