| Message ID | 24a09062-adeb-a7cf-5f59-f0015505d481@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | have -Warray-bounds treat [max, min] ranges same as anti-ranges (PR 89720) | expand |
On 3/18/19 1:03 PM, Martin Sebor wrote: > I the -Warray-bounds enhancement committed at the beginning > of the GCC 9 window I tried to correctly handle offsets in > MEM_REFs in the [max, min] kind of a range after converting > from sizetype to ptrdiff_type, but I did it wrong. The bug > results in false positives in some unusual use cases that > I didn't consider at the time. > > The attached patch removes this incorrect handling and uses > the conservative anti-range handling for these cases instead. > > Martin > > PS Is there some technical reason why pointer offsets are > represented as the unsigned sizetype when they can be signed? I'm not aware of a conscious decision to treat them as unsigned or a particular target need to do so. It's most likely a historical accident. > > gcc-89720.diff > > PR tree-optimization/89720 - Spurious -Warray-bounds warning on a range with max < min > > gcc/ChangeLog: > > PR tree-optimization/89720 > * tree-vrp.c (vrp_prop::check_mem_ref): Treat range with max < min > more conservatively, the same as anti-range. > > gcc/testsuite/ChangeLog: > > PR tree-optimization/89720 > * gcc.dg/Warray-bounds-42.c: New test. OK jeff
On Mon, Mar 18, 2019 at 10:31 PM Jeff Law <law@redhat.com> wrote: > > On 3/18/19 1:03 PM, Martin Sebor wrote: > > I the -Warray-bounds enhancement committed at the beginning > > of the GCC 9 window I tried to correctly handle offsets in > > MEM_REFs in the [max, min] kind of a range after converting > > from sizetype to ptrdiff_type, but I did it wrong. The bug > > results in false positives in some unusual use cases that > > I didn't consider at the time. > > > > The attached patch removes this incorrect handling and uses > > the conservative anti-range handling for these cases instead. > > > > Martin > > > > PS Is there some technical reason why pointer offsets are > > represented as the unsigned sizetype when they can be signed? > I'm not aware of a conscious decision to treat them as unsigned or a > particular target need to do so. It's most likely a historical accident. That historical accident included treating those unsigned types as sign-extending ... But yes, changing sizetype to ssizetype wherever we use it in offset context would be a cleanup I guess. But IIRC Bin tried this and the fallout is (as usual) non-trivial to fix... Richard. > > > > > gcc-89720.diff > > > > PR tree-optimization/89720 - Spurious -Warray-bounds warning on a range with max < min > > > > gcc/ChangeLog: > > > > PR tree-optimization/89720 > > * tree-vrp.c (vrp_prop::check_mem_ref): Treat range with max < min > > more conservatively, the same as anti-range. > > > > gcc/testsuite/ChangeLog: > > > > PR tree-optimization/89720 > > * gcc.dg/Warray-bounds-42.c: New test. > OK > jeff
PR tree-optimization/89720 - Spurious -Warray-bounds warning on a range with max < min gcc/ChangeLog: PR tree-optimization/89720 * tree-vrp.c (vrp_prop::check_mem_ref): Treat range with max < min more conservatively, the same as anti-range. gcc/testsuite/ChangeLog: PR tree-optimization/89720 * gcc.dg/Warray-bounds-42.c: New test. Index: gcc/tree-vrp.c =================================================================== --- gcc/tree-vrp.c (revision 269767) +++ gcc/tree-vrp.c (working copy) @@ -4546,9 +4546,9 @@ vrp_prop::check_mem_ref (location_t location, tree const value_range *vr = NULL; /* Determine the offsets and increment OFFRANGE for the bounds of each. - The loop computes the the range of the final offset for expressions - such as (A + i0 + ... + iN)[CSTOFF] where i0 through iN are SSA_NAMEs - in some range. */ + The loop computes the range of the final offset for expressions such + as (A + i0 + ... + iN)[CSTOFF] where i0 through iN are SSA_NAMEs in + some range. */ while (TREE_CODE (arg) == SSA_NAME) { gimple *def = SSA_NAME_DEF_STMT (arg); @@ -4583,26 +4583,21 @@ vrp_prop::check_mem_ref (location_t location, tree if (vr->kind () == VR_RANGE) { - if (tree_int_cst_lt (vr->min (), vr->max ())) + offset_int min + = wi::to_offset (fold_convert (ptrdiff_type_node, vr->min ())); + offset_int max + = wi::to_offset (fold_convert (ptrdiff_type_node, vr->max ())); + if (min < max) { - offset_int min - = wi::to_offset (fold_convert (ptrdiff_type_node, vr->min ())); - offset_int max - = wi::to_offset (fold_convert (ptrdiff_type_node, vr->max ())); - if (min < max) - { - offrange[0] += min; - offrange[1] += max; - } - else - { - offrange[0] += max; - offrange[1] += min; - } + offrange[0] += min; + offrange[1] += max; } else { - /* Conservatively add [-MAXOBJSIZE -1, MAXOBJSIZE] + /* When MIN >= MAX, the offset is effectively in a union + of two ranges: [-MAXOBJSIZE -1, MAX] and [MIN, MAXOBJSIZE]. + Since there is no way to represent such a range across + additions, conservatively add [-MAXOBJSIZE -1, MAXOBJSIZE] to OFFRANGE. */ offrange[0] += arrbounds[0]; offrange[1] += arrbounds[1]; Index: gcc/testsuite/gcc.dg/Warray-bounds-42.c =================================================================== --- gcc/testsuite/gcc.dg/Warray-bounds-42.c (nonexistent) +++ gcc/testsuite/gcc.dg/Warray-bounds-42.c (working copy) @@ -0,0 +1,26 @@ +/* PR tree-optimization/89720 - Spurious -Warray-bounds warning on a range + with max < min + { dg-do compile } + { dg-options "-O2 -Wall" } */ + +void f (char*, int); + +#if __SIZEOF_POINTER__ == 8 + +void g (__INT64_TYPE__ i) +{ + char a[65536] = ""; + char *p = a + (i & (__INT64_TYPE__)0xffffffff3fffffffLL); + f (p, *(p - 6)); /* { dg-bogus "\\\[-Warray-bounds" } */ +} + +#elif __SIZEOF_POINTER__ == 4 + +void h (__INT32_TYPE__ i) +{ + char a[65536] = ""; + char *p = a + (i & (__INT32_TYPE__)0x8fffffffLL); + f (p, *(p - 6)); /* { dg-bogus "\\\[-Warray-bounds" } */ +} + +#endif