[PATCH] use zero as the lower bound for a signed-unsigned range (PR 79327)

Martin Sebor msebor@gmail.com
Thu Feb 2 19:59:00 GMT 2017


>> So far I haven't done full bootstrap/regtest on this, just
>> make check-gcc -j16 -k RUNTESTFLAGS=tree-ssa.exp
>> which revealed the need for gcc.dg/tree-ssa/builtin-sprintf-warn-1.c hunk
>> below.  Here it is turned into a short wrong-code without the patch
>> below:
>> volatile int a;
>>
>> int
>> main (void)
>> {
>>   int i;
>>   char buf[64];
>>   if (__builtin_sprintf (buf, "%#hho", a) > 1)
>>     __builtin_abort ();
>>   if (__builtin_sprintf (buf, "%#hhx", a) > 1)
>>     __builtin_abort ();
>>   return 0;
>> }
>> Current trunk as well as trunk + your patch computes ranges [2, 4] and
>> [3, 4], but the correct ranges are [1, 4] and [1, 4], because for a 0
>> (or say -131072 etc.) it sets buf to "0" in both cases.
>
> That's right.  This is a good find.  This case is being tested in
> builtin-sprintf-warn-1.c but the assertions (on lines 1154 and
> 1155) seem to expect the wrong numbers.  I would expect your fix
> to cause failures here. (And now that I've read the rest of the
> patch I see it does.)
...
>> -  T (2, "%#hho",        a);     /* { dg-warning "nul past the end" } */
>> -  T (2, "%#hhx",        a);     /* { dg-warning ".%#hhx. directive
>> writing between 3 and . bytes into a region of size 2" } */
>> +  T (2, "%#hho",        a);
>> +  T (2, "%#hhx",        a);

On reflection, this isn't quite the right fix.  We want to both set
the correct range and warn because the call will likely overflow.
This is an example of why the likely/unlikely counters have been
introduced.  By setting min = 1 and likely = 2 for the %#hho and
3 for the %#hhx we get the desired result.

Martin



More information about the Gcc-patches mailing list