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.

Sign up for GitHub

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

Jump to bottom

Remove error handler invocation as a result of an unwind #302

Closed
davidchisnall opened this issue Sep 23, 2024 · 1 comment · Fixed by #306
Closed

Remove error handler invocation as a result of an unwind #302

davidchisnall opened this issue Sep 23, 2024 · 1 comment · Fixed by #306

Comments

@davidchisnall
Copy link
Collaborator

Currently, we have the following flow:

  1. Compartment A calls compartment B
  2. A fault occurs in B.
  3. If we force unwind out of B and A has an error handler, A's error handler is invoked.

This was useful in the test suite for checking that tests do not terminate unexpectedly. So far, it has been annoying in 100% of all other cases, and every error handler that we've written has a special case to silently ignore it.

This might not be the worst idea I ever had, but it's definitely near the top of the list. Removing this path would also simplify the switcher code.
@rmn30
Copy link
Collaborator

rmn30 commented Sep 23, 2024 •
edited
Loading

This does seem like it would simplify things and make error handling more predictable, but I think it would be good to combine it with compiler warnings about checking the result of compartment calls as per CHERIoT-Platform/llvm-project#34 .

davidchisnall added a commit that referenced this issue Oct 3, 2024
@davidchisnall
Don't invoke error handlers when the callee errors.
bd02487 
I originally made a forced unwind out of a compartment invoke the
caller's error handler (if it existed) with a special error code.  The
idea was that you could use error handlers to jump to recovery paths,
rather than checking return values.

This seemed like a good idea at the time, but turned out to be a
mistake.  The only code that has ever used this behaviour was the test
suite.  Other error handlers have special-case logic to check if this is
the reason and ignore it if so.  It also added a lot of confusing
control flow in the switcher, which is bad because the switcher is the
core of the TCB.

Fixes #302
@davidchisnall davidchisnall mentioned this issue Oct 3, 2024
Merged
davidchisnall added a commit that referenced this issue Oct 4, 2024
@davidchisnall
Don't invoke error handlers when the callee errors.
c144643 
I originally made a forced unwind out of a compartment invoke the
caller's error handler (if it existed) with a special error code.  The
idea was that you could use error handlers to jump to recovery paths,
rather than checking return values.

This seemed like a good idea at the time, but turned out to be a
mistake.  The only code that has ever used this behaviour was the test
suite.  Other error handlers have special-case logic to check if this is
the reason and ignore it if so.  It also added a lot of confusing
control flow in the switcher, which is bad because the switcher is the
core of the TCB.

Fixes #302
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
RTOS Version 1.0 release
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Don't invoke error handlers when the callee errors. CHERIoT-Platform/cheriot-rtos
2 participants
@davidchisnall @rmn30