Skip to content

[libunwind] Add c18n support on RISC-V #785

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

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
+61 −17
Open

[libunwind] Add c18n support on RISC-V #785

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b4b76b6
libunwind: Remove UNW_ARM64_ECSP
dpgao Aug 11, 2025
e49683b
libunwind: Unwind across c18n compartment boundaries on RISC-V
dpgao Aug 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions libunwind/include/__libunwind_config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,8 @@
# define RISCV_FLEN 0
# endif
# ifdef __CHERI_PURE_CAPABILITY__
# define _LIBUNWIND_CONTEXT_SIZE 96
# define _LIBUNWIND_CURSOR_SIZE 120
# define _LIBUNWIND_CONTEXT_SIZE 98
# define _LIBUNWIND_CURSOR_SIZE 122
# else
# define _LIBUNWIND_CONTEXT_SIZE (32 * (__riscv_xlen + RISCV_FLEN) / 64)
# if __riscv_xlen == 32
Expand Down
4 changes: 0 additions & 4 deletions libunwind/include/libunwind.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -679,10 +679,6 @@ enum {
UNW_ARM64_CLR = 228,
UNW_ARM64_C31 = 229,
UNW_ARM64_CSP = 229,
// Use 240 for ECSP (executive stack pointer). ECSP is not a real DWARF
// register, but we need it to implement c18n-aware unwinding. We pick 240
// because it is far enough away from the range of reserved registers on Arm.
UNW_ARM64_ECSP = 240,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess in an earlier iteration of the patch we really did need it to be a DWARF register?..

};

