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

Martin Sebor msebor@gmail.com
Fri Feb 3 00:50:00 GMT 2017


On 02/02/2017 04:23 PM, Jakub Jelinek wrote:
> On Thu, Feb 02, 2017 at 12:59:11PM -0700, Martin Sebor wrote:
>>>> -  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.
>
> Then please first define what should likely mean and document that.

That's a fair request.   I implemented the "likely counter" approach
based on your suggestion (in comment #3 on bug 78703) so I'd expect
the concept to feel intuitive to you, but documenting it is a good
idea (and in fact on my list of things to do).

>
> That is unrelated to the patch, both in the current trunk, with your
> patch as well as with my patch there is just
>   res.range.likely = res.knownrange ? res.range.max : res.range.min;
>   res.range.unlikely = res.range.max;
> for these cases.
>
> Do you want likely 2 because that the shortest length for more than
> one value (only a single value has the shortest length)?
> Something else?

For "%#o" the shortest output of one byte (for zero) is less likely
than the next shortest output of 2 bytes (0 vs 01).

For "%#x" it's one byte vs three bytes (0 vs 0x1).

Since specifying '#' clearly indicates the user wants the base
prefix it seems very likely that the argument will be non-zero.
Whether it's going to be greater than 7 or 15 is not so clear.

So I think setting the likely counter to 2 for octal and 3 for
hexadecimal makes the most sense.

That's what I did in the patch I just posted:
   https://gcc.gnu.org/ml/gcc-patches/2017-02/msg00223.html

Martin



More information about the Gcc-patches mailing list