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

[Richard Earnshaw (lists)]

On 23/01/17 18:00, Andreas Tobler wrote:
> On 23.01.17 18:48, Jakub Jelinek wrote:
>> On Mon, Jan 23, 2017 at 06:42:15PM +0100, Andreas Tobler wrote:
>>> Something like below?
>>>
>>> If ok, I can commit, right?
>>>
>>> Thanks,
>>>
>>> Andreas
>>>
>>> 2017-01-23  Andreas Tobler  
>>>
>>> * config/aarch64/aarch64.c (aarch64_elf_asm_constructor): Increase
>>> size of buf.
>>> (aarch64_elf_asm_destructor): Likewise.
>>> --- config/aarch64/aarch64.c(revision 244819)
>>> +++ config/aarch64/aarch64.c(working copy)
>>> @@ -5787,7 +5787,11 @@
>>>else
>>>  {
>>>section *s;
>>> -  char buf[18];
>>> +  /* The size of the buf must be big enough to hold the string
>>> and the
>>> + full integer size of priority. Otherwise we will get a warning
>>> + about format-truncation.
>>> +  */
>>
>> Please put the */ on the same line as about format-truncation, like:
>>  about format-truncation.  */
>> After . there should be 2 spaces rather than one.
>> Also, the comment doesn't tell the truth, the buffer doesn't have to
>> be that
>> big, because we know priority is bounded, just the compiler doesn't know
>> that.  So perhaps:
>>   /* While priority is known to be in range [0, 65535], so 18 bytes
>>  would be enough, the compiler might not know that.  To avoid
>>  -Wformat-truncation false positive, use a larger size.  */
>>   char buf[23];
>> or so?
> 
> 
> Danke.
> 
> Andreas
> 
> 2017-01-23  Andreas Tobler  
> 
> * config/aarch64/aarch64.c (aarch64_elf_asm_constructor): Increase
> size of buf.
> (aarch64_elf_asm_destructor): Likewise.
> 

OK.

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

[Andreas Tobler]

On 23.01.17 18:48, Jakub Jelinek wrote:

On Mon, Jan 23, 2017 at 06:42:15PM +0100, Andreas Tobler wrote:

Something like below?

If ok, I can commit, right?

Thanks,

Andreas

2017-01-23  Andreas Tobler  

* config/aarch64/aarch64.c (aarch64_elf_asm_constructor): Increase
size of buf.
(aarch64_elf_asm_destructor): Likewise.
--- config/aarch64/aarch64.c(revision 244819)
+++ config/aarch64/aarch64.c(working copy)
@@ -5787,7 +5787,11 @@
   else
 {
   section *s;
-  char buf[18];
+  /* The size of the buf must be big enough to hold the string and the
+ full integer size of priority. Otherwise we will get a warning
+ about format-truncation.
+  */


Please put the */ on the same line as about format-truncation, like:
 about format-truncation.  */
After . there should be 2 spaces rather than one.
Also, the comment doesn't tell the truth, the buffer doesn't have to be that
big, because we know priority is bounded, just the compiler doesn't know
that.  So perhaps:
  /* While priority is known to be in range [0, 65535], so 18 bytes
 would be enough, the compiler might not know that.  To avoid
 -Wformat-truncation false positive, use a larger size.  */
  char buf[23];
or so?



Danke.

Andreas

2017-01-23  Andreas Tobler  

* config/aarch64/aarch64.c (aarch64_elf_asm_constructor): Increase
size of buf.
(aarch64_elf_asm_destructor): Likewise.

Index: config/aarch64/aarch64.c
===
--- config/aarch64/aarch64.c(revision 244819)
+++ config/aarch64/aarch64.c(working copy)
@@ -5787,7 +5787,10 @@
   else
 {
   section *s;
-  char buf[18];
+  /* While priority is known to be in range [0, 65535], so 18 bytes
+ would be enough, the compiler might not know that.  To avoid
+ -Wformat-truncation false positive, use a larger size.  */
+  char buf[23];
   snprintf (buf, sizeof (buf), ".init_array.%.5u", priority);
   s = get_section (buf, SECTION_WRITE, NULL);
   switch_to_section (s);
@@ -5804,7 +5807,10 @@
   else
 {
   section *s;
-  char buf[18];
+  /* While priority is known to be in range [0, 65535], so 18 bytes
+ would be enough, the compiler might not know that.  To avoid
+ -Wformat-truncation false positive, use a larger size.  */
+  char buf[23];
   snprintf (buf, sizeof (buf), ".fini_array.%.5u", priority);
   s = get_section (buf, SECTION_WRITE, NULL);
   switch_to_section (s);

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

[Jakub Jelinek]

On Mon, Jan 23, 2017 at 06:42:15PM +0100, Andreas Tobler wrote:
> Something like below?
> 
> If ok, I can commit, right?
> 
> Thanks,
> 
> Andreas
> 
> 2017-01-23  Andreas Tobler  
> 
>   * config/aarch64/aarch64.c (aarch64_elf_asm_constructor): Increase
>   size of buf.
>   (aarch64_elf_asm_destructor): Likewise.
> --- config/aarch64/aarch64.c  (revision 244819)
> +++ config/aarch64/aarch64.c  (working copy)
> @@ -5787,7 +5787,11 @@
>else
>  {
>section *s;
> -  char buf[18];
> +  /* The size of the buf must be big enough to hold the string and the
> + full integer size of priority. Otherwise we will get a warning
> + about format-truncation.
> +  */

Please put the */ on the same line as about format-truncation, like:
 about format-truncation.  */
After . there should be 2 spaces rather than one.
Also, the comment doesn't tell the truth, the buffer doesn't have to be that
big, because we know priority is bounded, just the compiler doesn't know
that.  So perhaps:
  /* While priority is known to be in range [0, 65535], so 18 bytes
 would be enough, the compiler might not know that.  To avoid
 -Wformat-truncation false positive, use a larger size.  */
  char buf[23];
or so?

Jakub

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

[Andreas Tobler]

On 23.01.17 11:07, Richard Earnshaw (lists) wrote:

On 21/01/17 20:15, Andreas Tobler wrote:

On 21.01.17 00:42, Jakub Jelinek wrote:

On Fri, Jan 20, 2017 at 01:37:13PM -0700, Jeff Law wrote:

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)

I'd go with #2.  I think #3 still requires range propagation to avoid
the
false positive.


I actually think 1.) is best work-around, after 18 bytes there will be
some
padding so 23 bytes will almost always eat the same amount of stack
space,
and we avoid actually adding some instruction to cast it or hoping all
the
libcs we run on support %.5hu properly.


Richard, what do you think about this suggestion?

Thanks,
Andreas




I'm happy to go with that, but please add a comment as well, or some
well meaning individual may accidentally reverse the change after
counting the bytes...


Something like below?

If ok, I can commit, right?

Thanks,

Andreas

2017-01-23  Andreas Tobler  

* config/aarch64/aarch64.c (aarch64_elf_asm_constructor): Increase
size of buf.
(aarch64_elf_asm_destructor): Likewise.


Index: config/aarch64/aarch64.c
===
--- config/aarch64/aarch64.c(revision 244819)
+++ config/aarch64/aarch64.c(working copy)
@@ -5787,7 +5787,11 @@
   else
 {
   section *s;
-  char buf[18];
+  /* The size of the buf must be big enough to hold the string and the
+ full integer size of priority. Otherwise we will get a warning
+ about format-truncation.
+  */
+  char buf[23];
   snprintf (buf, sizeof (buf), ".init_array.%.5u", priority);
   s = get_section (buf, SECTION_WRITE, NULL);
   switch_to_section (s);
@@ -5804,7 +5808,11 @@
   else
 {
   section *s;
-  char buf[18];
+  /* The size of the buf must be big enough to hold the string and the
+ full integer size of priority. Otherwise we will get a warning
+ about format-truncation.
+  */
+  char buf[23];
   snprintf (buf, sizeof (buf), ".fini_array.%.5u", priority);
   s = get_section (buf, SECTION_WRITE, NULL);
   switch_to_section (s);

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

[Richard Earnshaw (lists)]

On 21/01/17 20:15, Andreas Tobler wrote:
> On 21.01.17 00:42, Jakub Jelinek wrote:
>> On Fri, Jan 20, 2017 at 01:37:13PM -0700, Jeff Law wrote:
> 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)
>>> I'd go with #2.  I think #3 still requires range propagation to avoid
>>> the
>>> false positive.
>>
>> I actually think 1.) is best work-around, after 18 bytes there will be
>> some
>> padding so 23 bytes will almost always eat the same amount of stack
>> space,
>> and we avoid actually adding some instruction to cast it or hoping all
>> the
>> libcs we run on support %.5hu properly.
> 
> Richard, what do you think about this suggestion?
> 
> Thanks,
> Andreas
> 


I'm happy to go with that, but please add a comment as well, or some
well meaning individual may accidentally reverse the change after
counting the bytes...

R.

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

[Andreas Tobler]

On 21.01.17 00:42, Jakub Jelinek wrote:

On Fri, Jan 20, 2017 at 01:37:13PM -0700, Jeff Law wrote:

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)

I'd go with #2.  I think #3 still requires range propagation to avoid the
false positive.


I actually think 1.) is best work-around, after 18 bytes there will be some
padding so 23 bytes will almost always eat the same amount of stack space,
and we avoid actually adding some instruction to cast it or hoping all the
libcs we run on support %.5hu properly.


Richard, what do you think about this suggestion?

Thanks,
Andreas

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

[Jakub Jelinek]

On Fri, Jan 20, 2017 at 01:37:13PM -0700, Jeff Law wrote:
> > > 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)
> I'd go with #2.  I think #3 still requires range propagation to avoid the
> false positive.

