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

[Pedro Alves <palves at redhat dot com>]

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?

> 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.

Thanks,
Pedro Alves