From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32288 invoked by alias); 7 Jun 2007 20:59:21 -0000 Received: (qmail 32275 invoked by uid 22791); 7 Jun 2007 20:59:20 -0000 X-Spam-Check-By: sourceware.org Received: from mtagate2.de.ibm.com (HELO mtagate2.de.ibm.com) (195.212.29.151) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 07 Jun 2007 20:59:17 +0000 Received: from d12nrmr1607.megacenter.de.ibm.com (d12nrmr1607.megacenter.de.ibm.com [9.149.167.49]) by mtagate2.de.ibm.com (8.13.8/8.13.8) with ESMTP id l57KxEeB081864 for ; Thu, 7 Jun 2007 20:59:14 GMT Received: from d12av02.megacenter.de.ibm.com (d12av02.megacenter.de.ibm.com [9.149.165.228]) by d12nrmr1607.megacenter.de.ibm.com (8.13.8/8.13.8/NCO v8.3) with ESMTP id l57KxEF44108536 for ; Thu, 7 Jun 2007 22:59:14 +0200 Received: from d12av02.megacenter.de.ibm.com (loopback [127.0.0.1]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id l57KxE7X020033 for ; Thu, 7 Jun 2007 22:59:14 +0200 Received: from tuxmaker.boeblingen.de.ibm.com (tuxmaker.boeblingen.de.ibm.com [9.152.85.9]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.12.11) with SMTP id l57KxEst020028 for ; Thu, 7 Jun 2007 22:59:14 +0200 Message-Id: <200706072059.l57KxEst020028@d12av02.megacenter.de.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Thu, 7 Jun 2007 22:59:14 +0200 Subject: [rfc][6/13] Eliminate read_register: read_register (invalidly) used in prologue analyzers To: gdb-patches@sourceware.org Date: Thu, 07 Jun 2007 20:59:00 -0000 From: "Ulrich Weigand" X-Mailer: ELM [version 2.5 PL2] MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2007-06/txt/msg00085.txt.bz2 Hello, while eliminating the read_register family of routines, I noticed a couple of places where read_register is currently used in what appears to be an invalid fashion as part of prologue analyzer routines. Those routines are generally called from two places: the unwinder, in which case you should examine register values from the next frame instead of referring to global state, and the skip_prologue callback, which doesn't refer to *any* frame, but the global register state is invalid as well, since the callback can be invoked at any time (e.g. when re-evaluating breakpoints). The three problematic locations I found are: - v850_analyze_prologue uses read_register to retrieve the CTBP register (call table base pointer). As this is used *only* from within the unwinder, the patch adds a CTBP argument to v850_analyze_prologue and has the unwinder pass in the unwound value of the register. - Similarly, sh_analyze_prologue reads the FPSCR register to find out whether a floating-point "push" instruction decrements the stack pointer by 4 or 8 bytes. Again, I've added a FPSCR parameter and have the unwinder pass in the unwound register value. However, this routine is also called from the skip_prologue callback. In this case, we simply do not know the contents of that flags register. However, the only thing we get wrong if we don't know that setting is the stack offset of saved registers, and skip_prologue does not actually care about those. So just arbitrarily passing in 0 in that case should work fine. This all really needs to be separated out more cleanly ... - Finally, the mips{16,32}_scan_prologue routines are called either from the unwinder with a next_frame argument, or from skip_prologue with a NULL next_frame argument. They make use of a helper routine read_next_frame_reg that either unwinds from next_frame or else reads from the current_regcache. That strikes me as bogus, as the current_regcache isn't at all related to the function being queried via skip_prologue ... However, again the only thing we'd be getting wrong should be stack offsets of saved registers in the case of functions using alloca. So the patch below changes the scan_prologue routines to skip trying to handle that case if called without next_frame. Bye, Ulrich ChangeLog: * mips-tdep.c (mips16_scan_prologue): Replace read_next_frame_reg by frame_unwind_register_signed calls. (mips32_scan_prologue): Likewise. Skip analysis of alloca stack frame allocations when called with NULL NEXT_FRAME parameter. (read_next_frame_reg): Remove. * sh-tdep.c (sh_analyze_prologue): Add FPSCR parameter. Use it instead of reading the FPSCR register. (sh_frame_cache): Pass unwound FPSCR register value to sh_analyze_prologue. (sh_skip_prologue): Pass dummy FPSCR value to sh_analyze_prologue. * v850-tdep.c (v850_analyze_prologue): Add CTBP parameter. Use it instead of reading the CTBP register. (v850_frame_cache): Pass unwound CTBP register value to v850_analyze_prologue. diff -urNp gdb-orig/gdb/mips-tdep.c gdb-head/gdb/mips-tdep.c --- gdb-orig/gdb/mips-tdep.c 2007-06-06 18:57:49.422550908 +0200 +++ gdb-head/gdb/mips-tdep.c 2007-06-06 18:58:57.267028491 +0200 @@ -402,8 +402,6 @@ mips2_fp_compat (void) static CORE_ADDR heuristic_proc_start (CORE_ADDR); -static CORE_ADDR read_next_frame_reg (struct frame_info *, int); - static void reinit_frame_cache_sfunc (char *, int, struct cmd_list_element *); static struct type *mips_float_register_type (void); @@ -1436,8 +1434,9 @@ mips16_scan_prologue (CORE_ADDR start_pc /* Can be called when there's no process, and hence when there's no NEXT_FRAME. */ if (next_frame != NULL) - sp = read_next_frame_reg (next_frame, gdbarch_num_regs (current_gdbarch) - + MIPS_SP_REGNUM); + sp = frame_unwind_register_signed (next_frame, + gdbarch_num_regs (current_gdbarch) + + MIPS_SP_REGNUM); else sp = 0; @@ -1757,8 +1756,9 @@ mips32_scan_prologue (CORE_ADDR start_pc /* Can be called when there's no process, and hence when there's no NEXT_FRAME. */ if (next_frame != NULL) - sp = read_next_frame_reg (next_frame, gdbarch_num_regs (current_gdbarch) - + MIPS_SP_REGNUM); + sp = frame_unwind_register_signed (next_frame, + gdbarch_num_regs (current_gdbarch) + + MIPS_SP_REGNUM); else sp = 0; @@ -1808,14 +1808,15 @@ restart: /* Old gcc frame, r30 is virtual frame pointer. */ if ((long) low_word != frame_offset) frame_addr = sp + low_word; - else if (frame_reg == MIPS_SP_REGNUM) + else if (next_frame && frame_reg == MIPS_SP_REGNUM) { unsigned alloca_adjust; frame_reg = 30; - frame_addr = read_next_frame_reg (next_frame, - gdbarch_num_regs - (current_gdbarch) + 30); + frame_addr = frame_unwind_register_signed + (next_frame, + gdbarch_num_regs (current_gdbarch) + 30); + alloca_adjust = (unsigned) (frame_addr - (sp + low_word)); if (alloca_adjust > 0) { @@ -1838,14 +1839,15 @@ restart: else if (inst == 0x03A0F021 || inst == 0x03a0f025 || inst == 0x03a0f02d) { /* New gcc frame, virtual frame pointer is at r30 + frame_size. */ - if (frame_reg == MIPS_SP_REGNUM) + if (next_frame && frame_reg == MIPS_SP_REGNUM) { unsigned alloca_adjust; frame_reg = 30; - frame_addr = read_next_frame_reg (next_frame, - gdbarch_num_regs - (current_gdbarch) + 30); + frame_addr = frame_unwind_register_signed + (next_frame, + gdbarch_num_regs (current_gdbarch) + 30); + alloca_adjust = (unsigned) (frame_addr - sp); if (alloca_adjust > 0) { @@ -2153,22 +2155,6 @@ mips_stub_frame_base_sniffer (struct fra return NULL; } -static CORE_ADDR -read_next_frame_reg (struct frame_info *fi, int regno) -{ - /* Always a pseudo. */ - gdb_assert (regno >= gdbarch_num_regs (current_gdbarch)); - if (fi == NULL) - { - LONGEST val; - regcache_cooked_read_signed (current_regcache, regno, &val); - return val; - } - else - return frame_unwind_register_signed (fi, regno); - -} - /* mips_addr_bits_remove - remove useless address bits */ static CORE_ADDR diff -urNp gdb-orig/gdb/sh-tdep.c gdb-head/gdb/sh-tdep.c --- gdb-orig/gdb/sh-tdep.c 2007-06-06 18:58:46.803937873 +0200 +++ gdb-head/gdb/sh-tdep.c 2007-06-06 18:58:57.322020578 +0200 @@ -508,7 +508,7 @@ gdb_print_insn_sh (bfd_vma memaddr, disa static CORE_ADDR sh_analyze_prologue (CORE_ADDR pc, CORE_ADDR current_pc, - struct sh_frame_cache *cache) + struct sh_frame_cache *cache, ULONGEST fpscr) { ULONGEST inst; CORE_ADDR opc; @@ -615,7 +615,7 @@ sh_analyze_prologue (CORE_ADDR pc, CORE_ } else if (IS_FPUSH (inst)) { - if (read_register (FPSCR_REGNUM) & FPSCR_SZ) + if (fpscr & FPSCR_SZ) { cache->sp_offset += 8; } @@ -728,7 +728,7 @@ sh_skip_prologue (CORE_ADDR start_pc) return max (pc, start_pc); cache.sp_offset = -4; - pc = sh_analyze_prologue (start_pc, (CORE_ADDR) -1, &cache); + pc = sh_analyze_prologue (start_pc, (CORE_ADDR) -1, &cache, 0); if (!cache.uses_fp) return start_pc; @@ -2360,7 +2360,11 @@ sh_frame_cache (struct frame_info *next_ cache->pc = frame_func_unwind (next_frame, NORMAL_FRAME); current_pc = frame_pc_unwind (next_frame); if (cache->pc != 0) - sh_analyze_prologue (cache->pc, current_pc, cache); + { + ULONGEST fpscr; + fpscr = frame_unwind_register_unsigned (next_frame, FPSCR_REGNUM); + sh_analyze_prologue (cache->pc, current_pc, cache, fpscr); + } if (!cache->uses_fp) { diff -urNp gdb-orig/gdb/v850-tdep.c gdb-head/gdb/v850-tdep.c --- gdb-orig/gdb/v850-tdep.c 2007-06-04 22:23:28.000000000 +0200 +++ gdb-head/gdb/v850-tdep.c 2007-06-06 18:58:57.364014535 +0200 @@ -456,7 +456,7 @@ v850_is_save_register (int reg) static CORE_ADDR v850_analyze_prologue (CORE_ADDR func_addr, CORE_ADDR pc, - struct v850_frame_cache *pi) + struct v850_frame_cache *pi, ULONGEST ctbp) { CORE_ADDR prologue_end, current_pc; struct pifsr pifsrs[E_NUM_REGS + 1]; @@ -517,7 +517,6 @@ v850_analyze_prologue (CORE_ADDR func_ad } else if ((insn & 0xffc0) == 0x0200 && !regsave_func_p) { /* callt */ - long ctbp = read_register (E_CTBP_REGNUM); long adr = ctbp + ((insn & 0x3f) << 1); save_pc = current_pc; @@ -859,7 +858,11 @@ v850_frame_cache (struct frame_info *nex cache->pc = frame_func_unwind (next_frame, NORMAL_FRAME); current_pc = frame_pc_unwind (next_frame); if (cache->pc != 0) - v850_analyze_prologue (cache->pc, current_pc, cache); + { + ULONGEST ctbp; + ctbp = frame_unwind_register_unsigned (next_frame, E_CTBP_REGNUM); + v850_analyze_prologue (cache->pc, current_pc, cache, ctbp); + } if (!cache->uses_fp) { -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com