[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