| Message ID | a1976f86-fc8f-0685-e532-e8fb2aadcad0@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Oct 27, 2016 at 6:14 PM, Aldy Hernandez <aldyh@redhat.com> wrote: > On 10/27/2016 12:35 AM, Richard Biener wrote: >> >> On Wed, Oct 26, 2016 at 9:17 PM, Aldy Hernandez <aldyh@redhat.com> wrote: >>> >>> The following one-liner segfaults on arm-eabi when compiled with >>> -mfloat-abi=hard -g: >>> >>> __simd64_float16_t usingit; >>> >>> The problem is that the pretty printer (in simple_type_specificer()) is >>> dereferencing a NULL result from c_common_type_for_mode: >>> >>> int prec = TYPE_PRECISION (t); >>> if (ALL_FIXED_POINT_MODE_P (TYPE_MODE (t))) >>> t = c_common_type_for_mode (TYPE_MODE (t), TYPE_SATURATING >>> (t)); >>> else >>> t = c_common_type_for_mode (TYPE_MODE (t), TYPE_UNSIGNED >>> (t)); >>> if (TYPE_NAME (t)) >>> >>> The type in question is: >>> >>> <real_type 0x7fffefdeb150 HF ...> >>> >>> which corresponds to HFmode and which AFAICT, does not have a type by >>> design. >>> >>> I see that other uses of *type_for_node() throughout the compiler check >>> the >>> result for NULL, so perhaps we should do the same here. >>> >>> The attached patch fixes the problem. >>> >>> OK for trunk? >> >> >> Your added assert shows another possible issue - can you fix this by >> assigning >> the result of c_common_type_for_mode to a new variable, like common_t and >> use that for the TYPE_NAME (...) case? I think this was what was >> intended. > > > Certainly. > > OK pending tests? Ok. Thanks, Richard. > Aldy
On 10/28/2016 01:40 AM, Richard Biener wrote: > On Thu, Oct 27, 2016 at 6:14 PM, Aldy Hernandez <aldyh@redhat.com> wrote: >> On 10/27/2016 12:35 AM, Richard Biener wrote: >>> >>> On Wed, Oct 26, 2016 at 9:17 PM, Aldy Hernandez <aldyh@redhat.com> wrote: >>>> >>>> The following one-liner segfaults on arm-eabi when compiled with >>>> -mfloat-abi=hard -g: >>>> >>>> __simd64_float16_t usingit; >>>> >>>> The problem is that the pretty printer (in simple_type_specificer()) is >>>> dereferencing a NULL result from c_common_type_for_mode: >>>> >>>> int prec = TYPE_PRECISION (t); >>>> if (ALL_FIXED_POINT_MODE_P (TYPE_MODE (t))) >>>> t = c_common_type_for_mode (TYPE_MODE (t), TYPE_SATURATING >>>> (t)); >>>> else >>>> t = c_common_type_for_mode (TYPE_MODE (t), TYPE_UNSIGNED >>>> (t)); >>>> if (TYPE_NAME (t)) >>>> >>>> The type in question is: >>>> >>>> <real_type 0x7fffefdeb150 HF ...> >>>> >>>> which corresponds to HFmode and which AFAICT, does not have a type by >>>> design. >>>> >>>> I see that other uses of *type_for_node() throughout the compiler check >>>> the >>>> result for NULL, so perhaps we should do the same here. >>>> >>>> The attached patch fixes the problem. >>>> >>>> OK for trunk? >>> >>> >>> Your added assert shows another possible issue - can you fix this by >>> assigning >>> the result of c_common_type_for_mode to a new variable, like common_t and >>> use that for the TYPE_NAME (...) case? I think this was what was >>> intended. >> >> >> Certainly. >> >> OK pending tests? > > Ok. > Thanks. I just noticed this is also a GCC 6 regression. Assuming the GCC 6 branch is open for regression bugfixes, is this OK for the branch? Aldy
On October 28, 2016 6:46:07 PM GMT+02:00, Aldy Hernandez <aldyh@redhat.com> wrote: >On 10/28/2016 01:40 AM, Richard Biener wrote: >> On Thu, Oct 27, 2016 at 6:14 PM, Aldy Hernandez <aldyh@redhat.com> >wrote: >>> On 10/27/2016 12:35 AM, Richard Biener wrote: >>>> >>>> On Wed, Oct 26, 2016 at 9:17 PM, Aldy Hernandez <aldyh@redhat.com> >wrote: >>>>> >>>>> The following one-liner segfaults on arm-eabi when compiled with >>>>> -mfloat-abi=hard -g: >>>>> >>>>> __simd64_float16_t usingit; >>>>> >>>>> The problem is that the pretty printer (in >simple_type_specificer()) is >>>>> dereferencing a NULL result from c_common_type_for_mode: >>>>> >>>>> int prec = TYPE_PRECISION (t); >>>>> if (ALL_FIXED_POINT_MODE_P (TYPE_MODE (t))) >>>>> t = c_common_type_for_mode (TYPE_MODE (t), >TYPE_SATURATING >>>>> (t)); >>>>> else >>>>> t = c_common_type_for_mode (TYPE_MODE (t), >TYPE_UNSIGNED >>>>> (t)); >>>>> if (TYPE_NAME (t)) >>>>> >>>>> The type in question is: >>>>> >>>>> <real_type 0x7fffefdeb150 HF ...> >>>>> >>>>> which corresponds to HFmode and which AFAICT, does not have a type >by >>>>> design. >>>>> >>>>> I see that other uses of *type_for_node() throughout the compiler >check >>>>> the >>>>> result for NULL, so perhaps we should do the same here. >>>>> >>>>> The attached patch fixes the problem. >>>>> >>>>> OK for trunk? >>>> >>>> >>>> Your added assert shows another possible issue - can you fix this >by >>>> assigning >>>> the result of c_common_type_for_mode to a new variable, like >common_t and >>>> use that for the TYPE_NAME (...) case? I think this was what was >>>> intended. >>> >>> >>> Certainly. >>> >>> OK pending tests? >> >> Ok. >> > >Thanks. > >I just noticed this is also a GCC 6 regression. Assuming the GCC 6 >branch is open for regression bugfixes, is this OK for the branch? Yes. Richard. >Aldy
diff --git a/gcc/c-family/c-pretty-print.c b/gcc/c-family/c-pretty-print.c index 90428ca..7ad5900 100644 --- a/gcc/c-family/c-pretty-print.c +++ b/gcc/c-family/c-pretty-print.c @@ -344,14 +344,17 @@ c_pretty_printer::simple_type_specifier (tree t) else { int prec = TYPE_PRECISION (t); + tree common_t; if (ALL_FIXED_POINT_MODE_P (TYPE_MODE (t))) - t = c_common_type_for_mode (TYPE_MODE (t), TYPE_SATURATING (t)); + common_t = c_common_type_for_mode (TYPE_MODE (t), + TYPE_SATURATING (t)); else - t = c_common_type_for_mode (TYPE_MODE (t), TYPE_UNSIGNED (t)); - if (TYPE_NAME (t)) + common_t = c_common_type_for_mode (TYPE_MODE (t), + TYPE_UNSIGNED (t)); + if (common_t && TYPE_NAME (common_t)) { - simple_type_specifier (t); - if (TYPE_PRECISION (t) != prec) + simple_type_specifier (common_t); + if (TYPE_PRECISION (common_t) != prec) { pp_colon (this); pp_decimal_int (this, prec);