I actually think 1.) is best work-around, after 18 bytes there will be some
padding so 23 bytes will almost always eat the same amount of stack space,
and we avoid actually adding some instruction to cast it or hoping all the
libcs we run on support %.5hu properly.

Jakub

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

[Richard Earnshaw (lists)]

On 20/01/17 19:56, Andreas Tobler wrote:
> 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
> 

Not a really valid use of destructors, but

void  __attribute__((destructor(65536))) f()
{
}

causes
/tmp/x.c:2:1: error: destructor priorities must be integers from 0 to
65535 inclusive
 {


So I think we can ignore saturation.  Now it's down to assert then cast
or simply cast.

My inclination is just the cast at this point.

R.

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

[Jeff Law]

On 01/20/2017 12:56 PM, Andreas Tobler wrote:

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)
I'd go with #2.  I think #3 still requires range propagation to avoid 
the false positive.


jeff

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

[Andreas Tobler]

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

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

[Richard Earnshaw (lists)]

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.

R.

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

[Andreas Tobler]

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

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

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

[Jeff Law]

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.

Jeff

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

[Andreas Tobler]

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.

The last option is the cleanest one, but it involves changing priority 
everywhere in the gcc/gcc sources. I started and saw how many places I 
have to change before I continue I would like to ask you guys, which 
one is the prefered solution?


Thanks for feedback.
Andreas