Message ID | b2a9389a-68c0-4142-bf09-6afa7677f9c0@EXCHHUB01.MIPS.com |
---|---|
State | New |
Headers | show |
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.
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; } @@ -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,