// 32-bit ARM registers. Numbers match DWARF for ARM spec #3.1 Table 1.
Expand Down
28 changes: 22 additions & 6 deletions libunwind/src/CompartmentInfo.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ struct CompartmentInfo {

static int unwindIfAtBoundary(R &registers) {
#ifdef _LIBUNWIND_HAS_CHERI_LIB_C18N
#ifdef _LIBUNWIND_TARGET_AARCH64
struct dl_c18n_compart_state state;
pint_t pc = registers.getIP();
pint_t tf = registers.getTrustedStack();
Expand All @@ -41,23 +40,40 @@ struct CompartmentInfo {
registers.setTrustedStack(tf);
CHERI_DBG("C18N: SET TRUSTED STACK %#p\n", (void *)tf);

#ifdef _LIBUNWIND_TARGET_AARCH64
registers.setFP((pint_t)state.fp);
CHERI_DBG("C18N: SET FP %#p\n", state.fp);
#endif

registers.setSP((pint_t)state.sp);
CHERI_DBG("C18N: SET SP: %#p\n", state.sp);

registers.setIP((pint_t)state.pc);
CHERI_DBG("C18N: SET IP: %#p\n", state.pc);

#ifdef _LIBUNWIND_TARGET_AARCH64
static constexpr int callee_saved[] = {
UNW_ARM64_C19, UNW_ARM64_C20, UNW_ARM64_C21, UNW_ARM64_C22, UNW_ARM64_C23,
UNW_ARM64_C24, UNW_ARM64_C26, UNW_ARM64_C27, UNW_ARM64_C28, UNW_ARM64_C29
};
#elif defined(_LIBUNWIND_TARGET_RISCV)
static constexpr int callee_saved[] = {
UNW_RISCV_X9, UNW_RISCV_X18, UNW_RISCV_X19, UNW_RISCV_X20, UNW_RISCV_X21,
UNW_RISCV_X22, UNW_RISCV_X23, UNW_RISCV_X24, UNW_RISCV_X25, UNW_RISCV_X26,
UNW_RISCV_X27, UNW_RISCV_X8
};
#endif
static_assert(sizeof(callee_saved) / sizeof(*callee_saved) ==
sizeof(state.regs) / sizeof(*state.regs),
"unexpected number of saved registers");

for (size_t i = 0; i < sizeof(state.regs) / sizeof(*state.regs); ++i) {
registers.setCapabilityRegister(UNW_ARM64_C19 + i, (pint_t)state.regs[i]);
CHERI_DBG("C18N: SET REGISTER: %lu (%s): %#p\n",
UNW_ARM64_C19 + i,
registers.getRegisterName(UNW_ARM64_C19 + i),
registers.setCapabilityRegister(callee_saved[i], (pint_t)state.regs[i]);
CHERI_DBG("C18N: SET REGISTER: %d (%s): %#p\n",
callee_saved[i],
registers.getRegisterName(callee_saved[i]),
state.regs[i]);
}
#endif
#endif // _LIBUNWIND_HAS_CHERI_LIB_C18N
return UNW_STEP_SUCCESS;
}
Expand Down
15 changes: 11 additions & 4 deletions libunwind/src/Registers.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2206,8 +2206,6 @@ inline const char *Registers_arm64::getRegisterName(int regNum) {
return "clr";
case UNW_ARM64_C31:
return "csp";
case UNW_ARM64_ECSP:
return "ecsp";
default:
return "unknown register";
}
Expand Down Expand Up @@ -4554,10 +4552,19 @@ class _LIBUNWIND_HIDDEN Registers_riscv {
void setSP(reg_t value) { _registers[2] = value; }
reg_t getIP() const { return _registers[0]; }
void setIP(reg_t value) { _registers[0] = value; }
#ifdef __CHERI_PURE_CAPABILITY__
reg_t getTrustedStack() const { return _registers[32]; }
void setTrustedStack(reg_t value) { _registers[32] = value; }
#endif

private:
// _registers[0] holds the pc
#ifdef __CHERI_PURE_CAPABILITY__
// _registers[32] holds the trusted stack pointer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just add another field after? PC is in element 0 because it makes indexing the array a bit easier for all the other registers, but there's no benefit AFAICT to (ab)using a 33rd register for the trusted stack pointer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The constructor Registers_riscv(const void *registers) assumes that floating point registers directly follow the _registers array, so it would be quite ugly to adapt the memcpy that initializes the floating point registers.

An alternative is to put the field after the floating point registers. But then the assembly code which saves and restores the registers would have to condition the offset of the trusted stack register on whether __riscv_flen. Again not very ideal.

Registers_arm64 does things properly by wrapping the GPR registers in a structure, so we can just add an element there. I decided that the closest thing to this on RISC-V would be to append to the _registers array. If upstream one day decides to convert this to a structure, then we wouldn't have to do much to pull that change in.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

# if defined(__riscv_flen)
  const uint8_t *floats = static_cast<const uint8_t *>(registers);
#  if defined(__CHERI_PURE_CAPABILITY__)
  floats += sizeof(_whatever_you_call_the_field);
#  endif
  floats += sizeof(_registers);
  memcpy(_floats, floats, sizeof(_floats));
# endif

?

reg_t _registers[33];
#else
reg_t _registers[32];
#endif
# if defined(__riscv_flen)
fp_t _floats[32];
# endif
Expand All @@ -4568,8 +4575,8 @@ inline Registers_riscv::Registers_riscv(const void *registers) {
"riscv registers do not fit into unw_context_t");
memcpy(&_registers, registers, sizeof(_registers));
# ifdef __CHERI_PURE_CAPABILITY__
static_assert(sizeof(_registers) == 0x200,
"expected float registers to be at offset 512");
static_assert(sizeof(_registers) == 0x210,
"expected float registers to be at offset 528");
# elif __riscv_xlen == 32
static_assert(sizeof(_registers) == 0x80,
"expected float registers to be at offset 128");
Expand Down
12 changes: 12 additions & 0 deletions libunwind/src/UnwindRegistersRestore.S
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1330,6 +1330,18 @@ DEFINE_LIBUNWIND_FUNCTION(_ZN9libunwind15Registers_sparc6jumptoEv)
//
.p2align 2
DEFINE_LIBUNWIND_FUNCTION(_ZN9libunwind15Registers_riscv6jumptoEv)
#ifdef _LIBUNWIND_HAS_CHERI_LIB_C18N
// Preserve the argument in a callee-saved register instead of pushing it onto
// the stack because stack unwinding will switch the stack.
cmv cs1, ca0
// Pass the target untrusted stack pointer
lc ca0, (__SIZEOF_CHERI_CAPABILITY__ * 2)(cs1)
// Pass the target trusted stack pointer
lc ca1, (__SIZEOF_CHERI_CAPABILITY__ * 32)(cs1)
call dl_c18n_unwind_trusted_stack
cmv ca0, cs1
#endif

# if defined(__riscv_flen)
.irp i,FROM_0_TO_31
restore_fpr \i, a0
Expand Down
13 changes: 13 additions & 0 deletions libunwind/src/UnwindRegistersSave.S
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1273,6 +1273,19 @@ DEFINE_LIBUNWIND_FUNCTION(__unw_getcontext)
.endr
# endif

#ifdef _LIBUNWIND_HAS_CHERI_LIB_C18N
// Store the trusted stack pointer
caddi csp, csp, -(__SIZEOF_CHERI_CAPABILITY__ * 2)
sc ca0, 0(csp)
sc cra, __SIZEOF_CHERI_CAPABILITY__(csp)
cmv ca0, cra
call dl_c18n_get_trusted_stack
lc ca1, 0(csp)
lc cra, __SIZEOF_CHERI_CAPABILITY__(csp)
caddi csp, csp, (__SIZEOF_CHERI_CAPABILITY__ * 2)
sc ca0, (__SIZEOF_CHERI_CAPABILITY__ * 32)(ca1)
#endif

li a0, 0 // return UNW_ESUCCESS
ret // jump to ra
END_LIBUNWIND_FUNCTION(__unw_getcontext)
Expand Down
2 changes: 1 addition & 1 deletion libunwind/src/assembly.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
#define SEPARATOR %%
#elif defined(__riscv)
#ifdef __CHERI_PURE_CAPABILITY__
# define RISCV_FOFFSET (__SIZEOF_CHERI_CAPABILITY__ * 32)
# define RISCV_FOFFSET (__SIZEOF_CHERI_CAPABILITY__ * 33)
#else
# define RISCV_ISIZE (__riscv_xlen / 8)
# define RISCV_FOFFSET (RISCV_ISIZE * 32)
Expand Down