match: Properly align heapframes for CHERI/Arm's Morello prototype #72
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
On CHERI, and thus Arm's Morello prototype, pointers are represented as hardware capabilities, which consist of both an integer address and additional metadata, meaning they are twice the size of the platform's size_t type, i.e. 16 bytes on a 64-bit system. The ovector member of heapframe happens to only be 8 byte aligned, and so computing frame_size ends up with a multiple of 8 but not 16. Whilst the first frame is always suitably aligned, this then misaligns the frame that follows it, resulting in an alignment fault when storing a pointer to Fecode at the start of match. Thus, round up frame_size to a multiple of heapframe's alignment to ensure alignment is preserved. This can be completely optimised away on traditional architectures and, since CHERI's capabilities are in fact 2 * sizeof(PCRE2_SIZE) bytes in size, the variable part of the expression is also proven to be a multiple of the alignment and so the aligning gets folded into the offsetof part by adding an additional 8, so no dynamic alignment code is needed even on CHERI architectures.
|
Nice patch, thank you. |
jrtc27
referenced
this pull request
in CTSRD-CHERI/cheribsd-ports
Apr 3, 2022
We could use the tiny flavor for devel/git to disable the unsupported options. However, devel/git@tiny seems not to compile without perl. We probably should change the default options only for CHERI-enabled architectures leaving them as in upstream for the rest of architectures. We could disable the default options in an options directory mounted by Poudriere instead of changing it in the cheribsd-ports ports tree. We'll introduce these improvements separately.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
On CHERI, and thus Arm's Morello prototype, pointers are represented as
hardware capabilities, which consist of both an integer address and
additional metadata, meaning they are twice the size of the platform's
size_t type, i.e. 16 bytes on a 64-bit system. The ovector member of
heapframe happens to only be 8 byte aligned, and so computing frame_size
ends up with a multiple of 8 but not 16. Whilst the first frame is
always suitably aligned, this then misaligns the frame that follows it,
resulting in an alignment fault when storing a pointer to Fecode at the
start of match.
Thus, round up frame_size to a multiple of heapframe's alignment to
ensure alignment is preserved. This can be completely optimised away on
traditional architectures and, since CHERI's capabilities are in fact
2 * sizeof(PCRE2_SIZE) bytes in size, the variable part of the
expression is also proven to be a multiple of the alignment and so the
aligning gets folded into the offsetof part by adding an additional 8,
so no dynamic alignment code is needed even on CHERI architectures.