PING: [PATCH] Convert -Wrestrict pass to ranger.
Andrew MacLeod
amacleod@redhat.com
Tue Oct 20 17:14:31 GMT 2020
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>
More information about the Gcc-patches
mailing list