Skip to content

[cppia] Fix missing jit exception checks for method calls #1212

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

tobil4sk
Copy link
Member

Even with #1188, some checks for jit exceptions in interp code were still missing. This causes control flow bugs when local functions interact with other code, as they are currently not jit compiled.

In short, we have to make sure an exception hasn't been thrown after evaluating the object for a method call.

In interp mode, we need to stop immediately if a jit exception has been
thrown.
@tobil4sk tobil4sk changed the title [cppia] Fix missing jit exception checks [cppia] Fix missing jit exception checks for method calls Apr 10, 2025
This covers the CallHaxe bug
The host must be compiled with HXCPP_CATCH_SEGV so that seg faults are
handled as hxcpp exceptions.
@tobil4sk tobil4sk force-pushed the fix/missing-jit-exception-checks branch from 2643240 to c75a21a Compare April 10, 2025 21:09
@Simn
Copy link
Member

Simn commented Apr 11, 2025

@hughsando Please check!

@tobil4sk
Copy link
Member Author

@hughsando Please check!

It would be worth double checking if the full BCR_CHECKs are required in all cases, perhaps in some places it's enough just to check only ctx->exception.

CallFunExpr has the full check after getting this, and CallDynamicFunction has it after each argument, so I based my changes off those two. Perhaps there was a genuine reason why Call, CallHaxe and CallMemberVTable might not require the BCR check everywhere.

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.

2 participants