diff mbox

[bootstrap-O1] enlarge sprintf output buffer to avoid warning

Message ID orwpec3e5y.fsf@lxoliva.fsfla.org
State New
Headers show

Commit Message

Alexandre Oliva Jan. 3, 2017, 5:28 a.m. UTC
In stage2 of bootstrap-O1, the code that warns if sprintf might
overflow its output buffer cannot tell that an unsigned value narrowed
to 16 bits will fit in 4 bytes with %4x.

I couldn't find a better way to avoid the warning at -O1 than growing
the buffer so that there's no doubt the output will fit.

Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  Ok to install?

for  gcc/c-family/ChangeLog

	* c-pretty-print.c (pp_c_tree_decl_identifier): Grow static
	buffer to avoid false-positive warning.
---
 gcc/c-family/c-pretty-print.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Jeff Law Jan. 3, 2017, 5:39 p.m. UTC | #1
On 01/02/2017 10:28 PM, Alexandre Oliva wrote:
> In stage2 of bootstrap-O1, the code that warns if sprintf might
> overflow its output buffer cannot tell that an unsigned value narrowed
> to 16 bits will fit in 4 bytes with %4x.
>
> I couldn't find a better way to avoid the warning at -O1 than growing
> the buffer so that there's no doubt the output will fit.
>
> Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  Ok to install?
>
> for  gcc/c-family/ChangeLog
>
> 	* c-pretty-print.c (pp_c_tree_decl_identifier): Grow static
> 	buffer to avoid false-positive warning.
Presumably this is an artifact of not running VRP at -O1 and thus we 
don't have a narrowed range from the masking operation.

This isn't performance critical code so we *could* avoid the statically 
sized array.  But I doubt it's worth the effort.

OK for the trunk.

jeff
Martin Sebor Jan. 4, 2017, 5:59 p.m. UTC | #2
On 01/03/2017 10:39 AM, Jeff Law wrote:
> On 01/02/2017 10:28 PM, Alexandre Oliva wrote:
>> In stage2 of bootstrap-O1, the code that warns if sprintf might
>> overflow its output buffer cannot tell that an unsigned value narrowed
>> to 16 bits will fit in 4 bytes with %4x.
>>
>> I couldn't find a better way to avoid the warning at -O1 than growing
>> the buffer so that there's no doubt the output will fit.
>>
>> Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  Ok to install?
>>
>> for  gcc/c-family/ChangeLog
>>
>>     * c-pretty-print.c (pp_c_tree_decl_identifier): Grow static
>>     buffer to avoid false-positive warning.
> Presumably this is an artifact of not running VRP at -O1 and thus we
> don't have a narrowed range from the masking operation.
>
> This isn't performance critical code so we *could* avoid the statically
> sized array.  But I doubt it's worth the effort.

The manual recommends to use a length modifier to constrain the length
of output to that of a narrower type:

   sprintf (xname, "<U%4hx>", ((unsigned short)((uintptr_t)(t) & 0xffff)));

This should work even without optimization.

Martin
diff mbox

Patch

diff --git a/gcc/c-family/c-pretty-print.c b/gcc/c-family/c-pretty-print.c
index c32d0a0..dc6856a 100644
--- a/gcc/c-family/c-pretty-print.c
+++ b/gcc/c-family/c-pretty-print.c
@@ -2375,7 +2375,8 @@  pp_c_tree_decl_identifier (c_pretty_printer *pp, tree t)
     name = IDENTIFIER_POINTER (DECL_NAME (t));
   else
     {
-      static char xname[8];
+      /* xname[8] would do, but at -O1 we can't tell it's enough.  */
+      static char xname[12];
       sprintf (xname, "<U%4x>", ((unsigned)((uintptr_t)(t) & 0xffff)));
       name = xname;
     }