[cheriintrin.h] Avoid unnecessary truncate in cheri_perms_get() #674
Conversation
In RV32 the return type of __builtin_cheri_type_get() is int since that is what ptrdiff_t is by default. Use __PTRDIFF_TYPE__ instead of long an re-generate the test checks.
The intrinsic returns an i64 for 64-bit so we should use `ptraddr_t` as the underlying type for `cheri_perms_t`.
| #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__ { |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
Good point, I can't remember why I originally made this an enum. Maybe to avoid redefinition warnings in CheriBSD?
There was a problem hiding this comment.
That only works so long as cheriintrin.h is included before cherireg.h, which is the opposite of what style(9) says
There was a problem hiding this comment.
It could also be that the reason was for nicer visualization in debuggers, but I'm not sure if that actually works.
|
Ping? I would prefer to keep the enum to reduce the amount of changes here. If you feel strongly that it should be a series of #defines instead that should IMO be a follow-up change. |
The intrinsic returns an i64 for 64-bit so we should use
ptraddr_tas the underlying type for
cheri_perms_t.