On Sun, Nov 17, 2024 at 4:25 AM Lewis Hyatt <lhy...@gmail.com> wrote:
>
> The array gimple_ops_offset_[], which is used to find the trailing op[]
> array for a given gimple struct, is computed assuming that op[] will be
> found at sizeof(tree) bytes away from the end of the struct. This is only
> correct if the alignment requirement of a pointer is the same as the
> alignment requirement of the struct, otherwise there will be padding bytes
> that invalidate the calculation. On 64-bit platforms, this generally works
> fine because a pointer has 8-byte alignment and none of the structs make use
> of more than that. On 32-bit platforms, it also currently works fine because
> there are no 64-bit integers in the gimple structs. There are 32-bit
> platforms (e.g. sparc) on which a pointer has 4-byte alignment and a
> uint64_t has 8-byte alignment. On such platforms, adding a uint64_t to the
> gimple structs (as will take place when location_t is changed to be 64-bit)
> causes gimple_ops_offset_ to be 4 bytes too large.
>
> It would be nice to use offsetof() to compute the offset exactly, but
> offsetof() is not guaranteed to work for these types, because they use
> inheritance and so are not standard layout types. This patch attempts to
> detect the presence of tail padding by detecting when such padding is reused
> by inheritance; the padding should generally be reused for the same reason
> that offsetof() is not available, namely that all the relevant types use
> inheritance. One could envision systems on which this fix does not go far
> enough (e.g., if the ABI forbids reuse of tail padding), but it makes things
> better without affecting anything that currently works.
>
> gcc/ChangeLog:
>
> * gimple.cc (get_tail_padding_adjustment): New function.
> (DEFGSSTRUCT): Adjust the computation of gimple_ops_offset_ to be
> correct in the presence of tail padding.
> ---
> gcc/gimple.cc | 34 +++++++++++++++++++++++++++++-----
> 1 file changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/gimple.cc b/gcc/gimple.cc
> index f7b313be40e..f0a642f5b51 100644
> --- a/gcc/gimple.cc
> +++ b/gcc/gimple.cc
> @@ -52,12 +52,36 @@ along with GCC; see the file COPYING3. If not see
> #include "ipa-modref.h"
> #include "dbgcnt.h"
>
> -/* All the tuples have their operand vector (if present) at the very bottom
> - of the structure. Therefore, the offset required to find the
> - operands vector the size of the structure minus the size of the 1
> - element tree array at the end (see gimple_ops). */
> +/* All the tuples have their operand vector (if present) at the very bottom
> of
> + the structure. Therefore, the offset required to find the operands
> vector is
> + the size of the structure minus the size of the 1-element tree array at
> the
> + end (see gimple_ops). An adjustment may be required if there is tail
> + padding, as may happen on a host (e.g. sparc) where a pointer has 4-byte
> + alignment while a uint64_t has 8-byte alignment.
> +
> + Unfortunately, we can't use offsetof to do this computation 100%
> + straightforwardly, because these structs use inheritance and so are not
> + standard layout types. However, the fact that they are not standard
> layout
> + types also means that tail padding will be reused in inheritance, which
> makes
> + it possible to check for the problematic case with the following logic
> + instead. If tail padding is detected, the offset should be decreased
> + accordingly. */
> +
> +template<typename G>
> +static constexpr size_t
> +get_tail_padding_adjustment ()
> +{
> + struct padding_check : G
> + {
> + tree t;
> + };
> + return sizeof (padding_check) == sizeof (G) ? sizeof (tree) : 0;
> +}
> +
> #define DEFGSSTRUCT(SYM, STRUCT, HAS_TREE_OP) \
> - (HAS_TREE_OP ? sizeof (struct STRUCT) - sizeof (tree) : 0),
> + (HAS_TREE_OP \
> + ? sizeof (STRUCT) - sizeof (tree) - get_tail_padding_adjustment<STRUCT>
> () \
> + : 0),
I wonder if we cannot simply use offsetof (STRUCT, ops) and some
"trick" to avoid
parsing/sanitizing this when HAS_TREE_OP is false? Maybe even a
template <class T, bool has_offset>
constexpr size_t ops_offset () { return 0; }
template <class T>
constexpr size_t ops_offset<T, true> ()
{
return offsetof (T, ops);
/* or T x; return (char *)x.ops - (char *)x; */
}
? That is I don't like the "indirect" adjustment via computing the padding.
Richard.
> EXPORTED_CONST size_t gimple_ops_offset_[] = {
> #include "gsstruct.def"
> };
