Skip to content

Commit

Permalink
Clean up the thread-exit path.
Browse files Browse the repository at this point in the history
With the most recent changes, we were exiting threads via the
fault-in-the-switcher path.  The new code put the initial csp (full
bounds, address 16 bytes from the top) on the first trusted-stack frame.
When we tried to pop this, we were getting a bounds fault because we
tried to restore two registers from the 16-byte offset for error
handlers / local state, and then a third one from off the top.  This
then triggered a bounds fault in the switcher and we bounced to the
thread-exit path.

Prior to the most recent change, the top trusted-stack frame had a 0
csp, so we'd get a tag violation and hit the same path a couple of
instructions earlier.

This was due to the fact that our bounds check was checking if the
*current* trusted stack frame was the top one, not checking the previous
one.  The fix for that was simply to move the subtraction earlier.

This then caused a worse error because the (unused, due to other bugs)
path into the thread exit code was not restoring the trusted stack
pointer into csp where the exception path expected it.  This is now
fixed.

Reported by @rmn30, who noticed that we were taking a surprising number
of exceptions in the hello world example.  We now take precisely one:
the tag error when we do the `cjr $cra` to return from a thread
entry-point function that doesn't have a caller.
  • Loading branch information
davidchisnall committed Oct 16, 2024
1 parent e7f4962 commit e15fe44
Showing 1 changed file with 8 additions and 1 deletion.
9 changes: 8 additions & 1 deletion sdk/core/switcher/entry.S
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,8 @@ exception_entry_asm:
// Value 24 is reserved for custom use.
.Lset_mcause_and_exit_thread:
csrw mcause, 24
// The thread exit code expects the trusted stack pointer to be in csp.
cspecialr csp, mtdc
j .Lthread_exit

// The continue-resume path expects the location that we will mret to to be
Expand All @@ -802,9 +804,14 @@ exception_entry_asm:
// make sure there is a frame left in the trusted stack
clhu t2, TrustedStack_offset_frameoffset(ctp)
li tp, TrustedStack_offset_frames
// Move to the previous trusted stack frame.
addi t2, t2, -TrustedStackFrame_size
// If this is the first trusted stack frame, then the csp that we would be
// loading is the csp on entry, which does not have a spilled area. In
// this case, we would fault when loading, so would exit the thread, but we
// should instead gracefully exit the thread.
bgeu tp, t2, .Lset_mcause_and_exit_thread
cspecialr ctp, mtdc
addi t2, t2, -TrustedStackFrame_size
cincoffset ct1, ctp, t2
// Restore the stack pointer. All other spilled values are spilled there.
clc csp, TrustedStackFrame_offset_csp(ct1)
Expand Down

0 comments on commit e15fe44

Please sign in to comment.