* [PATCH] Fix EQ_ATTR_ALT size calculation (PR bootstrap/87417)
@ 2018-09-24 21:53 Ilya Leoshkevich
2018-09-25 6:44 ` Richard Sandiford
2018-10-25 10:41 ` Richard Earnshaw (lists)
0 siblings, 2 replies; 4+ messages in thread
From: Ilya Leoshkevich @ 2018-09-24 21:53 UTC (permalink / raw)
To: gcc-patches; +Cc: krebbel, rdapp, richard.sandiford, ro, Ilya Leoshkevich
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) \
--
2.19.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix EQ_ATTR_ALT size calculation (PR bootstrap/87417)
2018-09-24 21:53 [PATCH] Fix EQ_ATTR_ALT size calculation (PR bootstrap/87417) Ilya Leoshkevich
@ 2018-09-25 6:44 ` Richard Sandiford
2018-09-25 18:36 ` Jeff Law
2018-10-25 10:41 ` Richard Earnshaw (lists)
1 sibling, 1 reply; 4+ messages in thread
From: Richard Sandiford @ 2018-09-25 6:44 UTC (permalink / raw)
To: Ilya Leoshkevich; +Cc: gcc-patches, krebbel, rdapp, ro
Ilya Leoshkevich <iii@linux.ibm.com> writes:
> 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.
OK. Thanks for the quick fix.
Richard
> ---
> 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) \
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix EQ_ATTR_ALT size calculation (PR bootstrap/87417)
2018-09-25 6:44 ` Richard Sandiford
@ 2018-09-25 18:36 ` Jeff Law
0 siblings, 0 replies; 4+ messages in thread
From: Jeff Law @ 2018-09-25 18:36 UTC (permalink / raw)
To: Ilya Leoshkevich, gcc-patches, krebbel, rdapp, ro, rdsandiford
On 9/24/18 11:15 PM, Richard Sandiford wrote:
> Ilya Leoshkevich <iii@linux.ibm.com> writes:
>> 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.
>
> OK. Thanks for the quick fix.
And I'll note that my testers were picking up this failure on various
targets -- they're all happy now.
jeff
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix EQ_ATTR_ALT size calculation (PR bootstrap/87417)
2018-09-24 21:53 [PATCH] Fix EQ_ATTR_ALT size calculation (PR bootstrap/87417) Ilya Leoshkevich
2018-09-25 6:44 ` Richard Sandiford
@ 2018-10-25 10:41 ` Richard Earnshaw (lists)
1 sibling, 0 replies; 4+ messages in thread
From: Richard Earnshaw (lists) @ 2018-10-25 10:41 UTC (permalink / raw)
To: Ilya Leoshkevich, gcc-patches; +Cc: krebbel, rdapp, richard.sandiford, ro
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-10-25 9:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-24 21:53 [PATCH] Fix EQ_ATTR_ALT size calculation (PR bootstrap/87417) Ilya Leoshkevich
2018-09-25 6:44 ` Richard Sandiford
2018-09-25 18:36 ` Jeff Law
2018-10-25 10:41 ` Richard Earnshaw (lists)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).