Re: [PATCH] Fix EQ_ATTR_ALT size calculation (PR bootstrap/87417)

["Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com>]

On 24/09/2018 22:49, Ilya Leoshkevich wrote:
> Bootstrap and regtest running on s390x-redhat-linux.
> 
> "r264537: Change EQ_ATTR_ALT to support up to 64 alternatives" changed
> the format of EQ_ATTR_ALT from ii to ww.  This broke the bootstrap on
> 32-bit systems, because the formula for rtx_code_size assumed that only
> certain codes contain HOST_WIDE_INTs.  This did not surface on 64-bit
> systems, because rtunion is 8 bytes anyway, but on 32-bit systems it's
> only 4 bytes.  This resulted in out-of-bounds writes and memory
> corruptions in genattrtab.
> 
> gcc/ChangeLog:
> 
> 2018-09-24  Ilya Leoshkevich  <iii@linux.ibm.com>
> 
> 	PR bootstrap/87417
> 	* rtl.c (rtx_code_size): Take into account that EQ_ATTR_ALT
> 	contains HOST_WIDE_INTs when computing its size.
> ---
>  gcc/rtl.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/gcc/rtl.c b/gcc/rtl.c
> index f9146afcf2c..ca5c25c422f 100644
> --- a/gcc/rtl.c
> +++ b/gcc/rtl.c
> @@ -110,8 +110,7 @@ const enum rtx_class rtx_class[NUM_RTX_CODE] = {
>  
>  const unsigned char rtx_code_size[NUM_RTX_CODE] = {
>  #define DEF_RTL_EXPR(ENUM, NAME, FORMAT, CLASS)				\
> -  (((ENUM) == CONST_INT || (ENUM) == CONST_DOUBLE			\
> -    || (ENUM) == CONST_FIXED || (ENUM) == CONST_WIDE_INT)		\
> +  ((FORMAT)[0] == 'w'							\
>     ? RTX_HDR_SIZE + (sizeof FORMAT - 1) * sizeof (HOST_WIDE_INT)	\
>     : (ENUM) == REG							\
>     ? RTX_HDR_SIZE + sizeof (reg_info)					\
> 

Unfortunately, this leads to a non-functioning stage1 compiler if built
with, eg gcc-4.6.  What happens is that we end up with a static
constructor for rtx_code_size that gets run _after_ a value from the
table is read for the static constructor for cselib.c's

  static pool_allocator value_pool ("value", RTX_CODE_SIZE (VALUE));

and the result is that 0 is passed as the object size.
The pool allocator then obviously does weird things.

I think the safest thing is to go back to using an explicit list of
codes to check; but perhaps we need to get rid of that static
constructor for the pool allocator as well.  This is all somewhat fragile.

R.