This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] correct maximum valid alignment in error message (PR 89812)
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