PING: [PATCH] Convert -Wrestrict pass to ranger.

Rebased on current trunk.

There is one adjustment to a C++ test which now gives a false positive.
After talking with Martin Sebor, we've concluded this is expected.  There
is no way to communicate that libstdc++ allocated objects are always
less than PTRDIFF_MAX.

OK?

    gcc/ChangeLog:

            * calls.c (get_size_range): Adjust to work with ranger.
            * calls.h (get_size_range): Add ranger argument to prototype.
            * gimple-ssa-warn-restrict.c (class wrestrict_dom_walker): Remove.
            (check_call): Pull out of wrestrict_dom_walker into a
            static function.
            (wrestrict_dom_walker::before_dom_children): Rename to...
            (wrestrict_walk): ...this.
            (pass_wrestrict::execute): Instantiate ranger.
            (class builtin_memref): Add stmt and query fields.
            (builtin_access::builtin_access): Add range_query field.
            (builtin_memref::builtin_memref): Same.
            (builtin_memref::extend_offset_range): Same.
            (builtin_access::builtin_access): Make work with ranger.
            (wrestrict_dom_walker::check_call): Pull out into...
            (check_call): ...here.
            (check_bounds_or_overlap): Add range_query argument.
            * gimple-ssa-warn-restrict.h (check_bounds_or_overlap):
            Add range_query and gimple stmt arguments.

    gcc/testsuite/ChangeLog:

        * gcc.dg/Wrestrict-22.c: New test.
        * g++.dg/torture/pr92421.C: Adjust for ranger.

    libstdc++-v3/ChangeLog:

	* testsuite/21_strings/basic_string/capacity/1.cc: Pass
	-Wno-stringop-overflow to test.
---
 gcc/calls.c                                   | 26 ++++-
 gcc/calls.h                                   |  2 +
 gcc/gimple-ssa-warn-restrict.c                | 99 +++++++++++--------
 gcc/gimple-ssa-warn-restrict.h                |  3 +
 gcc/testsuite/g++.dg/torture/pr92421.C        |  4 +
 gcc/testsuite/gcc.dg/Wrestrict-22.c           |  9 ++
 .../21_strings/basic_string/capacity/1.cc     |  2 +
 7 files changed, 104 insertions(+), 41 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/Wrestrict-22.c

On 10/19/20 1:23 PM, Aldy Hernandez wrote:
> Rebased on current trunk.
>
> There is one adjustment to a C++ test which now gives a false positive.
> After talking with Martin Sebor, we've concluded this is expected.  There
> is no way to communicate that libstdc++ allocated objects are always
> less than PTRDIFF_MAX.

I think you have addressed the various issues with Martin, as much as 
can be.

we'll address a mechanism to access a ranger instance from the memref 
and access classes later, as well as the get_size_range duality. I  dont 
think we should hold this up as it isn't an issue for other passes, and 
we'll consider a more central solution down the road.

OK

Andrew


