[PATCH] passes: Fix up subobject __bos [PR101419]
Richard Biener
rguenther@suse.de
Tue Jul 13 08:19:39 GMT 2021
On Tue, 13 Jul 2021, Jakub Jelinek wrote:
> Hi!
>
> The following testcase is miscompiled, because VN during cunrolli changes
> __bos argument from address of a larger field to address of a smaller field
> and so __builtin_object_size (, 1) then folds into smaller value than the
> actually available size.
> copy_reference_ops_from_ref has a hack for this, but it was using
> cfun->after_inlining as a check whether the hack can be ignored, and
> cunrolli is after_inlining.
>
> This patch uses a property to make it exact (set at the end of objsz
> pass that doesn't do insert_min_max_p) and additionally based on discussions
> in the PR moves the objsz pass earlier after IPA.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Thanks,
Richard.
> 2021-07-13 Jakub Jelinek <jakub@redhat.com>
> Richard Biener <rguenther@suse.de>
>
> PR tree-optimization/101419
> * tree-pass.h (PROP_objsz): Define.
> (make_pass_early_object_sizes): Declare.
> * passes.def (pass_all_early_optimizations): Rename pass_object_sizes
> there to pass_early_object_sizes, drop parameter.
> (pass_all_optimizations): Move pass_object_sizes right after pass_ccp,
> drop parameter, move pass_post_ipa_warn right after that.
> * tree-object-size.c (pass_object_sizes::execute): Rename to...
> (object_sizes_execute): ... this. Add insert_min_max_p argument.
> (pass_data_object_sizes): Move after object_sizes_execute.
> (pass_object_sizes): Likewise. In execute method call
> object_sizes_execute, drop set_pass_param method and insert_min_max_p
> non-static data member and its initializer in the ctor.
> (pass_data_early_object_sizes, pass_early_object_sizes,
> make_pass_early_object_sizes): New.
> * tree-ssa-sccvn.c (copy_reference_ops_from_ref): Use
> (cfun->curr_properties & PROP_objsz) instead of cfun->after_inlining.
>
> * gcc.dg/builtin-object-size-10.c: Pass -fdump-tree-early_objsz-details
> instead of -fdump-tree-objsz1-details in dg-options and adjust names
> of dump file in scan-tree-dump.
> * gcc.dg/pr101419.c: New test.
>
> --- gcc/tree-pass.h.jj 2021-01-27 10:10:00.525903635 +0100
> +++ gcc/tree-pass.h 2021-07-12 17:23:42.322648068 +0200
> @@ -208,6 +208,7 @@ protected:
> #define PROP_gimple_lcf (1 << 1) /* lowered control flow */
> #define PROP_gimple_leh (1 << 2) /* lowered eh */
> #define PROP_cfg (1 << 3)
> +#define PROP_objsz (1 << 4) /* object sizes computed */
> #define PROP_ssa (1 << 5)
> #define PROP_no_crit_edges (1 << 6)
> #define PROP_rtl (1 << 7)
> @@ -426,6 +427,7 @@ extern gimple_opt_pass *make_pass_omp_ta
> extern gimple_opt_pass *make_pass_oacc_device_lower (gcc::context *ctxt);
> extern gimple_opt_pass *make_pass_omp_device_lower (gcc::context *ctxt);
> extern gimple_opt_pass *make_pass_object_sizes (gcc::context *ctxt);
> +extern gimple_opt_pass *make_pass_early_object_sizes (gcc::context *ctxt);
> extern gimple_opt_pass *make_pass_warn_printf (gcc::context *ctxt);
> extern gimple_opt_pass *make_pass_strlen (gcc::context *ctxt);
> extern gimple_opt_pass *make_pass_fold_builtins (gcc::context *ctxt);
> --- gcc/passes.def.jj 2021-05-19 09:16:34.434046683 +0200
> +++ gcc/passes.def 2021-07-12 17:41:38.274859148 +0200
> @@ -74,7 +74,7 @@ along with GCC; see the file COPYING3.
> NEXT_PASS (pass_all_early_optimizations);
> PUSH_INSERT_PASSES_WITHIN (pass_all_early_optimizations)
> NEXT_PASS (pass_remove_cgraph_callee_edges);
> - NEXT_PASS (pass_object_sizes, true /* insert_min_max_p */);
> + NEXT_PASS (pass_early_object_sizes);
> /* Don't record nonzero bits before IPA to avoid
> using too much memory. */
> NEXT_PASS (pass_ccp, false /* nonzero_p */);
> @@ -194,14 +194,14 @@ along with GCC; see the file COPYING3.
> They ensure memory accesses are not indirect wherever possible. */
> NEXT_PASS (pass_strip_predict_hints, false /* early_p */);
> NEXT_PASS (pass_ccp, true /* nonzero_p */);
> - NEXT_PASS (pass_post_ipa_warn);
> /* After CCP we rewrite no longer addressed locals into SSA
> form if possible. */
> + NEXT_PASS (pass_object_sizes);
> + NEXT_PASS (pass_post_ipa_warn);
> NEXT_PASS (pass_complete_unrolli);
> NEXT_PASS (pass_backprop);
> NEXT_PASS (pass_phiprop);
> NEXT_PASS (pass_forwprop);
> - NEXT_PASS (pass_object_sizes, false /* insert_min_max_p */);
> /* pass_build_alias is a dummy pass that ensures that we
> execute TODO_rebuild_alias at this point. */
> NEXT_PASS (pass_build_alias);
> --- gcc/tree-object-size.c.jj 2021-01-04 10:25:39.911221618 +0100
> +++ gcc/tree-object-size.c 2021-07-12 17:47:30.497018569 +0200
> @@ -1304,45 +1304,6 @@ fini_object_sizes (void)
> }
> }
>
> -
> -/* Simple pass to optimize all __builtin_object_size () builtins. */
> -
> -namespace {
> -
> -const pass_data pass_data_object_sizes =
> -{
> - GIMPLE_PASS, /* type */
> - "objsz", /* name */
> - OPTGROUP_NONE, /* optinfo_flags */
> - TV_NONE, /* tv_id */
> - ( PROP_cfg | PROP_ssa ), /* properties_required */
> - 0, /* properties_provided */
> - 0, /* properties_destroyed */
> - 0, /* todo_flags_start */
> - 0, /* todo_flags_finish */
> -};
> -
> -class pass_object_sizes : public gimple_opt_pass
> -{
> -public:
> - pass_object_sizes (gcc::context *ctxt)
> - : gimple_opt_pass (pass_data_object_sizes, ctxt), insert_min_max_p (false)
> - {}
> -
> - /* opt_pass methods: */
> - opt_pass * clone () { return new pass_object_sizes (m_ctxt); }
> - void set_pass_param (unsigned int n, bool param)
> - {
> - gcc_assert (n == 0);
> - insert_min_max_p = param;
> - }
> - virtual unsigned int execute (function *);
> -
> - private:
> - /* Determines whether the pass instance creates MIN/MAX_EXPRs. */
> - bool insert_min_max_p;
> -}; // class pass_object_sizes
> -
> /* Dummy valueize function. */
>
> static tree
> @@ -1351,8 +1312,8 @@ do_valueize (tree t)
> return t;
> }
>
> -unsigned int
> -pass_object_sizes::execute (function *fun)
> +static unsigned int
> +object_sizes_execute (function *fun, bool insert_min_max_p)
> {
> basic_block bb;
> FOR_EACH_BB_FN (bb, fun)
> @@ -1453,6 +1414,38 @@ pass_object_sizes::execute (function *fu
> return 0;
> }
>
> +/* Simple pass to optimize all __builtin_object_size () builtins. */
> +
> +namespace {
> +
> +const pass_data pass_data_object_sizes =
> +{
> + GIMPLE_PASS, /* type */
> + "objsz", /* name */
> + OPTGROUP_NONE, /* optinfo_flags */
> + TV_NONE, /* tv_id */
> + ( PROP_cfg | PROP_ssa ), /* properties_required */
> + PROP_objsz, /* properties_provided */
> + 0, /* properties_destroyed */
> + 0, /* todo_flags_start */
> + 0, /* todo_flags_finish */
> +};
> +
> +class pass_object_sizes : public gimple_opt_pass
> +{
> +public:
> + pass_object_sizes (gcc::context *ctxt)
> + : gimple_opt_pass (pass_data_object_sizes, ctxt)
> + {}
> +
> + /* opt_pass methods: */
> + opt_pass * clone () { return new pass_object_sizes (m_ctxt); }
> + virtual unsigned int execute (function *fun)
> + {
> + return object_sizes_execute (fun, false);
> + }
> +}; // class pass_object_sizes
> +
> } // anon namespace
>
> gimple_opt_pass *
> @@ -1460,3 +1453,42 @@ make_pass_object_sizes (gcc::context *ct
> {
> return new pass_object_sizes (ctxt);
> }
> +
> +/* Early version of pass to optimize all __builtin_object_size () builtins. */
> +
> +namespace {
> +
> +const pass_data pass_data_early_object_sizes =
> +{
> + GIMPLE_PASS, /* type */
> + "early_objsz", /* name */
> + OPTGROUP_NONE, /* optinfo_flags */
> + TV_NONE, /* tv_id */
> + ( PROP_cfg | PROP_ssa ), /* properties_required */
> + 0, /* properties_provided */
> + 0, /* properties_destroyed */
> + 0, /* todo_flags_start */
> + 0, /* todo_flags_finish */
> +};
> +
> +class pass_early_object_sizes : public gimple_opt_pass
> +{
> +public:
> + pass_early_object_sizes (gcc::context *ctxt)
> + : gimple_opt_pass (pass_data_early_object_sizes, ctxt)
> + {}
> +
> + /* opt_pass methods: */
> + virtual unsigned int execute (function *fun)
> + {
> + return object_sizes_execute (fun, true);
> + }
> +}; // class pass_object_sizes
> +
> +} // anon namespace
> +
> +gimple_opt_pass *
> +make_pass_early_object_sizes (gcc::context *ctxt)
> +{
> + return new pass_early_object_sizes (ctxt);
> +}
> --- gcc/tree-ssa-sccvn.c.jj 2021-06-09 10:20:08.988342285 +0200
> +++ gcc/tree-ssa-sccvn.c 2021-07-12 13:14:33.482962387 +0200
> @@ -925,12 +925,10 @@ copy_reference_ops_from_ref (tree ref, v
> + (wi::to_offset (bit_offset) >> LOG2_BITS_PER_UNIT));
> /* Probibit value-numbering zero offset components
> of addresses the same before the pass folding
> - __builtin_object_size had a chance to run
> - (checking cfun->after_inlining does the
> - trick here). */
> + __builtin_object_size had a chance to run. */
> if (TREE_CODE (orig) != ADDR_EXPR
> || maybe_ne (off, 0)
> - || cfun->after_inlining)
> + || (cfun->curr_properties & PROP_objsz))
> off.to_shwi (&temp.off);
> }
> }
> --- gcc/testsuite/gcc.dg/builtin-object-size-10.c.jj 2020-01-12 11:54:37.387398714 +0100
> +++ gcc/testsuite/gcc.dg/builtin-object-size-10.c 2021-07-12 17:44:00.795900485 +0200
> @@ -1,5 +1,5 @@
> /* { dg-do compile } */
> -/* { dg-options "-O2 -fdump-tree-objsz1-details" } */
> +/* { dg-options "-O2 -fdump-tree-early_objsz-details" } */
> // { dg-skip-if "packed attribute missing for drone_source_packet" { "epiphany-*-*" } }
>
> typedef struct {
> @@ -22,5 +22,5 @@ foo(char *x)
> return dpkt;
> }
>
> -/* { dg-final { scan-tree-dump "maximum object size 21" "objsz1" } } */
> -/* { dg-final { scan-tree-dump "maximum subobject size 16" "objsz1" } } */
> +/* { dg-final { scan-tree-dump "maximum object size 21" "early_objsz" } } */
> +/* { dg-final { scan-tree-dump "maximum subobject size 16" "early_objsz" } } */
> --- gcc/testsuite/gcc.dg/pr101419.c.jj 2021-07-12 13:33:47.492945100 +0200
> +++ gcc/testsuite/gcc.dg/pr101419.c 2021-07-12 13:33:33.756135703 +0200
> @@ -0,0 +1,62 @@
> +/* PR tree-optimization/101419 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +typedef __SIZE_TYPE__ size_t;
> +void baz (int, int) __attribute__((__warning__("detected overflow")));
> +
> +union U {
> + int i;
> + char c;
> +};
> +
> +static void
> +foo (union U *u)
> +{
> + if (__builtin_object_size (&u->c, 1) < sizeof (u->c))
> + baz (__builtin_object_size (&u->c, 1), sizeof (u->c)); /* { dg-bogus "detected overflow" } */
> + __builtin_memset (&u->c, 0, sizeof (u->c));
> +
> + if (__builtin_object_size (&u->i, 1) < sizeof (u->i))
> + baz (__builtin_object_size (&u->i, 1), sizeof (u->i)); /* { dg-bogus "detected overflow" } */
> + __builtin_memset (&u->i, 0, sizeof (u->i));
> +}
> +
> +void
> +bar (union U *u)
> +{
> + int i, j;
> + for (i = 0; i < 1; i++)
> + {
> + foo (u);
> + for (j = 0; j < 2; j++)
> + asm volatile ("");
> + }
> +}
> +
> +static void
> +qux (void *p, size_t q)
> +{
> + if (__builtin_object_size (p, 1) < q)
> + baz (__builtin_object_size (p, 1), q); /* { dg-bogus "detected overflow" } */
> + __builtin_memset (p, 0, q);
> +}
> +
> +static void
> +corge (union U *u)
> +{
> + qux (&u->c, sizeof (u->c));
> + qux (&u->i, sizeof (u->i));
> +}
> +
> +void
> +garply (union U *u)
> +{
> + int i, j;
> + for (i = 0; i < 1; i++)
> + {
> + corge (u);
> + for (j = 0; j < 2; j++)
> + asm volatile ("");
> + }
> +}
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
More information about the Gcc-patches
mailing list