| Message ID | CAAgBjM=Bg7AiBu1_8+wTce=kh1yWuq+n9-bTuDmgG+1djEt45w@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Dec 1, 2016 at 11:07 AM, Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> wrote: > Hi, > As mentioned in PR, the issue seems to be that in > propagate_bits_accross_jump_functions(), > ipa_get_type() returns record_type during WPA and hence we pass > invalid precision to > ipcp_bits_lattice::meet_with (value, mask, precision) which eventually > leads to runtime error. > The attached patch tries to fix that, by bailing out if type of param > is not integral or pointer type. > This happens for the edge from deque_test -> _Z4copyIPd1BEvT_S2_T0_.isra.0/9. Feels more like a DECL_BY_REFERENCE mishandling and should be fixed elsewhere. > However I am not sure how ipcp_bits_lattice::meet_with (value, mask, > precision) gets called for this case. In > ipa_compute_jump_functions_for_edge(), we set jfunc->bits.known to > true only > if parm's type satisfies INTEGRAL_TYPE_P or POINTER_TYPE_P. > And ipcp_bits_lattice::meet_with (value, mask, precision) is called > only if jfunc->bits.known > is set to true. So I suppose it shouldn't really happen that > ipcp_bits_lattice::meet_with(value, mask, precision) gets called when > callee parameter's type is record_type, since the corresponding > argument's type would also need to be record_type and > jfunc->bits.known would be set to false. > > Without -flto, parm_type is reference_type so that satisfies POINTER_TYPE_P, > but with -flto it's appearing to be record_type. Is this possibly the > same issue of TYPE_ARG_TYPES returning bogus types during WPA ? > > I verified the attached patch fixes the runtime error with ubsan-built gcc. > Bootstrap+tested on x86_64-unknown-linux-gnu. > Cross-tested on arm*-*-*, aarch64*-*-*. > LTO bootstrap on x86_64-unknown-linux-gnu in progress. > Is it OK to commit if it succeeds ? > > Thanks, > Prathamesh
Hi, On Thu, Dec 01, 2016 at 01:43:16PM +0100, Richard Biener wrote: > On Thu, Dec 1, 2016 at 11:07 AM, Prathamesh Kulkarni > <prathamesh.kulkarni@linaro.org> wrote: > > Hi, > > As mentioned in PR, the issue seems to be that in > > propagate_bits_accross_jump_functions(), > > ipa_get_type() returns record_type during WPA and hence we pass > > invalid precision to > > ipcp_bits_lattice::meet_with (value, mask, precision) which eventually > > leads to runtime error. > > The attached patch tries to fix that, by bailing out if type of param > > is not integral or pointer type. > > This happens for the edge from deque_test -> _Z4copyIPd1BEvT_S2_T0_.isra.0/9. > > Feels more like a DECL_BY_REFERENCE mishandling and should be fixed elsewhere. That is indeed what is happening. Prathamesh, if you are going to save the type of arguments in the jump function, it should help you also with this issue. I know it was me who suggested using the function type to get at them and am sorry, I did not realize there potential issues with promotions and by_reference passing. By the way, please be careful not to introduce code style violations, especially lines exceeding 80 characters and adding trailing whitespace (propagate_bits_accross_jump_function has a few instances of both), I'd suggest setting your editor to highlight them. Thanks, Martin
On 3 December 2016 at 00:25, Martin Jambor <mjambor@suse.cz> wrote: > Hi, > > On Thu, Dec 01, 2016 at 01:43:16PM +0100, Richard Biener wrote: >> On Thu, Dec 1, 2016 at 11:07 AM, Prathamesh Kulkarni >> <prathamesh.kulkarni@linaro.org> wrote: >> > Hi, >> > As mentioned in PR, the issue seems to be that in >> > propagate_bits_accross_jump_functions(), >> > ipa_get_type() returns record_type during WPA and hence we pass >> > invalid precision to >> > ipcp_bits_lattice::meet_with (value, mask, precision) which eventually >> > leads to runtime error. >> > The attached patch tries to fix that, by bailing out if type of param >> > is not integral or pointer type. >> > This happens for the edge from deque_test -> _Z4copyIPd1BEvT_S2_T0_.isra.0/9. >> >> Feels more like a DECL_BY_REFERENCE mishandling and should be fixed elsewhere. > > That is indeed what is happening. Prathamesh, if you are going to save > the type of arguments in the jump function, it should help you also > with this issue. I know it was me who suggested using the function > type to get at them and am sorry, I did not realize there potential > issues with promotions and by_reference passing. > > By the way, please be careful not to introduce code style violations, > especially lines exceeding 80 characters and adding trailing > whitespace (propagate_bits_accross_jump_function has a few instances > of both), I'd suggest setting your editor to highlight them. Oops sorry about that, I will pay more attention to formatting henceforth. Using editor to highlight stray whitespace is indeed quite useful, thanks for that suggestion. Kugan has a patch for adding param type to jump function: https://gcc.gnu.org/ml/gcc-patches/2016-11/msg02732.html Once that gets committed, I will send a patch to use jfunc->param_type in propagate_bits_accross_jump_function(). Thanks, Prathamesh > > Thanks, > > Martin >
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c index 2ec671f..28eb74c 100644 --- a/gcc/ipa-cp.c +++ b/gcc/ipa-cp.c @@ -1770,12 +1770,15 @@ propagate_bits_accross_jump_function (cgraph_edge *cs, int idx, ipa_jump_func *j tree parm_type = ipa_get_type (callee_info, idx); /* For K&R C programs, ipa_get_type() could return NULL_TREE. - Avoid the transform for these cases. */ - if (!parm_type) + Avoid the transform for these cases or if parm type is not + integral or pointer type. */ + if (!parm_type + || !(INTEGRAL_TYPE_P (parm_type) || POINTER_TYPE_P (parm_type))) { if (dump_file && (dump_flags & TDF_DETAILS)) fprintf (dump_file, "Setting dest_lattice to bottom, because" - " param %i type is NULL for %s\n", idx, + " param %i type is %s for %s\n", idx, + (parm_type == NULL) ? "NULL" : "non-integral", cs->callee->name ()); return dest_lattice->set_to_bottom ();