[mips,debug] Fix PR 54061, mips compiler aborts in testsuite

This is my attempt to fix PR 54061, a bug where GCC aborts while trying to
put out debug information for a co-processor register.  My understanding is
that gdb cannot access the coprocessor registers and so there is no valid
debug information that can be put out for this case.  So my fix is to remove
the assert from dbx_reg_number and change the callers to check for a
INVALID_REGNUM value coming from dbx_reg_number.  If they get this value
then they do not put out any debug information for the register.  I have
tested this on gcc.c-torture/compile/mipscop-[1234].c and it fixes the abort.

I haven't done a full regression test but will do that shortly, in the mean
time I wanted to see if this approach was considered acceptable and if not,
how I should fix it?

Steve Ellcey
sellcey@mips.com


2012-12-07  Steve Ellcey  <sellcey@mips.com>

	PR target/54061
	* dwarfwout.c (dbx_reg_number): Remove assert.
	(reg_loc_descriptor): Check for INVALID_REGNUM.
	(mem_loc_descriptor): Ditto.

On Fri, Dec 7, 2012 at 1:59 PM, Steve Ellcey <sellcey@mips.com> wrote:
> This is my attempt to fix PR 54061, a bug where GCC aborts while trying to
> put out debug information for a co-processor register.  My understanding is
> that gdb cannot access the coprocessor registers and so there is no valid
> debug information that can be put out for this case.  So my fix is to remove
> the assert from dbx_reg_number and change the callers to check for a
> INVALID_REGNUM value coming from dbx_reg_number.  If they get this value
> then they do not put out any debug information for the register.  I have
> tested this on gcc.c-torture/compile/mipscop-[1234].c and it fixes the abort.
>
> I haven't done a full regression test but will do that shortly, in the mean
> time I wanted to see if this approach was considered acceptable and if not,
> how I should fix it?
>
> Steve Ellcey
> sellcey@mips.com
>
>
> 2012-12-07  Steve Ellcey  <sellcey@mips.com>
>
>         PR target/54061
>         * dwarfwout.c (dbx_reg_number): Remove assert.
>         (reg_loc_descriptor): Check for INVALID_REGNUM.
>         (mem_loc_descriptor): Ditto.
>
> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> index f0256ae..7d26e7e 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -10438,7 +10438,6 @@ dbx_reg_number (const_rtx rtl)
>  #endif
>
>    regno = DBX_REGISTER_NUMBER (regno);
> -  gcc_assert (regno != INVALID_REGNUM);
>    return regno;
>  }

I added it for

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52857

In this bug, we used the wrong register to generate DWARF
info.  Your patch removes the assert, which may make GCC
silently generate bad DWARF info.

