[PATCH v2] [PR symtab/17391] gdb internal error: assertion fails in regcache.c:178
Doug Evans
dje@google.com
Tue Aug 18 00:26:00 GMT 2015
Doug Evans writes:
> 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.
I missed a few obvious cases.
The main remaining cases that I can think of are rs6000 and spu.
Plus this supplemental patch makes stabs and ecoff more robust.
Stabs was using num_arch + num_pseudo to denote "bad reg",
but some targets use the same reg_to_regnum function for both
stabs and dwarf, so that's asking for trouble (since dwarf uses -1).
This patch makes stabs handle negative regnos too.
If it's too hard to review this apart from the original patch
I can resubmit the entire thing.
2015-08-17 Doug Evans <dje@google.com>
* m68k-tdep.c (m68k_dwarf_reg_to_regnum): Fix error result.
* mep-tdep.c (mep_debug_reg_to_regnum): Ditto.
* mips-tdep.c (mips_stab_reg_to_regnum): Ditto.
(mips_dwarf_dwarf2_ecoff_reg_to_regnum): Ditto.
* mn10300-tdep.c (mn10300_dwarf2_reg_to_regnum): Remove warning
for bad regs.
* xtensa-tdep.c (xtensa_reg_to_regnum): Remove internal error for
bad regs. Fix error result.
* stabsread.c (stab_reg_to_regnum): Watch for negative regno.
(reg_value_complaint): Update complaint text.
* mdebugread.c (reg_value_complaint): New function.
(mdebug_reg_to_regnum): Rewrite to watch for bad reg numbers.
diff --git a/gdb/m68k-tdep.c b/gdb/m68k-tdep.c
index d7347b6..5a3ed95 100644
--- a/gdb/m68k-tdep.c
+++ b/gdb/m68k-tdep.c
@@ -573,7 +573,7 @@ m68k_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int
num)
/* pc */
return M68K_PC_REGNUM;
else
- return gdbarch_num_regs (gdbarch) + gdbarch_num_pseudo_regs (gdbarch);
+ return -1;
}
diff --git a/gdb/mdebugread.c b/gdb/mdebugread.c
index 3a81615..90f1f51 100644
--- a/gdb/mdebugread.c
+++ b/gdb/mdebugread.c
@@ -521,6 +521,14 @@ add_pending (FDR *fh, char *sh, struct type *t)
/* Parsing Routines proper. */
+static void
+reg_value_complaint (int regnum, int num_regs, const char *sym)
+{
+ complaint (&symfile_complaints,
+ _("bad register number %d (max %d) in symbol %s"),
+ regnum, num_regs - 1, sym);
+}
+
/* Parse a single symbol. Mostly just make up a GDB symbol for it.
For blocks, procedures and types we open a new lexical context.
This is basically just a big switch on the symbol's type. Argument
@@ -533,7 +541,21 @@ add_pending (FDR *fh, char *sh, struct type *t)
static int
mdebug_reg_to_regnum (struct symbol *sym, struct gdbarch *gdbarch)
{
- return gdbarch_ecoff_reg_to_regnum (gdbarch, SYMBOL_VALUE (sym));
+ int regno = gdbarch_ecoff_reg_to_regnum (gdbarch, SYMBOL_VALUE (sym));
+
+ if (regno < 0
+ || regno >= (gdbarch_num_regs (gdbarch)
+ + gdbarch_num_pseudo_regs (gdbarch)))
+ {
+ reg_value_complaint (regno,
+ gdbarch_num_regs (gdbarch)
+ + gdbarch_num_pseudo_regs (gdbarch),
+ SYMBOL_PRINT_NAME (sym));
+
+ regno = gdbarch_sp_regnum (gdbarch); /* Known safe, though useless.
*/
+ }
+
+ return regno;
}
static const struct symbol_register_ops mdebug_register_funcs = {
diff --git a/gdb/mep-tdep.c b/gdb/mep-tdep.c
index d2831db..7ab4928 100644
--- a/gdb/mep-tdep.c
+++ b/gdb/mep-tdep.c
@@ -784,7 +784,9 @@ static int
mep_debug_reg_to_regnum (struct gdbarch *gdbarch, int debug_reg)
{
/* The debug info uses the raw register numbers. */
- return mep_raw_to_pseudo[debug_reg];
+ if (debug_reg >= 0 && debug_reg < ARRAY_SIZE (mep_raw_to_pseudo))
+ return mep_raw_to_pseudo[debug_reg];
+ return -1;
}
diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index e0706db..4bf0d99 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -8020,9 +8020,7 @@ mips_stab_reg_to_regnum (struct gdbarch *gdbarch, int
num)
else if (mips_regnum (gdbarch)->dspacc != -1 && num >= 72 && num < 78)
regnum = num + mips_regnum (gdbarch)->dspacc - 72;
else
- /* This will hopefully (eventually) provoke a warning. Should
- we be calling complaint() here? */
- return gdbarch_num_regs (gdbarch) + gdbarch_num_pseudo_regs (gdbarch);
+ return -1;
return gdbarch_num_regs (gdbarch) + regnum;
}
@@ -8045,9 +8043,7 @@ mips_dwarf_dwarf2_ecoff_reg_to_regnum (struct gdbarch
*gdbarch, int num)
else if (mips_regnum (gdbarch)->dspacc != -1 && num >= 66 && num < 72)
regnum = num + mips_regnum (gdbarch)->dspacc - 66;
else
- /* This will hopefully (eventually) provoke a warning. Should we
- be calling complaint() here? */
- return gdbarch_num_regs (gdbarch) + gdbarch_num_pseudo_regs (gdbarch);
+ return -1;
return gdbarch_num_regs (gdbarch) + regnum;
}
diff --git a/gdb/mn10300-tdep.c b/gdb/mn10300-tdep.c
index 985821c..83a3e48 100644
--- a/gdb/mn10300-tdep.c
+++ b/gdb/mn10300-tdep.c
@@ -1383,10 +1383,7 @@ mn10300_dwarf2_reg_to_regnum (struct gdbarch
*gdbarch, int dwarf2)
if (dwarf2 < 0
|| dwarf2 >= ARRAY_SIZE (dwarf2_to_gdb))
- {
- warning (_("Bogus register number in debug info: %d"), dwarf2);
- return -1;
- }
+ return -1;
return dwarf2_to_gdb[dwarf2];
}
diff --git a/gdb/stabsread.c b/gdb/stabsread.c
index 03c9eb1..f6d8343 100644
--- a/gdb/stabsread.c
+++ b/gdb/stabsread.c
@@ -169,7 +169,7 @@ static void
reg_value_complaint (int regnum, int num_regs, const char *sym)
{
complaint (&symfile_complaints,
- _("register number %d too large (max %d) in symbol %s"),
+ _("bad register number %d (max %d) in symbol %s"),
regnum, num_regs - 1, sym);
}
@@ -598,8 +598,9 @@ stab_reg_to_regnum (struct symbol *sym, struct gdbarch
*gdbarch)
{
int regno = gdbarch_stab_reg_to_regnum (gdbarch, SYMBOL_VALUE (sym));
- if (regno >= gdbarch_num_regs (gdbarch)
- + gdbarch_num_pseudo_regs (gdbarch))
+ if (regno < 0
+ || regno >= (gdbarch_num_regs (gdbarch)
+ + gdbarch_num_pseudo_regs (gdbarch)))
{
reg_value_complaint (regno,
gdbarch_num_regs (gdbarch)
diff --git a/gdb/xtensa-tdep.c b/gdb/xtensa-tdep.c
index 55e7d98..6c60070 100644
--- a/gdb/xtensa-tdep.c
+++ b/gdb/xtensa-tdep.c
@@ -354,9 +354,7 @@ xtensa_reg_to_regnum (struct gdbarch *gdbarch, int
regnum)
if (regnum == gdbarch_tdep (gdbarch)->regmap[i].target_number)
return i;
- internal_error (__FILE__, __LINE__,
- _("invalid dwarf/stabs register number %d"), regnum);
- return 0;
+ return -1;
}
More information about the Gdb-patches
mailing list