Re: [PATCH v2] [PR symtab/17391] gdb internal error: assertion fails in regcache.c:178

[Doug Evans <dje at google dot com>]

Pedro Alves writes:
 > On 08/13/2015 02:21 AM, Doug Evans wrote:
 > > Doug Evans writes:
 > >   > Hi.
 > >   >
 > >   > This patch, I hope, addresses PR 17391.
 > >   > I couldn't recreate 17391 but there is clearly a lack of  
robustness here:
 > >   > gdbarch_dwarf2_reg_to_regnum can return -1 but not all callers
 > >   > watch for that.  Some callers do watch for it but call error  
themselves.
 > >   > There is already dwarf2_reg_to_regnum_or_error so this patch just
 > >   > changes appropriate callers of gdbarch_dwarf2_reg_to_regnum to use  
it.
 > >   > Sometimes a register number is stored as a ULONGEST, so I changed
 > >   > dwarf2_reg_to_regnum_or_error to take one.
 > >   >
 > >   > Regression tested on amd64-linux.
 > >   >
 > >   > 2015-08-10  Doug Evans  <dje@google.com>
 > >   >
 > >   > 	PR symtab/17391
 > >   > 	* dwarf2-frame.c (read_addr_from_reg): Call
 > >   > 	dwarf2_reg_to_regnum_or_error instead of  
gdbarch_dwarf2_reg_to_regnum.
 > >   > 	(get_reg_value): Ditto.
 > >   > 	(dwarf2_fetch_cfa_info): Ditto.
 > >   > 	(dwarf2_frame_prev_register): Ditto.
 > >   > 	* dwarf2loc.c (dwarf_expr_read_addr_from_reg): Ditto.
 > >   > 	(dwarf_expr_get_reg_value): Ditto.
 > >   > 	(read_pieced_value): Ditto.
 > >   > 	(write_pieced_value): Ditto.
 > >   > 	(dwarf2_evaluate_loc_desc_full): Ditto.
 > >   > 	(dwarf2_reg_to_regnum_or_error): Change to take a ULONGEST regnum.
 > >   > 	(locexpr_regname): Improve text of bad register number.
 > >   > 	* dwarf2loc.h (dwarf2_reg_to_regnum_or_error): Update prototype.
 > >   >
 > >   > 	testsuite/
 > >   > 	* lib/gdb.exp (_location): Add support for DW_OP_regx.
 > >   > 	* gdb.dwarf2/bad-regnum.c: New file.
 > >   > 	* gdb.dwarf2/bad-regnum.exp: New file.
 > >
 > > Hi.
 > >
 > > This is a revised version.
 > > Digging deeper I found several broken targets.
 > > I'm not prepared to fix all of them,
 >
 > OOC, was that because the fix looked complicated, or because
 > you wanted to avoid breaking them?  If the latter, I'd be more inclined
 > to avoiding leaving a wart in place that is probably going to take
 > a long while to address, even if that meant the fix goes in untested.
 > Is there an easy way to tell which targets still need fixing?  Or what
 > to look for?

More like for those I left it wasn't clear to me how to fix them.
e.g., rs6000-tdep.c:rs6000_dwarf2_reg_to_regnum.
I'm certainly not making things worse by leaving them alone.

 > > but this fixes a lot of them and adds documentation
 > > to actually specify what the rules are
 > > (as opposed to targets just cut-n-pasting
 > > and hoping it was right).
 >
 > Otherwise the patch looked fine to me.