| Message ID | 91b01cb1-b0e5-f39b-a437-16252f421253@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | correct maximum valid alignment in error message (PR 89812) | expand |
> On Mar 24, 2019, at 8:21 PM, Martin Sebor <msebor@gmail.com> wrote: > > ... > PS I have a couple of questions related to the affected code: > 1) Does GCC support building with compilers where int is not 32 > bits wide, or where BITS_PER_UNIT is not 3? (I.e., either is > less or more?) Yes. pdp11 int can be 16 bits (or 32 bits, if you say -mint32). > 2) Is there a supported target that doesn't have __INT64_TYPE__? > (And if so, how do I find it in a test? I couldn't find > anythhing in target-supports.exp). Maybe one of the tiny targets. pdp11 does support int64. paul
On 3/24/19 6:21 PM, Martin Sebor wrote: > The error issued when the aligned attribute argument is too big > to be represented is incorrect: it says the maximum alignment > is 1U << 31 when it should actually be 1 << 28. This was a typo > introduced when the error message was enhanced earlier in GCC 9. > > The test I added to verify the fix for the typo exposed another > bug introduced in the same commit as the incorrect value in > the error message: assuming that the attribute aligned argument > fits in SHWI. > > The attached patch corrects both problems. It has been tested > on x86_64-linux. I will commit it as obvious sometime this week > unless there are any objections or suggestions for changes. > > Martin > > PS I have a couple of questions related to the affected code: > 1) Does GCC support building with compilers where int is not 32 > bits wide, or where BITS_PER_UNIT is not 3? (I.e., either is > less or more?) We've certainly supported 16 bit ints in the past. H8/300 would be an example. It defaults to 16 bit ints. But I don't think we've tested that in a very long time -- my tester is only testing with -mint32. Look for INT_TYPE_SIZE in config/*/*.h We've never supported bootstrapping in that mode, just crosses. AVR is probably the most interesting as it even has an flag to make "int" be 8 bits. It's probably the best tested target in this space. BITS_PER_UNIT is more of a hardware characteristic. It's generally 8. THough I thought one of the TI chips defined it to 32. I suspect you weren't really looking for BITS_PER_UNIT here. > 2) Is there a supported target that doesn't have __INT64_TYPE__? > (And if so, how do I find it in a test? I couldn't find > anythhing in target-supports.exp). Some of the embedded ports most likely. Again, H8/300 might be a port to look at. You can dig through config/newlib-stdint.h to see how the sizes of standard types are defined for newlib. You then have to dig into how the port defines LONG_TYPE_SIZE, LONG_LONG_TYPE_SIZE, etc etc. Given a cross, you can use sizeof (whatever) to get the sizes if you don't mind slogging through a bit of assembly code to figure things out. jeff
> On Mar 25, 2019, at 12:07 PM, Jeff Law <law@redhat.com> wrote: > >> ... >> 1) Does GCC support building with compilers where int is not 32 >> bits wide, or where BITS_PER_UNIT is not 3? (I.e., either is >> less or more?) > We've certainly supported 16 bit ints in the past. H8/300 would be an > example. It defaults to 16 bit ints. But I don't think we've tested > that in a very long time -- my tester is only testing with -mint32. pdp11 is set up the same way (16 bit int is the default, -mint32 supported). I run most of my pdp11 tests (including gcc testsuite runs) with the default 16 bit ints. I haven't seen issues related to int size handing in the GCC core. paul
On 3/25/19 10:07 AM, Jeff Law wrote: > On 3/24/19 6:21 PM, Martin Sebor wrote: >> The error issued when the aligned attribute argument is too big >> to be represented is incorrect: it says the maximum alignment >> is 1U << 31 when it should actually be 1 << 28. This was a typo >> introduced when the error message was enhanced earlier in GCC 9. >> >> The test I added to verify the fix for the typo exposed another >> bug introduced in the same commit as the incorrect value in >> the error message: assuming that the attribute aligned argument >> fits in SHWI. >> >> The attached patch corrects both problems. It has been tested >> on x86_64-linux. I will commit it as obvious sometime this week >> unless there are any objections or suggestions for changes. >> >> Martin >> >> PS I have a couple of questions related to the affected code: >> 1) Does GCC support building with compilers where int is not 32 >> bits wide, or where BITS_PER_UNIT is not 3? (I.e., either is >> less or more?) > We've certainly supported 16 bit ints in the past. H8/300 would be an > example. It defaults to 16 bit ints. But I don't think we've tested > that in a very long time -- my tester is only testing with -mint32. > > Look for INT_TYPE_SIZE in config/*/*.h > > We've never supported bootstrapping in that mode, just crosses. Thanks, that's what I was after: whether GCC can build natively with such a compiler where sizeof (int) != 32. Sounds like it can't, i.e., HOST_BITS_PER_INT is always at least 32. Or do you suppose it's always exactly 32? > > AVR is probably the most interesting as it even has an flag to make > "int" be 8 bits. It's probably the best tested target in this space. > > BITS_PER_UNIT is more of a hardware characteristic. It's generally 8. > THough I thought one of the TI chips defined it to 32. I suspect you > weren't really looking for BITS_PER_UNIT here. I think using BITS_PER_UNIT here is actually another bug in the function: it should be using CHAR_BITS instead, like so: if (log2align >= HOST_BITS_PER_INT - exact_log2 (CHAR_BITS)) { error ("requested alignment %qE exceeds maximum %u", align, 1U << (HOST_BITS_PER_INT - exact_log2 (CHAR_BITS) - 1)); return -1; } > >> 2) Is there a supported target that doesn't have __INT64_TYPE__? >> (And if so, how do I find it in a test? I couldn't find >> anythhing in target-supports.exp). > Some of the embedded ports most likely. Again, H8/300 might be a port > to look at. > > You can dig through config/newlib-stdint.h to see how the sizes of > standard types are defined for newlib. You then have to dig into how > the port defines LONG_TYPE_SIZE, LONG_LONG_TYPE_SIZE, etc etc. > > > Given a cross, you can use sizeof (whatever) to get the sizes if you > don't mind slogging through a bit of assembly code to figure things out. Thanks. I was concerned about the test I added breaking on systems that don't define __INT64_TYPE__, but I see other tests that assume that __INT64_TYPE__ exists, so if it does break, it won't be the only one. Martin
On 3/25/19 11:04 AM, Martin Sebor wrote: > On 3/25/19 10:07 AM, Jeff Law wrote: >> On 3/24/19 6:21 PM, Martin Sebor wrote: >>> The error issued when the aligned attribute argument is too big >>> to be represented is incorrect: it says the maximum alignment >>> is 1U << 31 when it should actually be 1 << 28. This was a typo >>> introduced when the error message was enhanced earlier in GCC 9. >>> >>> The test I added to verify the fix for the typo exposed another >>> bug introduced in the same commit as the incorrect value in >>> the error message: assuming that the attribute aligned argument >>> fits in SHWI. >>> >>> The attached patch corrects both problems. It has been tested >>> on x86_64-linux. I will commit it as obvious sometime this week >>> unless there are any objections or suggestions for changes. >>> >>> Martin >>> >>> PS I have a couple of questions related to the affected code: >>> 1) Does GCC support building with compilers where int is not 32 >>> bits wide, or where BITS_PER_UNIT is not 3? (I.e., either is >>> less or more?) >> We've certainly supported 16 bit ints in the past. H8/300 would be an >> example. It defaults to 16 bit ints. But I don't think we've tested >> that in a very long time -- my tester is only testing with -mint32. >> >> Look for INT_TYPE_SIZE in config/*/*.h >> >> We've never supported bootstrapping in that mode, just crosses. > > Thanks, that's what I was after: whether GCC can build natively > with such a compiler where sizeof (int) != 32. Sounds like it > can't, i.e., HOST_BITS_PER_INT is always at least 32. Or do > you suppose it's always exactly 32? At least 32 for HOST_BITS_PER_INT, I think we've supported 64 HOST_BITS_PER_INT as well. HOST_BITS_PER_INT is defined indirectly via CHAR_BIT * SIZEOF_INT, both of which are determined during the configure phase. > >> >> AVR is probably the most interesting as it even has an flag to make >> "int" be 8 bits. It's probably the best tested target in this space. >> >> BITS_PER_UNIT is more of a hardware characteristic. It's generally 8. >> THough I thought one of the TI chips defined it to 32. I suspect you >> weren't really looking for BITS_PER_UNIT here. > > I think using BITS_PER_UNIT here is actually another bug in > the function: it should be using CHAR_BITS instead, like so: > > if (log2align >= HOST_BITS_PER_INT - exact_log2 (CHAR_BITS)) > { > error ("requested alignment %qE exceeds maximum %u", > align, 1U << (HOST_BITS_PER_INT - exact_log2 (CHAR_BITS) - 1)); > return -1; > } Isn't CHAR_BIT a function of the host, not the target? In which case I don't think it's right since supported alignments are primarily a target property. > > Thanks. I was concerned about the test I added breaking on > systems that don't define __INT64_TYPE__, but I see other tests > that assume that __INT64_TYPE__ exists, so if it does break, it > won't be the only one. I wouldn't stress too much about it. The AVR guys might come back with a patch to adjust the test. But again, I wouldn't worry about it until they do. jeff
On 3/24/19 6:21 PM, Martin Sebor wrote: > The error issued when the aligned attribute argument is too big > to be represented is incorrect: it says the maximum alignment > is 1U << 31 when it should actually be 1 << 28. This was a typo > introduced when the error message was enhanced earlier in GCC 9. > > The test I added to verify the fix for the typo exposed another > bug introduced in the same commit as the incorrect value in > the error message: assuming that the attribute aligned argument > fits in SHWI. > > The attached patch corrects both problems. It has been tested > on x86_64-linux. I will commit it as obvious sometime this week > unless there are any objections or suggestions for changes. > > Martin > > PS I have a couple of questions related to the affected code: > 1) Does GCC support building with compilers where int is not 32 > bits wide, or where BITS_PER_UNIT is not 3? (I.e., either is > less or more?) > 2) Is there a supported target that doesn't have __INT64_TYPE__? > (And if so, how do I find it in a test? I couldn't find > anythhing in target-supports.exp). > > gcc-89812.diff > > PR c/89812 - incorrect maximum in error: requested alignment ‘536870912’ exceeds maximum 2147483648 > > gcc/c-family/ChangeLog: > > PR c/89812 > * c-common.c (check_user_alignment): Rename local. Correct maximum > alignment in diagnostic. Avoid assuming argument fits in SHWI, > convert it to UHWI when it fits. > > gcc/testsuite/ChangeLog: > > PR c/89812 > * gcc.dg/attr-aligned-3.c: New test. OK jeff
On 3/25/19 11:14 AM, Jeff Law wrote: > On 3/25/19 11:04 AM, Martin Sebor wrote: >> On 3/25/19 10:07 AM, Jeff Law wrote: >>> On 3/24/19 6:21 PM, Martin Sebor wrote: >>>> The error issued when the aligned attribute argument is too big >>>> to be represented is incorrect: it says the maximum alignment >>>> is 1U << 31 when it should actually be 1 << 28. This was a typo >>>> introduced when the error message was enhanced earlier in GCC 9. >>>> >>>> The test I added to verify the fix for the typo exposed another >>>> bug introduced in the same commit as the incorrect value in >>>> the error message: assuming that the attribute aligned argument >>>> fits in SHWI. >>>> >>>> The attached patch corrects both problems. It has been tested >>>> on x86_64-linux. I will commit it as obvious sometime this week >>>> unless there are any objections or suggestions for changes. >>>> >>>> Martin >>>> >>>> PS I have a couple of questions related to the affected code: >>>> 1) Does GCC support building with compilers where int is not 32 >>>> bits wide, or where BITS_PER_UNIT is not 3? (I.e., either is >>>> less or more?) >>> We've certainly supported 16 bit ints in the past. H8/300 would be an >>> example. It defaults to 16 bit ints. But I don't think we've tested >>> that in a very long time -- my tester is only testing with -mint32. >>> >>> Look for INT_TYPE_SIZE in config/*/*.h >>> >>> We've never supported bootstrapping in that mode, just crosses. >> >> Thanks, that's what I was after: whether GCC can build natively >> with such a compiler where sizeof (int) != 32. Sounds like it >> can't, i.e., HOST_BITS_PER_INT is always at least 32. Or do >> you suppose it's always exactly 32? > At least 32 for HOST_BITS_PER_INT, I think we've supported 64 > HOST_BITS_PER_INT as well. > > HOST_BITS_PER_INT is defined indirectly via CHAR_BIT * SIZEOF_INT, both > of which are determined during the configure phase. > > >> >>> >>> AVR is probably the most interesting as it even has an flag to make >>> "int" be 8 bits. It's probably the best tested target in this space. >>> >>> BITS_PER_UNIT is more of a hardware characteristic. It's generally 8. >>> THough I thought one of the TI chips defined it to 32. I suspect you >>> weren't really looking for BITS_PER_UNIT here. >> >> I think using BITS_PER_UNIT here is actually another bug in >> the function: it should be using CHAR_BITS instead, like so: >> >> if (log2align >= HOST_BITS_PER_INT - exact_log2 (CHAR_BITS)) >> { >> error ("requested alignment %qE exceeds maximum %u", >> align, 1U << (HOST_BITS_PER_INT - exact_log2 (CHAR_BITS) - 1)); >> return -1; >> } > Isn't CHAR_BIT a function of the host, not the target? In which case I > don't think it's right since supported alignments are primarily a target > property. Yes, both CHAR_BIT and HOST_BITS_PER_INT are host constants, and so is the maximum alignment GCC can represent -- it stores its log2 in a 6-bit field. From tree-core.h: /* TYPE_ALIGN in log2; this has to be large enough to hold values of the maximum of BIGGEST_ALIGNMENT and MAX_OFILE_ALIGNMENT, the latter being usually the larger. For ELF it is 8<<28, so we need to store the value 32 (not 31, as we need the zero as well), hence six bits. */ unsigned align : 6; But I think BITS_PER_UNIT is used here because the alignment GCC stores is the bit alignment on the target. Even so, six bits can fit an alignment of up to 2 ^ 63 and there should be no reason to cap it at 2 ^ 31 just because of the ELF limit. Stack variables could (in theory at least) be aligned more strictly. But there are many assumptions throughout GCC code(*) that the actual byte alignment value fits into an int that I'm not sure that taking advantage of that sixth bit would justify the effort involved in changing this. [*] E.g., the definition of TYPE_ALIGN: #define TYPE_ALIGN(NODE) \ (TYPE_CHECK (NODE)->type_common.align \ ? ((unsigned)1) << ((NODE)->type_common.align - 1) : 0) >> Thanks. I was concerned about the test I added breaking on >> systems that don't define __INT64_TYPE__, but I see other tests >> that assume that __INT64_TYPE__ exists, so if it does break, it >> won't be the only one. > I wouldn't stress too much about it. The AVR guys might come back with > a patch to adjust the test. But again, I wouldn't worry about it until > they do. Ack. Thanks Martin
On Mon, Mar 25, 2019 at 11:41:35AM -0600, Jeff Law wrote: > > PR c/89812 - incorrect maximum in error: requested alignment ‘536870912’ exceeds maximum 2147483648 > > > > gcc/c-family/ChangeLog: > > > > PR c/89812 > > * c-common.c (check_user_alignment): Rename local. Correct maximum > > alignment in diagnostic. Avoid assuming argument fits in SHWI, > > convert it to UHWI when it fits. > > > > gcc/testsuite/ChangeLog: > > > > PR c/89812 > > * gcc.dg/attr-aligned-3.c: New test. > OK The test FAILs on all 32-bit targets (where __UINT64_TYPE__ is unsigned long long) due to -pedantic-errors, and I bet will fail on all non-ELF targets on AVR, because only config/elfos.h defines 1 << 28 as MAX_OFILE_ALIGNMENT (in bytes) and the test relies on exactly that value, nothing else. Fixed thusly, tested on x86_64-linux (-m32/-m64), ok for trunk? If we have some elf targets that still don't use elfos.h, we might need to add them next to avr too. 2019-03-26 Jakub Jelinek <jakub@redhat.com> PR c/89812 * gcc.dg/attr-aligned-3.c: Limit the test to known ELF targets other than AVR. Add dg-options "". --- gcc/testsuite/gcc.dg/attr-aligned-3.c.jj 2019-03-26 08:52:54.510611778 +0100 +++ gcc/testsuite/gcc.dg/attr-aligned-3.c 2019-03-26 16:25:44.518085453 +0100 @@ -1,7 +1,10 @@ /* PR c/89812 - incorrect maximum in error: requested alignment '536870912' exceeds maximum 2147483648 - { dg-do compile } - { dg-require-effective-target size32plus } */ + Limit to ELF targets that are known to use MAX_OFILE_ALIGNMENT + (1 << 28) * BITS_PER_UNIT. + { dg-do compile { target { { *-*-elf* *-*-gnu* } && { ! avr*-*-* } } } } + { dg-require-effective-target size32plus } + { dg-options "" } */ #define POWALIGN(N) __attribute__ ((aligned ((__UINT64_TYPE__)1 << (N)))) Jakub
Hi Jakub, > On Mon, Mar 25, 2019 at 11:41:35AM -0600, Jeff Law wrote: >> > PR c/89812 - incorrect maximum in error: requested alignment >> > ‘536870912’ exceeds maximum 2147483648 >> > >> > gcc/c-family/ChangeLog: >> > >> > PR c/89812 >> > * c-common.c (check_user_alignment): Rename local. Correct maximum >> > alignment in diagnostic. Avoid assuming argument fits in SHWI, >> > convert it to UHWI when it fits. >> > >> > gcc/testsuite/ChangeLog: >> > >> > PR c/89812 >> > * gcc.dg/attr-aligned-3.c: New test. >> OK > > The test FAILs on all 32-bit targets (where __UINT64_TYPE__ is unsigned long > long) due to -pedantic-errors, and I bet will fail on all non-ELF targets on > AVR, because only config/elfos.h defines 1 << 28 as MAX_OFILE_ALIGNMENT (in > bytes) and the test relies on exactly that value, nothing else. > > Fixed thusly, tested on x86_64-linux (-m32/-m64), ok for trunk? > > If we have some elf targets that still don't use elfos.h, we might need to > add them next to avr too. FWIW, adding *-*-solaris2.* to the target list lets the test also PASS on i386-pc-solaris2.11 and sparc-sun-solaris2.11 (32 and 64-bit each). Rainer
On 3/26/19 9:29 AM, Jakub Jelinek wrote: > On Mon, Mar 25, 2019 at 11:41:35AM -0600, Jeff Law wrote: >>> PR c/89812 - incorrect maximum in error: requested alignment ‘536870912’ exceeds maximum 2147483648 >>> >>> gcc/c-family/ChangeLog: >>> >>> PR c/89812 >>> * c-common.c (check_user_alignment): Rename local. Correct maximum >>> alignment in diagnostic. Avoid assuming argument fits in SHWI, >>> convert it to UHWI when it fits. >>> >>> gcc/testsuite/ChangeLog: >>> >>> PR c/89812 >>> * gcc.dg/attr-aligned-3.c: New test. >> OK > > The test FAILs on all 32-bit targets (where __UINT64_TYPE__ is unsigned long > long) due to -pedantic-errors, and I bet will fail on all non-ELF targets on > AVR, because only config/elfos.h defines 1 << 28 as MAX_OFILE_ALIGNMENT (in > bytes) and the test relies on exactly that value, nothing else. > > Fixed thusly, tested on x86_64-linux (-m32/-m64), ok for trunk? > > If we have some elf targets that still don't use elfos.h, we might need to > add them next to avr too. > > 2019-03-26 Jakub Jelinek <jakub@redhat.com> > > PR c/89812 > * gcc.dg/attr-aligned-3.c: Limit the test to known ELF targets > other than AVR. Add dg-options "". OK. Jeff
On 3/26/19 4:49 PM, Rainer Orth wrote: > Hi Jakub, > >> On Mon, Mar 25, 2019 at 11:41:35AM -0600, Jeff Law wrote: >>>> PR c/89812 - incorrect maximum in error: requested alignment >>>> ‘536870912’ exceeds maximum 2147483648 >>>> >>>> gcc/c-family/ChangeLog: >>>> >>>> PR c/89812 >>>> * c-common.c (check_user_alignment): Rename local. Correct maximum >>>> alignment in diagnostic. Avoid assuming argument fits in SHWI, >>>> convert it to UHWI when it fits. >>>> >>>> gcc/testsuite/ChangeLog: >>>> >>>> PR c/89812 >>>> * gcc.dg/attr-aligned-3.c: New test. >>> OK >> >> The test FAILs on all 32-bit targets (where __UINT64_TYPE__ is unsigned long >> long) due to -pedantic-errors, and I bet will fail on all non-ELF targets on >> AVR, because only config/elfos.h defines 1 << 28 as MAX_OFILE_ALIGNMENT (in >> bytes) and the test relies on exactly that value, nothing else. >> >> Fixed thusly, tested on x86_64-linux (-m32/-m64), ok for trunk? >> >> If we have some elf targets that still don't use elfos.h, we might need to >> add them next to avr too. > > FWIW, adding *-*-solaris2.* to the target list lets the test also PASS > on i386-pc-solaris2.11 and sparc-sun-solaris2.11 (32 and 64-bit each). Go for it. And ISTM that this kind of change should be well within the space where you should be able to commit w/o approvals :-) jeff
Hi Jeff, > On 3/26/19 4:49 PM, Rainer Orth wrote: >> Hi Jakub, >> >>> On Mon, Mar 25, 2019 at 11:41:35AM -0600, Jeff Law wrote: >>>>> PR c/89812 - incorrect maximum in error: requested alignment >>>>> ‘536870912’ exceeds maximum 2147483648 >>>>> >>>>> gcc/c-family/ChangeLog: >>>>> >>>>> PR c/89812 >>>>> * c-common.c (check_user_alignment): Rename local. Correct maximum >>>>> alignment in diagnostic. Avoid assuming argument fits in SHWI, >>>>> convert it to UHWI when it fits. >>>>> >>>>> gcc/testsuite/ChangeLog: >>>>> >>>>> PR c/89812 >>>>> * gcc.dg/attr-aligned-3.c: New test. >>>> OK >>> >>> The test FAILs on all 32-bit targets (where __UINT64_TYPE__ is unsigned long >>> long) due to -pedantic-errors, and I bet will fail on all non-ELF targets on >>> AVR, because only config/elfos.h defines 1 << 28 as MAX_OFILE_ALIGNMENT (in >>> bytes) and the test relies on exactly that value, nothing else. >>> >>> Fixed thusly, tested on x86_64-linux (-m32/-m64), ok for trunk? >>> >>> If we have some elf targets that still don't use elfos.h, we might need to >>> add them next to avr too. >> >> FWIW, adding *-*-solaris2.* to the target list lets the test also PASS >> on i386-pc-solaris2.11 and sparc-sun-solaris2.11 (32 and 64-bit each). > Go for it. And ISTM that this kind of change should be well within the > space where you should be able to commit w/o approvals :-) I know and meant to install the patch unless Jakub incorporated it into his. However, I preferred to leave approval of his patch to a subject-matter expert which hadn't happened by the time I sent my comment :-) Here's what I've installed now. Rainer
PR c/89812 - incorrect maximum in error: requested alignment ‘536870912’ exceeds maximum 2147483648 gcc/c-family/ChangeLog: PR c/89812 * c-common.c (check_user_alignment): Rename local. Correct maximum alignment in diagnostic. Avoid assuming argument fits in SHWI, convert it to UHWI when it fits. gcc/testsuite/ChangeLog: PR c/89812 * gcc.dg/attr-aligned-3.c: New test. Index: gcc/c-family/c-common.c =================================================================== --- gcc/c-family/c-common.c (revision 269902) +++ gcc/c-family/c-common.c (working copy) @@ -5287,9 +5287,10 @@ check_user_alignment (const_tree align, bool objfi return -1; } - int log2bitalign; + /* Log2 of the byte alignment ALIGN. */ + int log2align; if (tree_int_cst_sgn (align) == -1 - || (log2bitalign = tree_log2 (align)) == -1) + || (log2align = tree_log2 (align)) == -1) { error ("requested alignment %qE is not a positive power of 2", align); @@ -5299,7 +5300,7 @@ check_user_alignment (const_tree align, bool objfi if (objfile) { unsigned maxalign = MAX_OFILE_ALIGNMENT / BITS_PER_UNIT; - if (tree_to_shwi (align) > maxalign) + if (!tree_fits_uhwi_p (align) || tree_to_uhwi (align) > maxalign) { error ("requested alignment %qE exceeds object file maximum %u", align, maxalign); @@ -5307,14 +5308,14 @@ check_user_alignment (const_tree align, bool objfi } } - if (log2bitalign >= HOST_BITS_PER_INT - LOG2_BITS_PER_UNIT) + if (log2align >= HOST_BITS_PER_INT - LOG2_BITS_PER_UNIT) { error ("requested alignment %qE exceeds maximum %u", - align, 1U << (HOST_BITS_PER_INT - 1)); + align, 1U << (HOST_BITS_PER_INT - LOG2_BITS_PER_UNIT - 1)); return -1; } - return log2bitalign; + return log2align; } /* Determine the ELF symbol visibility for DECL, which is either a Index: gcc/testsuite/gcc.dg/attr-aligned-3.c =================================================================== --- gcc/testsuite/gcc.dg/attr-aligned-3.c (nonexistent) +++ gcc/testsuite/gcc.dg/attr-aligned-3.c (working copy) @@ -0,0 +1,28 @@ +/* PR c/89812 - incorrect maximum in error: requested alignment '536870912' + exceeds maximum 2147483648 + { dg-do compile } + { dg-require-effective-target size32plus } */ + +#define POWALIGN(N) __attribute__ ((aligned ((__UINT64_TYPE__)1 << (N)))) + +typedef POWALIGN (28) char T28; + +/* The maximum alignment is constrained by the number of bits in int + on host minus 3: HOST_BITS_PER_INT - LOG2_BITS_PER_UNIT. The test + assumes host int is 32-bits wide. */ +typedef POWALIGN (29) char X29; /* { dg-error "requested alignment .536870912. exceeds maximum 268435456" } */ +typedef POWALIGN (30) char X30; /* { dg-error "requested alignment .1073741824. exceeds maximum 268435456" } */ +typedef POWALIGN (31) char X31; /* { dg-error "requested alignment .2147483648. exceeds maximum 268435456" } */ +typedef POWALIGN (32) char X32; /* { dg-error "requested alignment .4294967296. exceeds maximum 268435456" } */ +typedef POWALIGN (60) char X60; /* { dg-error "requested alignment .1152921504606846976. exceeds maximum 268435456" } */ +typedef POWALIGN (63) char X63; /* { dg-error "requested alignment .9223372036854775808. exceeds maximum 268435456" } */ + + +POWALIGN (28) char c28; + +POWALIGN (29) char c29; /* { dg-error "requested alignment .536870912. exceeds object file maximum 268435456" } */ +POWALIGN (30) char x30; /* { dg-error "requested alignment .1073741824. exceeds object file maximum 268435456" } */ +POWALIGN (31) char x31; /* { dg-error "requested alignment .2147483648. exceeds object file maximum 268435456" } */ +POWALIGN (32) char x32; /* { dg-error "requested alignment .4294967296. exceeds object file maximum 268435456" } */ +POWALIGN (60) char x60; /* { dg-error "requested alignment .1152921504606846976. exceeds object file maximum 268435456" } */ +POWALIGN (63) char x63; /* { dg-error "requested alignment .9223372036854775808. exceeds object file maximum 268435456" } */
The error issued when the aligned attribute argument is too big to be represented is incorrect: it says the maximum alignment is 1U << 31 when it should actually be 1 << 28. This was a typo introduced when the error message was enhanced earlier in GCC 9. The test I added to verify the fix for the typo exposed another bug introduced in the same commit as the incorrect value in the error message: assuming that the attribute aligned argument fits in SHWI. The attached patch corrects both problems. It has been tested on x86_64-linux. I will commit it as obvious sometime this week unless there are any objections or suggestions for changes. Martin PS I have a couple of questions related to the affected code: 1) Does GCC support building with compilers where int is not 32 bits wide, or where BITS_PER_UNIT is not 3? (I.e., either is less or more?) 2) Is there a supported target that doesn't have __INT64_TYPE__? (And if so, how do I find it in a test? I couldn't find anythhing in target-supports.exp).