-
Notifications
You must be signed in to change notification settings - Fork 7
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
add support for experimental CHasSpace instruction #31
base: master
Are you sure you want to change the base?
Conversation
cheri/cheri_insts.sail
Outdated
@@ -119,6 +119,23 @@ function clause execute (CGetAndAddr(rd, cb, rs)) = | |||
wGPR(rd) = capVal.address & rs_val; | |||
} | |||
|
|||
union clause ast = CHasSpace : (regno, regno, regno) | |||
function clause execute (CHasSpace (rd, cb, rs)) = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
function clause execute (CHasSpace (rd, cb, rs)) = | |
function clause execute (CHasSpace(rd, cb, rs)) = |
cheri/cheri_insts.sail
Outdated
if ((cb_cursor + rs_val) <= cb_top) & | ||
(cb_cursor < cb_top) & | ||
(cb_cursor >= cb_base) then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be nicer using n+1-bit arithmetic for the top check. Something like:
if ((cb_cursor + rs_val) <= cb_top) & | |
(cb_cursor < cb_top) & | |
(cb_cursor >= cb_base) then | |
let req_top : CapLenBits = zero_extend(cb_cursor) + zero_extend(rs_val); | |
if (cb_cursor >= cb_base) & (req_top <= cb_top) then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that cb_cursor
is an int
so you can't use zero_extend
on it (you could use cb.address
but you need the base and top as bits
to compare with). The cheri-mips spec as whole uses ints rather than bits because that's what the original pseudocode did but cheri-riscv has moved to bits in some places. The advantage of ints when bounds checking is you don't have to worry about overflow (ints are unbounded in sail) but obviously hardware has to use a fixed width.
On cheri-riscv I factored the bounds check into a inCapBounds
function which might be nicer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I'll refactor into an inCapBounds
function and use unbounded integers to be in line with the rest of cheri-mips. Please let me know whether the types etc. look OK.
I noticed that the cheri-riscv inCapBounds doesn't check that cursor < top, so that an invocation of inCapBounds with addr == top and size == 0 will return true even though the address isn't in bounds for the capability. Was this done intentionally? I was thinking that it would make more sense for CHasSpace
to return false in that case, so I have implemented the mips inCapBounds
accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a good point. I'm just wondering whether there is a bug in CSetBounds (and others) on cheri-riscv because of it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The behaviour of allowing that seems perfectly natural to me, and special-casing it would be strange. There's nothing wrong architecturally with a [top, top)
capability. Microarchitecturally, adding that extra check would just make the (already expensive) bounds check longer. From a software perspective, this allows you to take any valid C pointer and CSetBounds it with length <= remaining_length
, which includes one-past-the-end pointers. Purely abstractly, the [top, top)
capabilities are a subset of [bot, top)
, and other than precision issues, we should be able to get any subset of [bot, top)
, even the trivial ones.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To summarize the discussion from yesterday, I believe we decided that it is fine in general for instructions to allow for the creation of capabilities of zero length and base equal to the parent capability's top. The functionality of CHasSpace should be similar and return true if the given address is equal to the given capability's top and the size is 0. However, checking whether a cursor is in bounds should return false when the cursor is equal to top.
In line with this, I have modified the helper function for sail-mips to have the same checks as the sail-riscv inCapBounds
function. But I have named it capHasSpace
to make the difference between this check and the in-bounds check more clear.
CInBounds for a capability can be implemented as CHasSpace with the capability's cursor and a size of 1.
cheri/cheri_insts.sail
Outdated
if ((cb_cursor + rs_val) <= cb_top) & | ||
(cb_cursor < cb_top) & | ||
(cb_cursor >= cb_base) then | ||
wGPR(rd) = zero_extend(to_bits(1,1)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wGPR(rd) = zero_extend(to_bits(1,1)) | |
wGPR(rd) = zero_extend(0b1) |
And maybe do something like either CTestSubset or CGetType/CToPtr to avoid multiple wGPR(rd) =
statements.
3c19e95
to
0cf87b6
Compare
also add capHasSpace helper
No description provided.