From patchwork Tue Aug 18 00:26:07 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Evans X-Patchwork-Id: 8254 Received: (qmail 129649 invoked by alias); 18 Aug 2015 00:26:12 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 129640 invoked by uid 89); 18 Aug 2015 00:26:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.4 required=5.0 tests=AWL, BAYES_05, KAM_STOCKGEN, MIME_BASE64_BLANKS, RCVD_IN_DNSWL_LOW, RP_MATCHES_RCVD, SPF_PASS, UNSUBSCRIBE_BODY autolearn=no version=3.3.2 X-HELO: mail-pd0-f202.google.com Received: from mail-pd0-f202.google.com (HELO mail-pd0-f202.google.com) (209.85.192.202) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 18 Aug 2015 00:26:09 +0000 Received: by pdbfa8 with SMTP id fa8so14378549pdb.1 for ; Mon, 17 Aug 2015 17:26:07 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:message-id:date:subject:from:to:cc :content-type:content-transfer-encoding; bh=R+YOEUR2SXw5XFEShgjbKIHsfu4LcSZFLyVUIEgIPAw=; b=bytsOnogFny6K9KzJlkwxRxjn1Z30cCFKBM8XdCuqnYAiKZTZQmlMOR/PoegQSOCOp o5W709z6//wAcGtJAlzNO0e9SDzXLrmFSBDGizDCNzM7YUPoaCX6pDFZm5UvqK5zEZOf rXe6OQDUsHL6YMZ1balLBVDCHjtKizHILxUC6Ax/2sWxPRVlxwW/lcmvdXcEVGen0kmQ svDxNGyNdt+2+knCF/bHK3tqGCDxjHYmZMWjqePDPAONLfa8s7VjKAqIrlNSQ3q+y7H5 7HujPYRwojKMx+bBIR2rQsxHbxVlbhZvvL57IijeHeeUUkaOd4HM+2t0GaYfsJ/Sx/9B E1Xw== X-Gm-Message-State: ALoCoQkY+uwt3SSSkYIj7hL4P6lxaKNkea+xNNyq1T3VGAO/hMDxUJ3opspRVPvF90PAjTSnWbQR MIME-Version: 1.0 X-Received: by 10.66.141.137 with SMTP id ro9mr3487326pab.46.1439857567775; Mon, 17 Aug 2015 17:26:07 -0700 (PDT) Message-ID: <001a113308e8dafbc3051d8af62f@google.com> Date: Tue, 18 Aug 2015 00:26:07 +0000 Subject: Re: [PATCH v2] [PR symtab/17391] gdb internal error: assertion fails in regcache.c:178 From: Doug Evans To: Doug Evans Cc: Pedro Alves , gdb-patches@sourceware.org X-IsSubscribed: yes 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 > > > > > > > > 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 * 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; }