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.

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

[CHERIoT] CHERIOT R_RISCV_CHERI_COMPARTMENT_SIZE relocation out of range error #696

Open
ajrn1998 opened this issue May 9, 2023 · 1 comment
Assignees

Comments

@ajrn1998
Copy link

ajrn1998 commented May 9, 2023

CHERIOT linker fails with relocation out of range error when building the nettle-aes embench benchmark: https://github.com/embench/embench-iot/blob/d9b30cdf805133bef9db5f7ecf84ae1ce8124291/src/nettle-aes/nettle-aes.c#L52-L59

The problem can be more simply reproduced by just declaring a big structure type like aes_table and initilise a struct of this type such as _aes_encrypt_table: https://github.com/embench/embench-iot/blob/d9b30cdf805133bef9db5f7ecf84ae1ce8124291/src/nettle-aes/nettle-aes.c#L88-L389. By simply switching the AES_TABLE_SIZE #define to 2, the linker succeeds.

It seems that the relocation is always set to use the CSetBoundsImm instruction with a max 12 bits immediate. Is there a reason why it would not use the CSetBounds instruction and relax only when it is possible to fit it in the CSetBoundsImm instruction?

linker_error

@davidchisnall davidchisnall self-assigned this May 9, 2023
@davidchisnall davidchisnall changed the title CHERIOT R_RISCV_CHERI_COMPARTMENT_SIZE relocation out of range error [CHERIoT] CHERIOT R_RISCV_CHERI_COMPARTMENT_SIZE relocation out of range error May 9, 2023
@davidchisnall
Copy link
Member

This is a known (but not documented, sorry!) limitation. Unfortunately, this is a slightly different shape of relaxation to anything currently done and so it's not something that we want to add until we've rebased on a newer version of LLVM that has the upstream support for linker relaxations merged so that we do not diverge too far. The relaxation sequence is complex because there are a bunch of cases to consider:

  • CSetBounds with an immediate offset
  • CSetBounds with a register materialised from addiu + c.lui
  • CSetBounds with a register materialised from c.addiu + c.lui
  • CSetBounds with a register materialised from addiu + c.slli
  • CSetBounds with a register materialised from addiu + lui

We would have to emit the third form (which then requires two new relocation types) and relax to one of the others and this kind of relocation pattern is not yet well supported in lld.

In general, compartmentalised code that respects the principle of least privilege rarely has single globals that are over 4 KiB (we've only seen it in one other place so far) and I'd consider it a sign of potential bugs. It looks as if the AES code that you're using places all of the s-boxes in an array. The mBedTLS version (which we're using) places them in separate globals, which gives finer-grained bounds checking and is probably a better approach. In particular, unless you enable sub-object bounds, you won't get bounds errors if the AES implementation walks off one s-box and into the next one with the code structured this way.

Even implementing this relaxation is not ideal because it adds an extra live register, which causes worse code generation elsewhere (and often stack spills and restores), so we'd probably want to guard this behind a flag to avoid making code worse in the common case.

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

No branches or pull requests

2 participants