From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15950 invoked by alias); 6 Dec 2007 21:41:44 -0000 Received: (qmail 15913 invoked by uid 22791); 6 Dec 2007 21:41:39 -0000 X-Spam-Check-By: sourceware.org Received: from NaN.false.org (HELO nan.false.org) (208.75.86.248) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 06 Dec 2007 21:41:23 +0000 Received: from nan.false.org (localhost [127.0.0.1]) by nan.false.org (Postfix) with ESMTP id E0900982D9 for ; Thu, 6 Dec 2007 21:41:21 +0000 (GMT) Received: from caradoc.them.org (22.svnf5.xdsl.nauticom.net [209.195.183.55]) by nan.false.org (Postfix) with ESMTP id 8F73498150 for ; Thu, 6 Dec 2007 21:41:21 +0000 (GMT) Received: from drow by caradoc.them.org with local (Exim 4.68) (envelope-from ) id 1J0OTA-00087k-DY for gdb-patches@sourceware.org; Thu, 06 Dec 2007 16:41:20 -0500 Date: Fri, 07 Dec 2007 00:28:00 -0000 From: Daniel Jacobowitz To: gdb-patches@sourceware.org Subject: [RFC] PowerPC prologue unwinder / epilogue detection improvements Message-ID: <20071206214120.GA26771@caradoc.them.org> Mail-Followup-To: gdb-patches@sourceware.org MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.15 (2007-04-09) X-IsSubscribed: yes 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-12/txt/msg00112.txt.bz2 A MontaVista customer was trying software watchpoints. They set one on a local variable and then called printf in a loop; this takes us, of course, all the way through the dynamic resolver and then the body of printf itself. If the unwinder becomes confused at any point in this process, and the epilogue check doesn't bail us out, then GDB will delete the out-of-scope watchpoint. This patch fixes up a number of things I found in the PowerPC prologue unwinder in the course of making that work. Most of them are straightforward: - A function can end with a bctr, if it performs a tail call. Recognize this as an epilogue return instruction if it leaves the function. - Just because r30 has been saved doesn't mean r31 has been, even though we know where it will be once the prologue reaches that. GCC often stores the lower numbered registers first. - During the prologue, LR may be saved into r0 and then clobbered by a PIC base operation (bl .+4 or bl GOT-4 for a blrl) before it is eventually saved to the stack. Look in r0 for it during that interval. The patch also contains two things that are not straightforward and I am not going to commit as they are: - Both the oldish MontaVista toolchain their customer was using, and the Debian Sarge toolchain on the test machine I used for the port to HEAD GDB, are built with GCC 3.4. There's an annoying bug in this version of GCC. Given a sequence like this: stwu r1,-48(r1) mflr r0 bl 0x26384 stw r0,52(r1) ... GCC will not report the move of the return address from LR to r0. It leaves the return address column pointing at LR until the end of the prologue, and then updates it to point directly at 52(r1) on the stack. I forcibly detect this case in the patch below and use the prologue unwinder instead of the DWARF-2 unwinder if the prologue contains a bl instruction (-fpic or -fPIC). This is much too heavy-handed. Is it worth working around this bug in GDB? If so, we should find a way to limit it to some likely selection of GCC-generated objects. If not - my inclination is not - I'll simply drop this part of the patch. Anyone who needs it can find it in the list archives. - Also, I wrote a testcase for this situation. Like the original report, it steps into printf. The only way to handle this with software watchpoints is to have symbols or unwind tables for your ld.so / libc.so. If you don't, you won't have symbol addresses; there is no practical way to backtrace from the prologue of a called function without that. I'd sort of like to commit the test. But it's going to fail in lots of targets that are not as careful as I was below, and in cases where the necessary symbol data is not available. I'm thinking of XFAILing it if the top function in the backtrace at a failure is "??". How does that sound? Any comments? Tested on powerpc-linux with no regressions. -- Daniel Jacobowitz CodeSourcery 2007-12-06 Daniel Jacobowitz * rs6000-tdep.c (struct rs6000_framedata): Add gpr_mask, used_bl, lr_register. (rs6000_in_function_epilogue_p): Check for bctr. (skip_prologue): Initialize lr_register. Set lr_reg to a register number. Set gpr_mask and used_bl. Continue scanning while some expected registers are not saved. Set lr_register if LR is not stored. (rs6000_frame_cache): Handle gpr_mask and lr_register. (rs6000_force_frame_sniffer): New. (rs6000_gdbarch_init): Register rs6000_force_frame_sniffer. * gdb.base/watch-lib.c, gdb.base/watch-lib.exp: New files. * gdb.arch/powerpc-prologue.exp: Correct saved registers. Index: rs6000-tdep.c =================================================================== RCS file: /scratch/gcc/repos/src/src/gdb/rs6000-tdep.c,v retrieving revision 1.300 diff -u -p -r1.300 rs6000-tdep.c --- rs6000-tdep.c 19 Nov 2007 05:06:24 -0000 1.300 +++ rs6000-tdep.c 6 Dec 2007 20:09:33 -0000 @@ -118,17 +118,20 @@ struct rs6000_framedata by which we decrement sp to allocate the frame */ int saved_gpr; /* smallest # of saved gpr */ + unsigned int gpr_mask; /* Each bit is an individual saved GPR. */ int saved_fpr; /* smallest # of saved fpr */ int saved_vr; /* smallest # of saved vr */ int saved_ev; /* smallest # of saved ev */ int alloca_reg; /* alloca register number (frame ptr) */ char frameless; /* true if frameless functions. */ char nosavedpc; /* true if pc not saved. */ + char used_bl; /* true if link register clobbered */ int gpr_offset; /* offset of saved gprs from prev sp */ int fpr_offset; /* offset of saved fprs from prev sp */ int vr_offset; /* offset of saved vrs from prev sp */ int ev_offset; /* offset of saved evs from prev sp */ int lr_offset; /* offset of saved lr */ + int lr_register; /* register of saved lr, if trustworthy */ int cr_offset; /* offset of saved cr */ int vrsave_offset; /* offset of saved vrsave register */ }; @@ -838,6 +841,7 @@ insn_changes_sp_or_jumps (unsigned long static int rs6000_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc) { + struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch); bfd_byte insn_buf[PPC_INSN_SIZE]; CORE_ADDR scan_pc, func_start, func_end, epilogue_start, epilogue_end; unsigned long insn; @@ -865,6 +869,17 @@ rs6000_in_function_epilogue_p (struct gd insn = extract_unsigned_integer (insn_buf, PPC_INSN_SIZE); if (insn == 0x4e800020) break; + /* Assume a bctr is a tail call unless it points strictly within + this function. */ + if (insn == 0x4e800420) + { + CORE_ADDR ctr = get_frame_register_unsigned (curfrm, + tdep->ppc_ctr_regnum); + if (ctr > func_start && ctr < func_end) + return 0; + else + break; + } if (insn_changes_sp_or_jumps (insn)) return 0; } @@ -1283,6 +1298,7 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l fdata->alloca_reg = -1; fdata->frameless = 1; fdata->nosavedpc = 1; + fdata->lr_register = -1; for (;; pc += 4) { @@ -1324,7 +1340,7 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l remember just the first one, but skip over additional ones. */ if (lr_reg == -1) - lr_reg = (op & 0x03e00000); + lr_reg = (op & 0x03e00000) >> 21; if (lr_reg == 0) r0_contains_arg = 0; continue; @@ -1355,6 +1371,10 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l { reg = GET_SRC_REG (op); + if ((op & 0xfc1f0000) == 0xbc010000) + fdata->gpr_mask |= ~((1U << reg) - 1); + else + fdata->gpr_mask |= 1U << reg; if (fdata->saved_gpr == -1 || fdata->saved_gpr > reg) { fdata->saved_gpr = reg; @@ -1446,6 +1466,7 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l else if (op == 0x48000005) { /* bl .+4 used in -mrelocatable */ + fdata->used_bl = 1; continue; } @@ -1470,7 +1491,10 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l /* If the return address has already been saved, we can skip calls to blrl (for PIC). */ if (lr_reg != -1 && bl_to_blrl_insn_p (pc, op)) - continue; + { + fdata->used_bl = 1; + continue; + } /* Don't skip over the subroutine call if it is not within the first three instructions of the prologue and either @@ -1496,8 +1520,9 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l if (op == 0x4def7b82 || op == 0) /* crorc 15, 15, 15 */ break; /* don't skip over this branch */ - continue; + fdata->used_bl = 1; + continue; } /* update stack pointer */ else if ((op & 0xfc1f0000) == 0x94010000) @@ -1758,11 +1783,15 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l else { + unsigned int all_mask = ~((1U << fdata->saved_gpr) - 1); + /* Not a recognized prologue instruction. Handle optimizer code motions into the prologue by continuing the search if we have no valid frame yet or if the return - address is not yet saved in the frame. */ - if (fdata->frameless == 0 && fdata->nosavedpc == 0) + address is not yet saved in the frame. Also skip instructions + if some of the GPRs expected to be saved are not yet saved. */ + if (fdata->frameless == 0 && fdata->nosavedpc == 0 + && (fdata->gpr_mask & all_mask) == all_mask) break; if (op == 0x4e800020 /* blr */ @@ -1815,6 +1844,9 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l } #endif /* 0 */ + if (pc == lim_pc && lr_reg >= 0) + fdata->lr_register = lr_reg; + fdata->offset = -fdata->offset; return last_prologue_pc; } @@ -2954,7 +2986,8 @@ rs6000_frame_cache (struct frame_info *n } /* if != -1, fdata.saved_gpr is the smallest number of saved_gpr. - All gpr's from saved_gpr to gpr31 are saved. */ + All gpr's from saved_gpr to gpr31 are saved (except during the + prologue). */ if (fdata.saved_gpr >= 0) { @@ -2962,7 +2995,8 @@ rs6000_frame_cache (struct frame_info *n CORE_ADDR gpr_addr = cache->base + fdata.gpr_offset; for (i = fdata.saved_gpr; i < ppc_num_gprs; i++) { - cache->saved_regs[tdep->ppc_gp0_regnum + i].addr = gpr_addr; + if (fdata.gpr_mask & (1U << i)) + cache->saved_regs[tdep->ppc_gp0_regnum + i].addr = gpr_addr; gpr_addr += wordsize; } } @@ -3009,6 +3043,8 @@ rs6000_frame_cache (struct frame_info *n holds the LR. */ if (fdata.lr_offset != 0) cache->saved_regs[tdep->ppc_lr_regnum].addr = cache->base + fdata.lr_offset; + else if (fdata.lr_register != -1) + cache->saved_regs[tdep->ppc_lr_regnum].realreg = fdata.lr_register; /* The PC is found in the link register. */ cache->saved_regs[gdbarch_pc_regnum (gdbarch)] = cache->saved_regs[tdep->ppc_lr_regnum]; @@ -3066,6 +3102,33 @@ rs6000_frame_sniffer (struct frame_info return &rs6000_frame_unwind; } +static const struct frame_unwind * +rs6000_force_frame_sniffer (struct frame_info *next_frame) +{ + /* HACK: The unwind tables generated by GCC do not account for the + copy of LR to r0 before a PIC bl instruction pointing at a blrl. + We must avoid the FDE in this case if we are between the blrl + and the stack save. */ + CORE_ADDR pc = frame_pc_unwind (next_frame); + struct rs6000_framedata fdata; + CORE_ADDR prologue_end, func_addr; + + func_addr = frame_func_unwind (next_frame, NORMAL_FRAME); + if (func_addr != 0) + { + /* Find an upper limit on the function prologue using the debug + information. If the debug information could not be used to provide + that bound, then use an arbitrary large number as the upper bound. */ + prologue_end = skip_prologue (func_addr, pc, &fdata); + if (fdata.used_bl) + /* (fdata.lr_register != -1 || fdata.lr_offset != 0) + && prologue_end == pc) */ + return &rs6000_frame_unwind; + } + + return NULL; +} + static CORE_ADDR @@ -3690,6 +3753,10 @@ rs6000_gdbarch_init (struct gdbarch_info (gdbarch, rs6000_in_solib_return_trampoline); set_gdbarch_skip_trampoline_code (gdbarch, rs6000_skip_trampoline_code); + /* HACK: Override the DWARF-2 frame sniffer to correct for invalid data + during prologues. */ + frame_unwind_append_sniffer (gdbarch, rs6000_force_frame_sniffer); + /* Hook in the DWARF CFI frame unwinder. */ frame_unwind_append_sniffer (gdbarch, dwarf2_frame_sniffer); dwarf2_frame_set_adjust_regnum (gdbarch, rs6000_adjust_frame_regnum); Index: testsuite/gdb.base/watch-lib.c =================================================================== RCS file: testsuite/gdb.base/watch-lib.c diff -N testsuite/gdb.base/watch-lib.c --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ testsuite/gdb.base/watch-lib.c 6 Dec 2007 19:39:37 -0000 @@ -0,0 +1,30 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2007 + Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +int +main () +{ + char buf[32]; + int a = 0; + + buf[0] = 0; /* set breakpoint here */ + for (a = 0; a < 5; a++) + sprintf (buf, "Message %d", a); + + return 0; +} Index: testsuite/gdb.base/watch-lib.exp =================================================================== RCS file: testsuite/gdb.base/watch-lib.exp diff -N testsuite/gdb.base/watch-lib.exp --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ testsuite/gdb.base/watch-lib.exp 6 Dec 2007 19:53:37 -0000 @@ -0,0 +1,68 @@ +# Copyright 2007 +# Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +set testfile "watch-lib" +set srcfile ${testfile}.c +set binfile ${objdir}/${subdir}/${testfile} + +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } { + untested watch-lib.exp + return -1 +} + +proc test_lib_watchpoint { type } { + global pf_prefix decimal + + set old_pf_prefix $pf_prefix + set pf_prefix "$pf_prefix $type:" + + runto [gdb_get_line_number "set breakpoint here"] + + gdb_test "print a" "\\\$$decimal = 0" + gdb_test "watch a" ".*atchpoint $decimal: a" + gdb_test "continue" "Old value = 0\r\nNew value = 1.*" "first continue" + gdb_test "backtrace" "" "" + gdb_test "continue" "Old value = 1\r\nNew value = 2.*" "second continue" + gdb_test "backtrace" "" "" + + set pf_prefix $old_pf_prefix +} + +# Start with a fresh gdb. + +gdb_exit +gdb_start +gdb_reinitialize_dir $srcdir/$subdir +gdb_load $binfile +set prev_timeout $timeout +set timeout 600 +verbose "Timeout now 600 sec.\n" + +gdb_test "set can-use-hw-watchpoints 0" "" "" +test_lib_watchpoint sw + +if {! [target_info exists gdb,no_hardware_watchpoints]} { + gdb_exit + gdb_start + gdb_reinitialize_dir $srcdir/$subdir + gdb_load $binfile + gdb_test "set can-use-hw-watchpoints 1" "" "" + test_lib_watchpoint hw +} + +# Restore old timeout +set timeout $prev_timeout +verbose "Timeout now $timeout sec.\n" Index: testsuite/gdb.arch/powerpc-prologue.exp =================================================================== RCS file: /scratch/gcc/repos/src/src/gdb/testsuite/gdb.arch/powerpc-prologue.exp,v retrieving revision 1.4 diff -u -p -r1.4 powerpc-prologue.exp --- testsuite/gdb.arch/powerpc-prologue.exp 23 Aug 2007 18:14:16 -0000 1.4 +++ testsuite/gdb.arch/powerpc-prologue.exp 6 Dec 2007 21:39:53 -0000 @@ -84,5 +84,5 @@ gdb_test "backtrace 10" \ "backtrace in optimized" gdb_test "info frame" \ - ".*Saved registers:.*r30 at.*r31 at.*pc at.*lr at.*" \ + ".*Saved registers:.*r30 at.*pc at.*lr at.*" \ "saved registers in optimized"