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.

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

Jump to bottom

[cheriintrin.h] Avoid unnecessary truncate in cheri_perms_get() #674

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

[cheriintrin.h] Avoid unnecessary truncate in cheri_perms_get() #674

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
08c15d5
Fix cheri_otype_t in cheriintrin.h for RV32
arichardson Jan 12, 2023
b24e4a0
[cheriintrin.h] Avoid unnecessary truncate in cheri_perms_get()
arichardson Jan 12, 2023
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
11 changes: 7 additions & 4 deletions clang/lib/Headers/cheriintrin.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
#define cheri_bounds_set_exact(x, y) __builtin_cheri_bounds_set_exact((x), (y))

/* Object types, sealing and unsealing: */
typedef long cheri_otype_t;
typedef __PTRDIFF_TYPE__ cheri_otype_t;
#if defined(__mips__) || defined(__riscv)
/* CHERI-MIPS and CHERI-RISC-V use negative numbers for hardware-interpreted
* otypes */
Expand Down Expand Up @@ -67,7 +67,10 @@ typedef long cheri_otype_t;
#define cheri_type_copy(x, y) __builtin_cheri_cap_type_copy((x), (y))

/* Capability permissions: */
typedef enum __attribute__((flag_enum, enum_extensibility(open))) {
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wfixed-enum-extension"
/* Use __PTRADDR_TYPE__ as the underlying type to avoid unnecessary truncates */
typedef enum __attribute__((flag_enum, enum_extensibility(open))) : __PTRADDR_TYPE__ {
Copy link
Member

Choose a reason for hiding this comment

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

What is the point of this enum? Are we not better off with a list of #define CHERI_PERM_FOO (__CHERI_CAP_PERMISSION_FOO__)? We can make sure the preprocessor gives the latter the right type (or cast each one here, but that seems worse).

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, I can't remember why I originally made this an enum. Maybe to avoid redefinition warnings in CheriBSD?

Copy link
Member

Choose a reason for hiding this comment

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

That only works so long as cheriintrin.h is included before cherireg.h, which is the opposite of what style(9) says

Copy link
Member Author

Choose a reason for hiding this comment

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

It could also be that the reason was for nicer visualization in debuggers, but I'm not sure if that actually works.

CHERI_PERM_GLOBAL = __CHERI_CAP_PERMISSION_GLOBAL__,
CHERI_PERM_EXECUTE = __CHERI_CAP_PERMISSION_PERMIT_EXECUTE__,
CHERI_PERM_LOAD = __CHERI_CAP_PERMISSION_PERMIT_LOAD__,
Expand All @@ -82,9 +85,9 @@ typedef enum __attribute__((flag_enum, enum_extensibility(open))) {
/* TODO: architecture-dependent permissions */
} cheri_perms_t;
#define cheri_perms_get(x) ((cheri_perms_t)(__builtin_cheri_perms_get(x)))
#define cheri_perms_and(x, y) __builtin_cheri_perms_and((x), (__SIZE_TYPE__)(y))
#define cheri_perms_and(x, y) __builtin_cheri_perms_and((x), (cheri_perms_t)(y))
#define cheri_perms_clear(x, y) \
__builtin_cheri_perms_and((x), ~(__SIZE_TYPE__)(y))
__builtin_cheri_perms_and((x), ~(__PTRADDR_TYPE__)(y))

/* Accessors for capability registers. Currently exposes DDC and PCC. */
#define cheri_ddc_get() __builtin_cheri_global_data_get()
Expand Down
Loading