match: Properly align heapframes for CHERI/Arm's Morello prototype (#72)

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.
This commit is contained in:
Jessica Clarke 2022-01-04 17:06:14 +00:00 committed by GitHub
parent 534b4760e3
commit 04ecb267c0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 21 additions and 4 deletions

View File

@ -838,6 +838,17 @@ multiple of PCRE2_SIZE. See various comments above. */
typedef char check_heapframe_size[ typedef char check_heapframe_size[
((sizeof(heapframe) % sizeof(PCRE2_SIZE)) == 0)? (+1):(-1)]; ((sizeof(heapframe) % sizeof(PCRE2_SIZE)) == 0)? (+1):(-1)];
/* Structure for computing the alignment of heapframe. */
typedef struct heapframe_align {
char unalign; /* Completely unalign the current offset */
heapframe frame; /* Offset is its alignment */
} heapframe_align;
/* This define is the minimum alignment required for a heapframe, in bytes. */
#define HEAPFRAME_ALIGNMENT offsetof(heapframe_align, frame)
/* Structure for passing "static" information around between the functions /* Structure for passing "static" information around between the functions
doing traditional NFA matching (pcre2_match() and friends). */ doing traditional NFA matching (pcre2_match() and friends). */

View File

@ -6781,10 +6781,16 @@ the pattern. It is not used at all if there are no capturing parentheses.
The last of these is changed within the match() function if the frame vector The last of these is changed within the match() function if the frame vector
has to be expanded. We therefore put it into the match block so that it is has to be expanded. We therefore put it into the match block so that it is
correct when calling match() more than once for non-anchored patterns. */ correct when calling match() more than once for non-anchored patterns.
frame_size = offsetof(heapframe, ovector) + We must also pad frame_size for alignment to ensure subsequent frames are as
re->top_bracket * 2 * sizeof(PCRE2_SIZE); aligned as heapframe. Whilst ovector is word-aligned due to being a PCRE2_SIZE
array, that does not guarantee it is suitably aligned for pointers, as some
architectures have pointers that are larger than a size_t. */
frame_size = (offsetof(heapframe, ovector) +
re->top_bracket * 2 * sizeof(PCRE2_SIZE) + HEAPFRAME_ALIGNMENT - 1) &
~(HEAPFRAME_ALIGNMENT - 1);
/* Limits set in the pattern override the match context only if they are /* Limits set in the pattern override the match context only if they are
smaller. */ smaller. */
@ -6828,7 +6834,7 @@ mb->match_frames_top =
to avoid uninitialized memory read errors when it is copied to a new frame. */ to avoid uninitialized memory read errors when it is copied to a new frame. */
memset((char *)(mb->match_frames) + offsetof(heapframe, ovector), 0xff, memset((char *)(mb->match_frames) + offsetof(heapframe, ovector), 0xff,
re->top_bracket * 2 * sizeof(PCRE2_SIZE)); frame_size - offsetof(heapframe, ovector));
/* Pointers to the individual character tables */ /* Pointers to the individual character tables */