Re: [RFC] fix bootstrap on aarch64-*-freebsd and probably others

[Andreas Tobler <andreast-list at fgznet dot ch>]

On 20.01.17 17:12, Richard Earnshaw (lists) wrote:
> On 19/01/17 06:38, Andreas Tobler wrote:
> > On 19.01.17 00:33, Jeff Law wrote:
> > > On 01/18/2017 11:43 AM, Andreas Tobler wrote:
> > > > Hi all,
> > > >
> > > > I have the following issue here on aarch64-*-freebsd:
> > > >
> > > > (sorry if the format is hardly readable)
> > > >
> > > > ......
> > > > /export/devel/net/src/gcc/head/gcc/gcc/config/aarch64/aarch64.c: In
> > > > function 'void aarch64_elf_asm_destructor(rtx, int)':
> > > > /export/devel/net/src/gcc/head/gcc/gcc/config/aarch64/aarch64.c:5760:1:
> > > > error: %.5u' directive output may be truncated writing between 5 and 10
> > > > bytes into a region of size 6 [-Werror=format-truncation=]
> > > >  aarch64_elf_asm_destructor (rtx symbol, int priority)
> > > >  ^~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > /export/devel/net/src/gcc/head/gcc/gcc/config/aarch64/aarch64.c:5760:1:
> > > > note: using the range [1, 4294967295] for directive argument
> > > > /export/devel/net/src/gcc/head/gcc/gcc/config/aarch64/aarch64.c:5768:65:
> > > > note: format output between 18 and 23 bytes into a destination of
> > > > size 18
> > > >        snprintf (buf, sizeof (buf), ".fini_array.%.5u", priority);
> > > >
> > > >                                                                 ^
> > > > .......
> > > >
> > > > This is the code snippet, it does not only occur in aarch64, but also at
> > > > least in pa and avr.
> > > >
> > > > ----
> > > > static void
> > > > aarch64_elf_asm_destructor (rtx symbol, unsigned short priority)
> > > > {
> > > >   if (priority == DEFAULT_INIT_PRIORITY)
> > > >     default_dtor_section_asm_out_destructor (symbol, priority);
> > > >   else
> > > >     {
> > > >       section *s;
> > > >       char buf[18];
> > > >       snprintf (buf, sizeof (buf), ".fini_array.%.5u", priority);
> > > >       s = get_section (buf, SECTION_WRITE, NULL);
> > > >       switch_to_section (s);
> > > >       assemble_align (POINTER_SIZE);
> > > >       assemble_aligned_integer (POINTER_BYTES, symbol);
> > > >     }
> > > > }
> > > > ----
> > > >
> > > > I have now four options to solve this, (a fifth one would be to remove
> > > > format-truncation from -Wall/Wextra?)
> > > >
> > > > 1.) increase buf to 23
> > > > 2.) use %.5hu in snprintf
> > > > 3.) cast priority in snprintf to (unsigned int)
> > > > 4.) make priority unsigned short.
> > > >
> > > > Solution 1, 2 and 3 work, but with pros and cons.
> > > #3 likely won't work with with lower optimization levels since it
> > > depends on VRP to narrow the object's range.
> > >
> > > I'd approve #2 or #1 without hesitation.
> >
> > Ok.
> >
> > I did a mistake while describing the situation. The function has this
> > parameter:
> >
> > aarch64_elf_asm_destructor (rtx symbol, int priority)
> >
> > I copied the already modified piece of code....
>
> Ah, that makes much more sense.
>
> > So the cast in #3 would be (unsigned short) iso (unsigned int).
> >
> > If no other opinions come up I'll go with #2.
> >
> > Thanks.
> > Andreas
>
> I agree, some sort of cast seems preferable.  The documentation for
> constructor priorities says that the lowest priority is 65535, so
> alternatives here are:
>
> - assert (priority < 65536) then cast to unsigned short.
> - simply cast to unsigned short
> - saturate to 16 bits unsigned then cast to short.
>
> Which is best will depend on what the front/mid ends might have done to
> apply the documented limit.

Here I know not enough to give a decision. In tree the priority_type is 
unsigned short. In varasm priority is an int.

Similar functions, like arm_elf_asm_cdtor, do use the sprintf instead of 
snprintf. They theoretically have the same issue.

So, either:
2.) use %.5hu in snprintf
or
3.) cast priority in snprintf to (unsigned short)


Thanks,
Andreas