On 8/4/20 3:23 PM, Aldy Hernandez wrote:


On 8/4/20 9:34 PM, Martin Sebor wrote:
On 8/4/20 5:33 AM, Aldy Hernandez via Gcc-patches wrote:
This patch adapts the strlen pass to use the irange API.

I wasn't able to remove the one annoying use of VR_ANTI_RANGE, because
I'm not sure what to do.  Perhaps Martin can shed some light.  The
current code has:

   else if (rng == VR_ANTI_RANGE)
    {
      wide_int maxobjsize = wi::to_wide (TYPE_MAX_VALUE (ptrdiff_type_node));
      if (wi::ltu_p (cntrange[1], maxobjsize))
        {
          cntrange[0] = cntrange[1] + 1;
          cntrange[1] = maxobjsize;

Suppose we have ~[10,20], won't the above set cntrange[] to [21,MAX]? Won't
this ignore the 0..9 that is part of the range?  What should we do here?

cntrange is the range of the strncpy (and strncat) bound.  It does
ignore the lower subrange but I think that's intentional because
the lower the bound the more likely the truncation, so it serves
to minimize false positives.

I didn't see any tests fail with the anti-range block disabled but
with some effort I was able to come up with one:

   char a[7];

   void f (int n)
   {
     if (n > 3)
       n = 0;

     strncpy (a, "12345678", n);   // -Wstringop-truncation
   }

The warning disappears when the anti-range handling is removed so
unless that's causing headaches for the new API I think we want to
keep it (and add the test case :)

Hi Martin.

Thanks for taking the time to respond.

On the strlen1 dump I see that the 3rd argument to strncpy above is:

   long unsigned int ~[4, 18446744071562067967]

which is a fancy way of saying:

   long unsigned int [0,3][18446744071562067967,+INF]

The second sub-range is basically [INT_MIN,+INF] for the original int N, which makes sense because N could be negative on the way in.

I don't understand the warning though:

a.c:8:5: warning: ‘__builtin_strncpy’ output truncated copying between 0 and 3 bytes from a
  string of length 8 [-Wstringop-truncation]
    8 |     __builtin_strncpy (a, "12345678", n);   // -Wstringop-truncation
       |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The range of the bound to strncpy can certainly be [0,3], but it can also be [1844...,+INF] which shouldn't warn.

strncpy(d, s, n) zeroes out the destination after it copies s, up
to n - strlen (s).  It's been a while and -Wstringop-truncation
certainly has its quirks (to put it nicely) but I think this one
is a feature.


In a world without anti-ranges, we'd see the 2 sub-ranges above.  How would you suggest handling it?  We could nuke out the uppermost sub-range, but what if the range is [0,3][10,20]?  Perhaps remove from some arbitrary number on up?  Say...[0xf.....,+INF]?  This seems like a hack, but perhaps is what's needed???

The second subrange in [0, 3][10, 20] is out of bounds for char[7]
and the first one truncates so either we issue -Wstringop-truncation
or if not, -Wstringop-overflow.

A better example might be [0, 3][5, 7] with "1234" as the source.
Only one of these ranges truncates so a warning would probably not
be called for.  Only warn when there's no range that doesn't lead
to truncation.

It will get more interesting if/when the length of the source string
is also in a discontiguous range.  My head is already starting to hurt.

Martin


It doesn't seem like the above source should warn.  Am I missing something?

Thanks.
Aldy


Reply via email to