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 <ja...@redhat.com> > Richard Biener <rguent...@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 <rguent...@suse.de> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)