On Sun, Nov 3, 2024 at 11:23 PM Lewis Hyatt <[email protected]> wrote:
>
> libcpp makes use of the cpp_buffer pfile->a_buff to store things while it is
> handling macros. It uses it to store pointers (cpp_hashnode*, for macro
> arguments) and cpp_macro objects. This works fine because a cpp_hashnode*
> and a cpp_macro have the same alignment requirement on either 32-bit or
> 64-bit systems (namely, the same alignment as a pointer.)
>
> When 64-bit location_t is enabled on a 32-bit sytem, the alignment
> requirement may cease to be the same, because the alignment requirement of a
> cpp_macro object changes to that of a uint64_t, which be larger than that of
> a pointer. It's not the case for x86 32-bit, but for example, on sparc, a
> pointer has 4-byte alignment while a uint64_t has 8. In that case,
> intermixing the two within the same cpp_buffer leads to a misaligned
> access. The code path that triggers this is the one in _cpp_commit_buff in
> which a hash table with its own allocator (i.e. ggc) is not being used, so
> it doesn't happen within the compiler itself, but it happens in the other
> libcpp clients, such as genmatch.
>
> Fix that up by ensuring _cpp_commit_buff commits a fully aligned chunk of the
> buffer, so it's ready for anything it may be used for next.
>
> For good measure, also modify CPP_ALIGN so that it guarantees to return an
> alignment at least the size of location_t. Currently it returns the max of
> a pointer and a double. I am not aware of any platform where a double may
> have smaller alignment than a uint64_t, but it does not hurt to add
> location_t here to be sure.
OK.
Thanks,
Richard.
> libcpp/ChangeLog:
>
> * lex.cc (_cpp_commit_buff): Make sure that the buffer is properly
> aligned for the next allocation.
> * internal.h (struct dummy): Make sure alignment is large enough for
> a location_t, just in case.
> ---
> libcpp/internal.h | 1 +
> libcpp/lex.cc | 10 ++++++++--
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/libcpp/internal.h b/libcpp/internal.h
> index e65198e89da..358e77cd622 100644
> --- a/libcpp/internal.h
> +++ b/libcpp/internal.h
> @@ -85,6 +85,7 @@ struct dummy
> {
> double d;
> int *p;
> + location_t l;
> } u;
> };
>
> diff --git a/libcpp/lex.cc b/libcpp/lex.cc
> index 849447eb4d7..858970b5d17 100644
> --- a/libcpp/lex.cc
> +++ b/libcpp/lex.cc
> @@ -4997,7 +4997,8 @@ _cpp_aligned_alloc (cpp_reader *pfile, size_t len)
> void *
> _cpp_commit_buff (cpp_reader *pfile, size_t size)
> {
> - void *ptr = BUFF_FRONT (pfile->a_buff);
> + const auto buff = pfile->a_buff;
> + void *ptr = BUFF_FRONT (buff);
>
> if (pfile->hash_table->alloc_subobject)
> {
> @@ -5006,7 +5007,12 @@ _cpp_commit_buff (cpp_reader *pfile, size_t size)
> ptr = copy;
> }
> else
> - BUFF_FRONT (pfile->a_buff) += size;
> + {
> + BUFF_FRONT (buff) += size;
> + /* Make sure the remaining space is maximally aligned for whatever this
> + buffer holds next. */
> + BUFF_FRONT (buff) += BUFF_ROOM (buff) % DEFAULT_ALIGNMENT;
> + }
>
> return ptr;
> }
