Add support for stackless error handlers.
These are given a stack bounded it was on entry into the compartment,
with an undefined offset. As such, they can be used even when the cause
of the error is stack overflow.
Unlike the existing error handler, this cannot take a snapshot of the
register file, because there is no space for it.
Both kinds of error handler can be present. When both are present, the
existing kind will be used if there is sufficient stack space to invoke
it, otherwise the stackless one will be invoked.
This also adds a 'library' that provides an implementation of a
stackless error handler that interoperates with the new setjmp-based
unwind code. When an error occurs, it pops the top jump buffer and
invokes it.
A few other small cleanups while I was touching adjacent things:
- There were some issues in the switcher API to recover the initial
stack, the tests were passing largely by coincidence. This code is
now fixed and the tests improved.
- A load of recently added libraries were not linked to in the README
for libraries. They are now.
- Reserve the 16 bytes at the top of the stack on thread entry as well
as on cross-compartment call.
Fixes #298
diff --git a/sdk/compartment.ldscript b/sdk/compartment.ldscript
index 6419245..ba46ec4 100644
--- a/sdk/compartment.ldscript
+++ b/sdk/compartment.ldscript
@@ -13,7 +13,9 @@
# delta. The final layout will be the compartment import table
# followed by the text segment. There won't be any padding, because
# the compartment import table is more strongly aligned than text.
- LONG(DEFINED(compartment_error_handler) ? compartment_error_handler - __compartment_code_start + SIZEOF(.compartment_import_table) : -1);
+ SHORT(DEFINED(compartment_error_handler) ? compartment_error_handler - __compartment_code_start + SIZEOF(.compartment_import_table) : 0xffff);
+ # The stackless compartment error handler, if it is defined.
+ SHORT(DEFINED(compartment_error_handler_stackless) ? compartment_error_handler_stackless - __compartment_code_start + SIZEOF(.compartment_import_table) : 0xffff);
# Array of compartment exports
*(.compartment_exports .compartment_exports.*);
}
@@ -34,6 +36,7 @@
# If there is a compartment error handler, make sure that it is before
# anything that can have linker relaxations so that its displacement
# from __compartment_code_start is a constant.
+ *(.compartment_error_handler_stackless));
*(.compartment_error_handler);
*(.text .text.*);
}
diff --git a/sdk/core/loader/boot.cc b/sdk/core/loader/boot.cc
index 11cb2c2..a72541f 100644
--- a/sdk/core/loader/boot.cc
+++ b/sdk/core/loader/boot.cc
@@ -855,6 +855,9 @@
// Stack pointer points to the top of the stack.
stack.address() += config.stack.size();
+ // Reserve space at the start of the stack for error handling and so
+ // on.
+ stack.address() -= STACK_ENTRY_RESERVED_SPACE;
Debug::log("Thread's stack is {}", stack);
threadTStack->csp = stack;
@@ -881,6 +884,8 @@
threadTStack->frameoffset = offsetof(TrustedStack, frames[1]);
threadTStack->frames[0].calleeExportTable =
build(compartment.exportTable);
+ // Special case: The first frame has the initial csp.
+ threadTStack->frames[0].csp = stack;
Debug::log("Thread's trusted stack is {}", threadTStack);
diff --git a/sdk/core/loader/types.h b/sdk/core/loader/types.h
index fea0a7e..ec488c9 100644
--- a/sdk/core/loader/types.h
+++ b/sdk/core/loader/types.h
@@ -1022,10 +1022,17 @@
/**
* The offset of the compartment's error handler from the start of
- * `pcc`. This must always be positive, a value of -1 is used to
- * indicate that this compartment does not provide an error handler.
+ * `pcc`. A value of 0xffff indicates that this compartment does not
+ * provide an error handler.
*/
- ptrdiff_t errorHandler;
+ uint16_t errorHandler;
+
+ /**
+ * The offset of the compartment's stackless error handler from the
+ * start of `pcc`. A value of 0xffff indicates that this compartment
+ * does not provide a stackless error handler.
+ */
+ uint16_t errorHandlerStackless;
};
/**
diff --git a/sdk/core/switcher/entry.S b/sdk/core/switcher/entry.S
index ff5d3e7..80af180 100644
--- a/sdk/core/switcher/entry.S
+++ b/sdk/core/switcher/entry.S
@@ -228,7 +228,7 @@
zero_stack t2, s0, gp
after_zero:
// Reserve space for unwind state and so on.
- cincoffset csp, csp, -16
+ cincoffset csp, csp, -STACK_ENTRY_RESERVED_SPACE
#ifdef CONFIG_MSHWM
// store new stack top as stack high water mark
csrw CSR_MSHWM, sp
@@ -259,7 +259,7 @@
// the stack is the length that the callee can use.
cgetlen t2, csp
// Include the space we reserved for the unwind state.
- addi t2, t2, -16
+ addi t2, t2, -STACK_ENTRY_RESERVED_SPACE
bgtu tp, t2, .Lstack_too_small
// Get the flags field into tp
@@ -597,36 +597,85 @@
// Load the interrupted thread's stack pointer into ct0
clc ct0, TrustedStack_offset_csp(csp)
- // Fetch the base of compartment stack before cincoffset for later
- // comparison. The subsequent cincoffset could cause the base to change, if
- // the capability becomes unrepresentable. That would clear the tag, and so
- // we'd fault later. Faulting in the switcher will then trigger a forced
- // unwind.
- cgetbase tp, ct0
- // Allocate space for the register save frame on the stack. If we didn't
- // have enough space here, we'll fault setting up the call frame, which
- // will detect a trap in the switcher and force unwind.
- cincoffset ct0, ct0, -(16*8)
// See if we can find a handler:
clhu tp, TrustedStack_offset_frameoffset(csp)
li t1, TrustedStack_offset_frames
beq tp, t1, .Lend_of_stack
addi tp, tp, -TrustedStackFrame_size
+
// ctp points to the current available trusted stack frame.
cincoffset ctp, csp, tp
- // ct1 now contains the export table for the callee
+ // a0 indicates whether we're calling a stackless error handler (0: stack,
+ // 1: stackless)
+ li a0, 0
+
+ // Allocate space for the register save frame on the stack.
+ cincoffset ct0, ct0, -(16*8)
+
+ // WARNING: ENCODING SPECIFIC.
+ // The following depends on the fact that before-the-start values are not
+ // representable in the CHERIoT encoding and so will clear the tag. If
+ // this property changes then this will need to be replaced by a check that
+ // against the base of the stack. Note that this check can't be a simple
+ // cgetbase on ct0, because moving the address below the base sufficiently
+ // far that it's out of *representable* bounds will move the reported base
+ // value (base is a displacement from the address).
+ cgettag t1, ct0
+ // If there isn't enough space on the stack, see if there's a stackless
+ // handler.
+ beqz t1, .Ltry_stackless_handler
+
clc ct1, TrustedStackFrame_offset_calleeExportTable(ctp)
- // Reset the export table pointer to point to the *start* of the export
+ // Set the export table pointer to point to the *start* of the export
// table. It will currently point to the entry point that was raised.
// TODO: We might want to pass this to the error handler, it might be
// useful for providing per-entry-point error results.
cgetbase s0, ct1
csetaddr ct1, ct1, s0
- clw s0, ExportTable_offset_errorHandler(ct1)
- // A value of -1 indicates no error handler
+ clhu s0, ExportTable_offset_errorHandler(ct1)
+ // A value of 0xffff indicates no error handler
+ // If we found one, use it, otherwise fall through and try to find a
+ // stackless handler.
+ li s1, 0xffff
+ bne s0, s1, .Lhandler_found
+
+.Ltry_stackless_handler:
+ clc ct1, TrustedStackFrame_offset_calleeExportTable(ctp)
+ // Set the export table pointer to point to the *start* of the export
+ // table. It will currently point to the entry point that was raised.
+ cgetbase s0, ct1
+ csetaddr ct1, ct1, s0
+ clhu s0, ExportTable_offset_errorHandlerStackless(ct1)
+ // A value of 0xffff indicates no error handler
// Give up if there is no error handler for this compartment.
- addi s1, s0, 1
- beqz s1, .Lno_handler_found
+ li s1, 0xffff
+ beq s0, s1, .Lno_handler_found
+
+ // The stack may have had its tag cleared at this point, so for stackless
+ // handlers we need to restore the on-entry stack.
+ // Get the previous trusted stack frame
+
+ // Load the caller's csp
+ clc ca0, TrustedStackFrame_offset_csp(ctp)
+
+ // If this is the top stack frame, then the csp field is the value on
+ // entry. If it's any other frame then we need to go to the previous one
+ cincoffset cs1, csp, TrustedStack_offset_frames
+ beq s1, t1, .Lrecovered_stack
+
+ // The address of the stack pointer will point to the bottom of the
+ // caller's save area, so we set the bounds to be the base up to the
+ // current address.
+ cgetaddr a1, ca0
+ cgetbase a2, ca0
+ sub a1, a1, a2
+ csetaddr ca0, ca0, a2
+ // The code that installs the context expects csp to be in ct0
+ csetboundsexact ct0, ca0, a1
+.Lrecovered_stack:
+ li a0, 1
+
+.Lhandler_found:
// If we have found a handler, mark this threads as no longer on the
// force-unwind path. Any future fault will trigger a forced unwind.
@@ -660,6 +709,16 @@
csetaddr cra, cra, s1
cincoffset cra, cra, s0
+ // If we're in an error handler with a stack, set up the stack, otherwise
+ // we just need to set up argument registers.
+ beqz a0, .Lset_up_stack_handler
+ clw a0, TrustedStack_offset_mcause(csp)
+ csrr a1, mtval
+ li a2, 0
+ cmove csp, ct0
+ j .Linvoke_error_handler
+
+.Lset_up_stack_handler:
// Set up the on-stack context for the callee
clc cs1, 0(csp)
ccleartag cs1, cs1
@@ -675,6 +734,8 @@
clw a1, TrustedStack_offset_mcause(csp)
csrr a2, mtval
cmove csp, ca0
+
+.Linvoke_error_handler:
// Clear all registers except:
// cra is set by cjalr. csp and cgp are needed for the called compartment.
// ca0, used for the register state
@@ -880,8 +941,24 @@
// two instructions.
cspecialr ca0, mtdc
clhu a1, TrustedStack_offset_frameoffset(ca0)
+ addi a1, a1, -TrustedStackFrame_size
cincoffset ca0, ca0, a1
clc ca0, TrustedStackFrame_offset_csp(ca0)
+ // If this is the first frame, then the recovered stack will be the stack
+ // on entry. If this is not the first frame then then we need to find the
+ // saved CSP from the caller and reset the bounds. The address of the
+ // saved CSP will be the value after the switcher spilled registers and so
+ // will be the top of the callee's stack.
+ li a2, TrustedStack_offset_frames
+ beq a1, a2, 0f
+
+ // Find the previous frame's csp and reset the bounds
+ cgetaddr a1, ca0
+ cgetbase a2, ca0
+ sub a1, a1, a2
+ csetaddr ca0, ca0, a2
+ csetboundsexact ca0, ca0, a1
+0:
cret
.section .text, "ax", @progbits
diff --git a/sdk/core/switcher/export-table-assembly.h b/sdk/core/switcher/export-table-assembly.h
index 3d446ef..5581f78 100644
--- a/sdk/core/switcher/export-table-assembly.h
+++ b/sdk/core/switcher/export-table-assembly.h
@@ -10,3 +10,4 @@
EXPORT_ASSEMBLY_OFFSET(ExportTable, pcc, 0)
EXPORT_ASSEMBLY_OFFSET(ExportTable, cgp, 8)
EXPORT_ASSEMBLY_OFFSET(ExportTable, errorHandler, 16)
+EXPORT_ASSEMBLY_OFFSET(ExportTable, errorHandlerStackless, 18)
diff --git a/sdk/core/switcher/trusted-stack-assembly.h b/sdk/core/switcher/trusted-stack-assembly.h
index f321548..0511ac1 100644
--- a/sdk/core/switcher/trusted-stack-assembly.h
+++ b/sdk/core/switcher/trusted-stack-assembly.h
@@ -66,3 +66,10 @@
* Load, Store, LoadStoreCapability, LoadMutable StoreLocal and LoadGlobal
*/
#define COMPARTMENT_STACK_PERMISSIONS 0x7e
+
+/**
+ * Space reserved at the top of a stack on entry to the compartment.
+ *
+ * This *must* be a multiple of 16, which is the stack alignment.
+ */
+#define STACK_ENTRY_RESERVED_SPACE 16
diff --git a/sdk/include/cdefs.h b/sdk/include/cdefs.h
index ad3b9ae..2242e7e 100644
--- a/sdk/include/cdefs.h
+++ b/sdk/include/cdefs.h
@@ -115,4 +115,8 @@
*/
unsigned __builtin_strlen(const char *str) __asm__("_Z6strlenPKc");
+#if !defined(CLANG_TIDY) && !__has_builtin(__builtin_cheri_top_get)
+# error Your compiler is too old for this version of CHERIoT RTOS, please upgrade to a newer version
+#endif
+
#endif // _CDEFS_H_
diff --git a/sdk/include/fail-simulator-on-error.h b/sdk/include/fail-simulator-on-error.h
index 52a65d3..da8f4eb 100644
--- a/sdk/include/fail-simulator-on-error.h
+++ b/sdk/include/fail-simulator-on-error.h
@@ -56,10 +56,12 @@
frame->get_register_value<CHERI::RegisterNumber::CSP>()};
CHERI::Capability returnCapability{
frame->get_register_value<CHERI::RegisterNumber::CRA>()};
+ // The top of the stack is 16 bytes above the stack pointer on entry,
+ // to provide space for unwind lists and so on.
if (registerNumber == CHERI::RegisterNumber::CRA &&
returnCapability.address() == 0 &&
exceptionCode == CHERI::CauseCode::TagViolation &&
- stackCapability.top() == stackCapability.address())
+ (stackCapability.top() - 16) == stackCapability.address())
{
// looks like thread exit -- just log it then ForceUnwind
DebugErrorHandler::log(
diff --git a/sdk/include/unwind.h b/sdk/include/unwind.h
index 9049828..7a1c898 100644
--- a/sdk/include/unwind.h
+++ b/sdk/include/unwind.h
@@ -1,6 +1,7 @@
#pragma once
#include <cdefs.h>
#include <setjmp.h>
+#include <switcher.h>
/**
* On-stack linked list of cleanup handlers.
@@ -23,9 +24,8 @@
__always_inline static inline struct CleanupList **cleanup_list_head()
{
void *csp = __builtin_cheri_stack_get();
- ptraddr_t top =
- __builtin_cheri_base_get(csp) + __builtin_cheri_length_get(csp);
- csp = __builtin_cheri_address_set(csp, top - 8);
+ ptraddr_t top = __builtin_cheri_top_get(csp);
+ csp = __builtin_cheri_address_set(csp, top - 8);
return (struct CleanupList **)csp;
}
@@ -37,6 +37,7 @@
CleanupList **__head = cleanup_list_head();
CleanupList *__top = *__head;
*__head = __top->next;
+ switcher_handler_invocation_count_reset();
longjmp(&__top->env, 1);
}
@@ -79,7 +80,7 @@
* `cleanup_unwind` are not called, but this function returns normally and so
* destructors of objects above this on the stack will be called normally.
*/
-void on_error(auto fn, auto err)
+static inline void on_error(auto fn, auto err)
{
CHERIOT_DURING
fn();
@@ -91,7 +92,7 @@
/**
* On-error helper with no error handler (returns normally from forced unwind).
*/
-void on_error(auto fn)
+static inline void on_error(auto fn)
{
on_error(fn, []() {});
}
diff --git a/sdk/lib/README.md b/sdk/lib/README.md
index d412525..8d272c6 100644
--- a/sdk/lib/README.md
+++ b/sdk/lib/README.md
@@ -7,11 +7,18 @@
This collection currently includes:
- [atomic](atomic/) provides atomic support functions.
+ - [compartment_helpers](compartment_helpers/) contains helpers for checking / ensuring that pointers are valid.
- [crt](crt/) provides C runtime functions that the compiler may emit.
- [cxxrt](cxxrt/) provides a minimal C++ runtime (no exceptions or RTTI support).
+ - [debug](debug/) contains functions to support the debug logging APIs.
+ - [event_group](event_group/) contains a FreeRTOS-like event-group API.
- [freestanding](freestanding/) provides a minimal free-standing C implementation.
+ - [locks](locks/) contains functions for various kinds of lock.
+ - [microvium](microvium/) builds the [microvium](https://github.com/coder-mike/microvium) JavaScript VM to provide an on-device JavaScript interpreter.
+ - [queue](queue/) contains functions for message queues.
- [stdio](stdio/) provides a *very* limited subset of `stdio.h` for debugging.
- [string](string/) provides `string.h` functions.
- [thread_pool](thread_pool) provides a simple thread pool that other threads can dispatch work to for asynchronous execution.
- - [microvium](microvium/) builds the [microvium](https://github.com/coder-mike/microvium) JavaScript VM to provide an on-device JavaScript interpreter.
+ - [unwind_error_handler](unwind_error_handler) provides an error handler that unwinds the stack.
+
diff --git a/sdk/lib/unwind_error_handler/README.md b/sdk/lib/unwind_error_handler/README.md
new file mode 100644
index 0000000..e7632d5
--- /dev/null
+++ b/sdk/lib/unwind_error_handler/README.md
@@ -0,0 +1,10 @@
+Unwind error handler library
+----------------------------
+
+This library provides an error handler that unwinds using the APIs in [`unwind.h`](../../include/unwind.h).
+
+This error handler can work even in cases of stack overflow.
+
+Note: Unlike most libraries, this is *not* built as a shared library.
+Error handlers must be part of the compartment that invokes them.
+As a result, this must be added as a dependency of each compartment that wishes to use it, rather than as a dependency of the firmware target.
diff --git a/sdk/lib/unwind_error_handler/unwind.S b/sdk/lib/unwind_error_handler/unwind.S
new file mode 100644
index 0000000..6e56a98
--- /dev/null
+++ b/sdk/lib/unwind_error_handler/unwind.S
@@ -0,0 +1,39 @@
+.section .compartment_error_handler_stackless,"aw",@progbits
+.globl compartment_error_handler_stackless
+.p2align 2
+.type compartment_error_handler_stackless,@function
+compartment_error_handler_stackless:
+// Get the head of the error list.
+ cgettop t0, csp
+ csetaddr csp, csp, t0
+ clc cs0, -8(csp)
+ beqz s0, .Lforce_unwind
+// Pop the top error from the list. */
+ clc ct0, 0(cs0)
+ csc ct0, -8(csp)
+// Mark this error handler as having finished. We may still trap again
+// and reenter this, but now that we've popped the top element from the
+// stack we will run some different cleanup code next time. */
+.Llookup_reset:
+ auipcc ct2, %cheriot_compartment_hi(__library_import_libcalls__Z39switcher_handler_invocation_count_resetv)
+ clc ct2, %cheriot_compartment_lo_i(.Llookup_reset)(ct2)
+ cjalr ct2
+// longjmp to the error handler.
+ clc cs1, 16(cs0)
+ clc csp, 24(cs0)
+ clc cra, 32(cs0)
+ clc cs0, 8(cs0)
+ cjr cra
+.Lforce_unwind:
+ li a0, 1
+ cret
+
+ .section .compartment_imports,"aG",@progbits,__library_import_libcalls__Z39switcher_handler_invocation_count_resetv,comdat
+ .type __library_import_libcalls__Z39switcher_handler_invocation_count_resetv,@object
+ .weak __library_import_libcalls__Z39switcher_handler_invocation_count_resetv
+ .p2align 3
+__library_import_libcalls__Z39switcher_handler_invocation_count_resetv:
+ .word __library_export_libcalls__Z39switcher_handler_invocation_count_resetv+1
+ .word 0
+ .size __library_import_libcalls__Z39switcher_handler_invocation_count_resetv, 8
+
diff --git a/sdk/lib/unwind_error_handler/xmake.lua b/sdk/lib/unwind_error_handler/xmake.lua
new file mode 100644
index 0000000..b27593a
--- /dev/null
+++ b/sdk/lib/unwind_error_handler/xmake.lua
@@ -0,0 +1,3 @@
+target("unwind_error_handler")
+ set_kind("object")
+ add_files("unwind.S")
diff --git a/sdk/lib/xmake.lua b/sdk/lib/xmake.lua
index 66701dd..4833910 100644
--- a/sdk/lib/xmake.lua
+++ b/sdk/lib/xmake.lua
@@ -11,4 +11,5 @@
"queue",
"stdio",
"string",
- "thread_pool")
+ "thread_pool",
+ "unwind_error_handler")
diff --git a/tests/compartment_calls-test.cc b/tests/compartment_calls-test.cc
index 1049560..68fa93f 100644
--- a/tests/compartment_calls-test.cc
+++ b/tests/compartment_calls-test.cc
@@ -58,9 +58,7 @@
TEST(!trusted_stack_has_space(9),
"Trusted stack should not have space for 9 more calls");
- register char *cspRegister asm("csp");
- asm("" : "=C"(cspRegister));
- CHERI::Capability<void> csp{cspRegister};
+ CHERI::Capability<void> csp{__builtin_cheri_stack_get()};
CHERI::Capability<void> originalCSP{switcher_recover_stack()};
csp.address() = originalCSP.address();
TEST(csp == originalCSP,
diff --git a/tests/test-runner.cc b/tests/test-runner.cc
index 824de18..f599156 100644
--- a/tests/test-runner.cc
+++ b/tests/test-runner.cc
@@ -111,6 +111,15 @@
std::string_view{testString, 13});
const std::string S = "I am a walrus"s;
debug_log("Trying to print std::string: {}", S);
+ // Test stack pointer recovery in the root compartment.
+ CHERI::Capability<void> csp{__builtin_cheri_stack_get()};
+ CHERI::Capability<void> originalCSP{switcher_recover_stack()};
+ csp.address() = originalCSP.address();
+ TEST(csp == originalCSP,
+ "Original stack pointer: {}\ndoes not match current stack pointer: {}",
+ originalCSP,
+ csp);
+
run_timed("All tests", []() {
run_timed("Debug helpers (C++)", test_debug_cxx);
run_timed("Debug helpers (C)", test_debug_c);
diff --git a/tests/unwind_cleanup-test.cc b/tests/unwind_cleanup-test.cc
index 42158d2..6cbf390 100644
--- a/tests/unwind_cleanup-test.cc
+++ b/tests/unwind_cleanup-test.cc
@@ -75,6 +75,16 @@
TEST_EQUAL(x, 53, "After longjmp, object should have been modified");
}
+ __noinline void overflow_stack(volatile int *x)
+ {
+ int hugeBuffer[4096];
+ debug_log("Overflowing stack: {}", hugeBuffer);
+ }
+
+ /**
+ * Make sure that we can unwind out of a trap. This will invoke the normal
+ * error handler.
+ */
void test_from_trap()
{
volatile int x = 0;
@@ -89,7 +99,30 @@
x = 53;
}
CHERIOT_END_HANDLER
- TEST_EQUAL(x, 53, "After longjmp, object should have been modified");
+ TEST_EQUAL(
+ x, 53, "After error handler, object should have been modified");
+ }
+
+ /**
+ * Make sure that we can unwind out of a stack overflow. This will invoke
+ * the stackless error handler.
+ */
+ void test_from_stack_overflow()
+ {
+ volatile int x = 0;
+ CHERIOT_DURING
+ {
+ x = 42;
+ overflow_stack(&x);
+ }
+ CHERIOT_HANDLER
+ {
+ debug_log("Error handler");
+ TEST_EQUAL(x, 42, "In the handler, x should have been modified");
+ x = 53;
+ }
+ CHERIOT_END_HANDLER
+ TEST_EQUAL(x, 53, "After handler, object should have been modified");
}
} // namespace
@@ -99,6 +132,12 @@
test_setjmp();
test_on_error();
test_c_macros();
+ // Try these in both orders to make sure that both error handlers correctly
+ // clean up.
+ test_from_trap();
+ test_from_stack_overflow();
+
+ test_from_stack_overflow();
test_from_trap();
debug_log("Test unwind_cleanup passed");
}
diff --git a/tests/xmake.lua b/tests/xmake.lua
index e40c5ab..df7727b 100644
--- a/tests/xmake.lua
+++ b/tests/xmake.lua
@@ -89,6 +89,7 @@
target:values_set("shared_objects", { exampleK = 1024, test_word = 4 }, {expand = false})
end)
test("unwind_cleanup")
+ add_deps("unwind_error_handler")
includes(path.join(sdkdir, "lib"))