> OK?
>
>      gcc/ChangeLog:
>
>              * calls.c (get_size_range): Adjust to work with ranger.
>              * calls.h (get_size_range): Add ranger argument to prototype.
>              * gimple-ssa-warn-restrict.c (class wrestrict_dom_walker): Remove.
>              (check_call): Pull out of wrestrict_dom_walker into a
>              static function.
>              (wrestrict_dom_walker::before_dom_children): Rename to...
>              (wrestrict_walk): ...this.
>              (pass_wrestrict::execute): Instantiate ranger.
>              (class builtin_memref): Add stmt and query fields.
>              (builtin_access::builtin_access): Add range_query field.
>              (builtin_memref::builtin_memref): Same.
>              (builtin_memref::extend_offset_range): Same.
>              (builtin_access::builtin_access): Make work with ranger.
>              (wrestrict_dom_walker::check_call): Pull out into...
>              (check_call): ...here.
>              (check_bounds_or_overlap): Add range_query argument.
>              * gimple-ssa-warn-restrict.h (check_bounds_or_overlap):
>              Add range_query and gimple stmt arguments.
>
>      gcc/testsuite/ChangeLog:
>
>          * gcc.dg/Wrestrict-22.c: New test.
>          * g++.dg/torture/pr92421.C: Adjust for ranger.
>
>      libstdc++-v3/ChangeLog:
>
> 	* testsuite/21_strings/basic_string/capacity/1.cc: Pass
> 	-Wno-stringop-overflow to test.
> ---
>   gcc/calls.c                                   | 26 ++++-
>   gcc/calls.h                                   |  2 +
>   gcc/gimple-ssa-warn-restrict.c                | 99 +++++++++++--------
>   gcc/gimple-ssa-warn-restrict.h                |  3 +
>   gcc/testsuite/g++.dg/torture/pr92421.C        |  4 +
>   gcc/testsuite/gcc.dg/Wrestrict-22.c           |  9 ++
>   .../21_strings/basic_string/capacity/1.cc     |  2 +
>   7 files changed, 104 insertions(+), 41 deletions(-)
>   create mode 100644 gcc/testsuite/gcc.dg/Wrestrict-22.c
>
> diff --git a/gcc/calls.c b/gcc/calls.c
> index d3120b23f60..a12b84744c0 100644
> --- a/gcc/calls.c
> +++ b/gcc/calls.c
> @@ -59,6 +59,7 @@ along with GCC; see the file COPYING3.  If not see
>   #include "builtins.h"
>   #include "gimple-fold.h"
>   #include "attr-fnspec.h"
> +#include "value-query.h"
>   
>   #include "tree-pretty-print.h"
>   
> @@ -1244,7 +1245,8 @@ alloc_max_size (void)
>      in a multi-range, otherwise to the smallest valid subrange.  */
>   
>   bool
> -get_size_range (tree exp, tree range[2], int flags /* = 0 */)
> +get_size_range (range_query *query, tree exp, gimple *stmt, tree range[2],
> +		int flags /* = 0 */)
>   {
>     if (!exp)
>       return false;
> @@ -1263,7 +1265,21 @@ get_size_range (tree exp, tree range[2], int flags /* = 0 */)
>     enum value_range_kind range_type;
>   
>     if (integral)
> -    range_type = determine_value_range (exp, &min, &max);
> +    {
> +      value_range vr;
> +      if (query && query->range_of_expr (vr, exp, stmt))
> +	{
> +	  range_type = vr.kind ();
> +	  if (!vr.undefined_p ())
> +	    {
> +	      min = wi::to_wide (vr.min ());
> +	      max = wi::to_wide (vr.max ());
> +	    }
> +	}
> +      else
> +	range_type = determine_value_range (exp, &min, &max);
> +
> +    }
>     else
>       range_type = VR_VARYING;
>   
> @@ -1369,6 +1385,12 @@ get_size_range (tree exp, tree range[2], int flags /* = 0 */)
>     return true;
>   }
>   
> +bool
> +get_size_range (tree exp, tree range[2], int flags /* = 0 */)
> +{
> +  return get_size_range (/*query=*/NULL, exp, /*stmt=*/NULL, range, flags);
> +}
> +
>   /* Diagnose a call EXP to function FN decorated with attribute alloc_size
>      whose argument numbers given by IDX with values given by ARGS exceed
>      the maximum object size or cause an unsigned oveflow (wrapping) when
> diff --git a/gcc/calls.h b/gcc/calls.h
> index 644ec45d92c..f32b6308b58 100644
> --- a/gcc/calls.h
> +++ b/gcc/calls.h
> @@ -142,6 +142,8 @@ enum size_range_flags
>      SR_USE_LARGEST = 2
>     };
>   extern bool get_size_range (tree, tree[2], int = 0);
> +extern bool get_size_range (class range_query *, tree, gimple *,
> +			    tree[2], int = 0);
>   extern rtx rtx_for_static_chain (const_tree, bool);
>   extern bool cxx17_empty_base_field_p (const_tree);
>   
> diff --git a/gcc/gimple-ssa-warn-restrict.c b/gcc/gimple-ssa-warn-restrict.c
> index e2734c81456..3a79e7240f9 100644
> --- a/gcc/gimple-ssa-warn-restrict.c
> +++ b/gcc/gimple-ssa-warn-restrict.c
> @@ -25,7 +25,6 @@
>   #include "backend.h"
>   #include "tree.h"
>   #include "gimple.h"
> -#include "domwalk.h"
>   #include "tree-pass.h"
>   #include "builtins.h"
>   #include "ssa.h"
> @@ -41,6 +40,7 @@
>   #include "calls.h"
>   #include "cfgloop.h"
>   #include "intl.h"
> +#include "gimple-range.h"
>   
>   namespace {
>   
> @@ -77,21 +77,10 @@ pass_wrestrict::gate (function *fun ATTRIBUTE_UNUSED)
>     return warn_array_bounds || warn_restrict || warn_stringop_overflow;
>   }
>   
> -/* Class to walk the basic blocks of a function in dominator order.  */
> -class wrestrict_dom_walker : public dom_walker
> -{
> - public:
> -  wrestrict_dom_walker () : dom_walker (CDI_DOMINATORS) {}
> +static void check_call (range_query *, gimple *);
>   
> -  edge before_dom_children (basic_block) FINAL OVERRIDE;
> -  bool handle_gimple_call (gimple_stmt_iterator *);
> -
> - private:
> -  void check_call (gimple *);
> -};
> -
> -edge
> -wrestrict_dom_walker::before_dom_children (basic_block bb)
> +static void
> +wrestrict_walk (range_query *query, basic_block bb)
>   {
>     /* Iterate over statements, looking for function calls.  */
>     for (gimple_stmt_iterator si = gsi_start_bb (bb); !gsi_end_p (si);
> @@ -101,21 +90,17 @@ wrestrict_dom_walker::before_dom_children (basic_block bb)
>         if (!is_gimple_call (stmt))
>   	continue;
>   
> -      check_call (stmt);
> +      check_call (query, stmt);
>       }
> -
> -  return NULL;
>   }
>   
> -/* Execute the pass for function FUN, walking in dominator order.  */
> -
>   unsigned
>   pass_wrestrict::execute (function *fun)
>   {
> -  calculate_dominance_info (CDI_DOMINATORS);
> -
> -  wrestrict_dom_walker walker;
> -  walker.walk (ENTRY_BLOCK_PTR_FOR_FN (fun));
> +  gimple_ranger ranger;
> +  basic_block bb;
> +  FOR_EACH_BB_FN (bb, fun)
> +    wrestrict_walk (&ranger, bb);
>   
>     return 0;
>   }
> @@ -159,11 +144,15 @@ public:
>        only the destination reference is.  */
>     bool strbounded_p;
>   
> -  builtin_memref (tree, tree);
> +  builtin_memref (range_query *, gimple *, tree, tree);
>   
>     tree offset_out_of_bounds (int, offset_int[3]) const;
>   
>   private:
> +  /* Call statement to the built-in.  */
> +  gimple *stmt;
> +
> +  range_query *query;
>   
>     /* Ctor helper to set or extend OFFRANGE based on argument.  */
>     void extend_offset_range (tree);
> @@ -197,7 +186,7 @@ class builtin_access
>   	    && detect_overlap != &builtin_access::no_overlap);
>     }
>   
> -  builtin_access (gimple *, builtin_memref &, builtin_memref &);
> +  builtin_access (range_query *, gimple *, builtin_memref &, builtin_memref &);
>   
>     /* Entry point to determine overlap.  */
>     bool overlap ();
> @@ -233,9 +222,10 @@ class builtin_access
>   
>   /* Initialize a memory reference representation from a pointer EXPR and
>      a size SIZE in bytes.  If SIZE is NULL_TREE then the size is assumed
> -   to be unknown.  */
> +   to be unknown.  STMT is the statement in which expr appears in.  */
>   
> -builtin_memref::builtin_memref (tree expr, tree size)
> +builtin_memref::builtin_memref (range_query *query, gimple *stmt, tree expr,
> +				tree size)
>   : ptr (expr),
>     ref (),
>     base (),
> @@ -245,7 +235,9 @@ builtin_memref::builtin_memref (tree expr, tree size)
>     offrange (),
>     sizrange (),
>     maxobjsize (tree_to_shwi (max_object_size ())),
> -  strbounded_p ()
> +  strbounded_p (),
> +  stmt (stmt),
> +  query (query)
>   {
>     /* Unfortunately, wide_int default ctor is a no-op so array members
>        of the type must be set individually.  */
> @@ -264,7 +256,7 @@ builtin_memref::builtin_memref (tree expr, tree size)
>         tree range[2];
>         /* Determine the size range, allowing for the result to be [0, 0]
>   	 for SIZE in the anti-range ~[0, N] where N >= PTRDIFF_MAX.  */
> -      get_size_range (size, range, SR_ALLOW_ZERO);
> +      get_size_range (query, size, stmt, range, SR_ALLOW_ZERO);
>         sizrange[0] = wi::to_offset (range[0]);
>         sizrange[1] = wi::to_offset (range[1]);
>         /* get_size_range returns SIZE_MAX for the maximum size.
> @@ -341,7 +333,24 @@ builtin_memref::extend_offset_range (tree offset)
>         /* A pointer offset is represented as sizetype but treated
>   	 as signed.  */
>         wide_int min, max;
> -      value_range_kind rng = get_range_info (offset, &min, &max);
> +      value_range_kind rng;
> +      value_range vr;
> +      if (query && query->range_of_expr (vr, offset, stmt))
> +	{
> +	  rng = vr.kind ();
> +	  if (!vr.undefined_p ())
> +	    {
> +	      min = wi::to_wide (vr.min ());
> +	      max = wi::to_wide (vr.max ());
> +	    }
> +	}
> +      else
> +	{
> +	  /* There is a global version here because
> +	     check_bounds_or_overlap may be called from gimple
> +	     fold during gimple lowering.  */
> +	  rng = get_range_info (offset, &min, &max);
> +	}
>         if (rng == VR_ANTI_RANGE && wi::lts_p (max, min))
>   	{
>   	  /* Convert an anti-range whose upper bound is less than
> @@ -658,7 +667,8 @@ builtin_memref::offset_out_of_bounds (int strict, offset_int ooboff[3]) const
>   /* Create an association between the memory references DST and SRC
>      for access by a call EXPR to a memory or string built-in funtion.  */
>   
> -builtin_access::builtin_access (gimple *call, builtin_memref &dst,
> +builtin_access::builtin_access (range_query *query, gimple *call,
> +				builtin_memref &dst,
>   				builtin_memref &src)
>   : dstref (&dst), srcref (&src), sizrange (), ovloff (), ovlsiz (),
>     dstoff (), srcoff (), dstsiz (), srcsiz ()
> @@ -797,7 +807,7 @@ builtin_access::builtin_access (gimple *call, builtin_memref &dst,
>   
>         tree size = gimple_call_arg (call, sizeargno);
>         tree range[2];
> -      if (get_size_range (size, range, true))
> +      if (get_size_range (query, size, call, range, true))
>   	{
>   	  bounds[0] = wi::to_offset (range[0]);
>   	  bounds[1] = wi::to_offset (range[1]);
> @@ -1873,8 +1883,8 @@ maybe_diag_access_bounds (gimple *call, tree func, int strict,
>   /* Check a CALL statement for restrict-violations and issue warnings
>      if/when appropriate.  */
>   
> -void
> -wrestrict_dom_walker::check_call (gimple *call)
> +static void
> +check_call (range_query *query, gimple *call)
>   {
>     /* Avoid checking the call if it has already been diagnosed for
>        some reason.  */
> @@ -1964,7 +1974,7 @@ wrestrict_dom_walker::check_call (gimple *call)
>         || (dstwr && !INTEGRAL_TYPE_P (TREE_TYPE (dstwr))))
>       return;
>   
> -  if (!check_bounds_or_overlap (call, dst, src, dstwr, NULL_TREE))
> +  if (!check_bounds_or_overlap (query, call, dst, src, dstwr, NULL_TREE))
>       return;
>   
>     /* Avoid diagnosing the call again.  */
> @@ -1984,15 +1994,26 @@ int
>   check_bounds_or_overlap (gimple *call, tree dst, tree src, tree dstsize,
>   			 tree srcsize, bool bounds_only /* = false */,
>   			 bool do_warn /* = true */)
> +{
> +  return check_bounds_or_overlap (/*range_query=*/NULL,
> +				  call, dst, src, dstsize, srcsize,
> +				  bounds_only, do_warn);
> +}
> +
> +int
> +check_bounds_or_overlap (range_query *query,
> +			 gimple *call, tree dst, tree src, tree dstsize,
> +			 tree srcsize, bool bounds_only /* = false */,
> +			 bool do_warn /* = true */)
>   {
>     tree func = gimple_call_fndecl (call);
>   
> -  builtin_memref dstref (dst, dstsize);
> -  builtin_memref srcref (src, srcsize);
> +  builtin_memref dstref (query, call, dst, dstsize);
> +  builtin_memref srcref (query, call, src, srcsize);
>   
>     /* Create a descriptor of the access.  This may adjust both DSTREF
>        and SRCREF based on one another and the kind of the access.  */
> -  builtin_access acs (call, dstref, srcref);
> +  builtin_access acs (query, call, dstref, srcref);
>   
>     /* Set STRICT to the value of the -Warray-bounds=N argument for
>        string functions or when N > 1.  */
> diff --git a/gcc/gimple-ssa-warn-restrict.h b/gcc/gimple-ssa-warn-restrict.h
> index 7bae95a9ad1..3dba9c0fe58 100644
> --- a/gcc/gimple-ssa-warn-restrict.h
> +++ b/gcc/gimple-ssa-warn-restrict.h
> @@ -22,5 +22,8 @@
>   
>   extern int check_bounds_or_overlap (gimple *, tree, tree, tree, tree,
>   				    bool = false, bool = true);
> +extern int check_bounds_or_overlap (class range_query *, gimple *,
> +				    tree, tree, tree, tree,
> +				    bool = false, bool = true);
>   
>   #endif /* GIMPLE_SSA_WARN_RESTRICT_H */
> diff --git a/gcc/testsuite/g++.dg/torture/pr92421.C b/gcc/testsuite/g++.dg/torture/pr92421.C
> index 2146e9416db..0489eb75d9a 100644
> --- a/gcc/testsuite/g++.dg/torture/pr92421.C
> +++ b/gcc/testsuite/g++.dg/torture/pr92421.C
> @@ -2,6 +2,10 @@
>   // { dg-do compile }
>   // { dg-additional-options "-Wno-return-type" }
>   
> +// VRP jump threading will create additional __builtin___memcpy_chk calls that
> +// may be out of bounds.
> +// { dg-additional-options "-Wno-stringop-overflow" }
> +
>   typedef long a;
>   void *b, *c;
>   template <typename, typename> class d {};
> diff --git a/gcc/testsuite/gcc.dg/Wrestrict-22.c b/gcc/testsuite/gcc.dg/Wrestrict-22.c
> new file mode 100644
> index 00000000000..46f507b56b6
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/Wrestrict-22.c
> @@ -0,0 +1,9 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Wrestrict" } */
> +
> +void test_memcpy_warn (char *d, unsigned n)
> +{
> +  for (unsigned i = n; i < 30; ++i)
> +    if (i > 10)
> +      __builtin_memcpy (d, d + 2, i); /* { dg-warning "overlaps" } */
> +}
> diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/capacity/1.cc b/libstdc++-v3/testsuite/21_strings/basic_string/capacity/1.cc
> index a60f2d62da8..92f2bf40f14 100644
> --- a/libstdc++-v3/testsuite/21_strings/basic_string/capacity/1.cc
> +++ b/libstdc++-v3/testsuite/21_strings/basic_string/capacity/1.cc
> @@ -17,6 +17,8 @@
>   // with this library; see the file COPYING3.  If not see
>   // <http://www.gnu.org/licenses/>.
>   
> +// { dg-options "-Wno-stringop-overflow" }
> +
>   // 21.3.3 string capacity
>   
>   #include <string>