> @@ -10473,6 +10472,9 @@ reg_loc_descriptor (rtx rtl, enum var_init_status initialized)
>    if (REGNO (rtl) >= FIRST_PSEUDO_REGISTER)
>      return 0;
>
> +  if (dbx_reg_number(rtl) == INVALID_REGNUM)
> +    return 0;
> +
>    /* We only use "frame base" when we're sure we're talking about the
>       post-prologue local stack frame.  We do this by *not* running
>       register elimination until this point, and recognizing the special
> @@ -11931,6 +11933,8 @@ mem_loc_descriptor (rtx rtl, enum machine_mode mode,
>             break;
>           if (REGNO (rtl) > FIRST_PSEUDO_REGISTER)
>             break;
> +         if (dbx_reg_number (rtl) == INVALID_REGNUM)
> +           break;
>           type_die = base_type_for_mode (mode,
>                                          GET_MODE_CLASS (mode) == MODE_INT);
>           if (type_die == NULL)
> @@ -12133,6 +12137,9 @@ mem_loc_descriptor (rtx rtl, enum machine_mode mode,
>         return NULL;
>        if (REG_P (ENTRY_VALUE_EXP (rtl)))
>         {
> +         if (dbx_reg_number (ENTRY_VALUE_EXP (rtl)) == INVALID_REGNUM)
> +           return NULL;
> +
>           if (GET_MODE_CLASS (mode) != MODE_INT
>               || GET_MODE_SIZE (mode) > DWARF2_ADDR_SIZE)
>             op0 = mem_loc_descriptor (ENTRY_VALUE_EXP (rtl), mode,

I think you need a way to tell a bad register from a good register
which doesn't have DWARF register number.

On Fri, Dec 7, 2012 at 2:17 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Dec 7, 2012 at 1:59 PM, Steve Ellcey <sellcey@mips.com> wrote:
>> This is my attempt to fix PR 54061, a bug where GCC aborts while trying to
>> put out debug information for a co-processor register.  My understanding is
>> that gdb cannot access the coprocessor registers and so there is no valid
>> debug information that can be put out for this case.  So my fix is to remove
>> the assert from dbx_reg_number and change the callers to check for a
>> INVALID_REGNUM value coming from dbx_reg_number.  If they get this value
>> then they do not put out any debug information for the register.  I have
>> tested this on gcc.c-torture/compile/mipscop-[1234].c and it fixes the abort.
>>
>> I haven't done a full regression test but will do that shortly, in the mean
>> time I wanted to see if this approach was considered acceptable and if not,
>> how I should fix it?
>>
>> Steve Ellcey
>> sellcey@mips.com
>>
>>
>> 2012-12-07  Steve Ellcey  <sellcey@mips.com>
>>
>>         PR target/54061
>>         * dwarfwout.c (dbx_reg_number): Remove assert.
>>         (reg_loc_descriptor): Check for INVALID_REGNUM.
>>         (mem_loc_descriptor): Ditto.
>>
>> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
>> index f0256ae..7d26e7e 100644
>> --- a/gcc/dwarf2out.c
>> +++ b/gcc/dwarf2out.c
>> @@ -10438,7 +10438,6 @@ dbx_reg_number (const_rtx rtl)
>>  #endif
>>
>>    regno = DBX_REGISTER_NUMBER (regno);
>> -  gcc_assert (regno != INVALID_REGNUM);
>>    return regno;
>>  }
>
> I added it for
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52857
>
> In this bug, we used the wrong register to generate DWARF
> info.  Your patch removes the assert, which may make GCC
> silently generate bad DWARF info.
>
>> @@ -10473,6 +10472,9 @@ reg_loc_descriptor (rtx rtl, enum var_init_status initialized)
>>    if (REGNO (rtl) >= FIRST_PSEUDO_REGISTER)
>>      return 0;
>>
>> +  if (dbx_reg_number(rtl) == INVALID_REGNUM)
>> +    return 0;
>> +
>>    /* We only use "frame base" when we're sure we're talking about the
>>       post-prologue local stack frame.  We do this by *not* running
>>       register elimination until this point, and recognizing the special
>> @@ -11931,6 +11933,8 @@ mem_loc_descriptor (rtx rtl, enum machine_mode mode,
>>             break;
>>           if (REGNO (rtl) > FIRST_PSEUDO_REGISTER)
>>             break;
>> +         if (dbx_reg_number (rtl) == INVALID_REGNUM)
>> +           break;
>>           type_die = base_type_for_mode (mode,
>>                                          GET_MODE_CLASS (mode) == MODE_INT);
>>           if (type_die == NULL)
>> @@ -12133,6 +12137,9 @@ mem_loc_descriptor (rtx rtl, enum machine_mode mode,
>>         return NULL;
>>        if (REG_P (ENTRY_VALUE_EXP (rtl)))
>>         {
>> +         if (dbx_reg_number (ENTRY_VALUE_EXP (rtl)) == INVALID_REGNUM)
>> +           return NULL;
>> +
>>           if (GET_MODE_CLASS (mode) != MODE_INT
>>               || GET_MODE_SIZE (mode) > DWARF2_ADDR_SIZE)
>>             op0 = mem_loc_descriptor (ENTRY_VALUE_EXP (rtl), mode,
>
> I think you need a way to tell a bad register from a good register
> which doesn't have DWARF register number.
>

Something like

#define IGNORED_DWARF_REGNUM            (INVALID_REGNUM - 1)

On Fri, 2012-12-07 at 14:43 -0800, H.J. Lu wrote:

> > I think you need a way to tell a bad register from a good register
> > which doesn't have DWARF register number.
> >
> 
> Something like
> 
> #define IGNORED_DWARF_REGNUM            (INVALID_REGNUM - 1)

The other option would be to have a new macro that returned true or
false if a register had a dbx register number.

#define VALID_DBX_REGNUM(regno) (true)

and check that instead of checking the return of dbx_reg_number().

A target would override this default definition of VALID_DBX_REGNUM
for registers it wanted to ignore.  It should probably be a target
macro but so should DBX_REGISTER_NUMBER and DWARF_FRAME_REGNUM, we
probably don't want to be making that change at this time in the release
cycle.

Steve Ellcey
sellcey@mips.com

On Fri, Dec 7, 2012 at 2:55 PM, Steve Ellcey <sellcey@mips.com> wrote:
> On Fri, 2012-12-07 at 14:43 -0800, H.J. Lu wrote:
>
>> > I think you need a way to tell a bad register from a good register
>> > which doesn't have DWARF register number.
>> >
>>
>> Something like
>>
>> #define IGNORED_DWARF_REGNUM            (INVALID_REGNUM - 1)
>
> The other option would be to have a new macro that returned true or
> false if a register had a dbx register number.
>
> #define VALID_DBX_REGNUM(regno) (true)
>
> and check that instead of checking the return of dbx_reg_number().
>
> A target would override this default definition of VALID_DBX_REGNUM
> for registers it wanted to ignore.  It should probably be a target
> macro but so should DBX_REGISTER_NUMBER and DWARF_FRAME_REGNUM, we
> probably don't want to be making that change at this time in the release
> cycle.

One case is a valid register without valid DWARF register
number.  The other is an invalid register.  Both have
invalid DWARF register number.  I don't think
VALID_DBX_REGNUM works here.