From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12720 invoked by alias); 21 Nov 2006 19:15:24 -0000 Received: (qmail 12696 invoked by uid 22791); 21 Nov 2006 19:15:21 -0000 X-Spam-Check-By: sourceware.org Received: from nevyn.them.org (HELO nevyn.them.org) (66.93.172.17) by sourceware.org (qpsmtpd/0.31.1) with ESMTP; Tue, 21 Nov 2006 19:15:11 +0000 Received: from drow by nevyn.them.org with local (Exim 4.63) (envelope-from ) id 1Gmb5I-0000fO-20; Tue, 21 Nov 2006 14:15:08 -0500 Date: Tue, 21 Nov 2006 19:15:00 -0000 From: Daniel Jacobowitz To: gdb-patches@sourceware.org Cc: Richard Earnshaw Subject: [rfa/arm] Improve Thumb prologue analysis Message-ID: <20061121191508.GA2056@nevyn.them.org> Mail-Followup-To: gdb-patches@sourceware.org, Richard Earnshaw MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.13 (2006-08-11) 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: 2006-11/txt/msg00229.txt.bz2 This patch rewrites the Thumb prologue analyzer to use prologue-value.c and be overall more robust. It also moves the call to the Thumb-specific code a little later in arm_skip_prologue, so that it isn't mistakenly called with func_end == 0. The new one is stricter about unknown instructions. That prevents it from hopping over branches; in the new test, not even "break main" works, because the breakpoint gets placed too late. It knows one important instruction that the previous version didn't: stores, as used in -mthumb -mtpcs-frame, which is where this bug was originally reported to me. The patch fixes a backtrace from abort in gdb1250.exp. With DWARF support turned off, it also fixes recurse.exp; in fact, with the patch applied, GDB testsuite results are the same with or without DWARF unwinding. I presume the new test will misbehave on a target which has no Thumb support; I don't have one handy, but once someone encounters that failure it should be easy to detect it and skip the test. We'll presumably get a SIGILL. OK? -- Daniel Jacobowitz CodeSourcery 2006-11-21 Daniel Jacobowitz * Makefile.in (arm-tdep.o): Update dependencies. * arm-tdep.c (thumb_skip_prologue): Remove. (thumb_analyze_prologue): New function. (arm_skip_prologue): Use thumb_analyze_prologue. (thumb_scan_prologue): Ditto. 2006-11-21 Daniel Jacobowitz * gdb.arch/thumb-prologue.c, gdb.arch/thumb-prologue.exp: New files. Index: Makefile.in =================================================================== RCS file: /scratch/gcc/repos/src/src/gdb/Makefile.in,v retrieving revision 1.848 diff -u -p -r1.848 Makefile.in --- Makefile.in 14 Nov 2006 21:53:59 -0000 1.848 +++ Makefile.in 21 Nov 2006 15:45:43 -0000 @@ -1805,7 +1805,7 @@ arm-tdep.o: arm-tdep.c $(defs_h) $(frame $(frame_unwind_h) $(frame_base_h) $(trad_frame_h) $(arm_tdep_h) \ $(gdb_sim_arm_h) $(elf_bfd_h) $(coff_internal_h) $(elf_arm_h) \ $(gdb_assert_h) $(bfd_in2_h) $(libcoff_h) $(objfiles_h) \ - $(dwarf2_frame_h) $(gdbtypes_h) + $(dwarf2_frame_h) $(gdbtypes_h) $(prologue_value_h) auxv.o: auxv.c $(defs_h) $(target_h) $(gdbtypes_h) $(command_h) \ $(inferior_h) $(valprint_h) $(gdb_assert_h) $(auxv_h) \ $(elf_common_h) Index: arm-tdep.c =================================================================== RCS file: /scratch/gcc/repos/src/src/gdb/arm-tdep.c,v retrieving revision 1.216 diff -u -p -r1.216 arm-tdep.c --- arm-tdep.c 12 Nov 2006 11:06:31 -0000 1.216 +++ arm-tdep.c 21 Nov 2006 18:54:30 -0000 @@ -41,6 +41,7 @@ #include "objfiles.h" #include "dwarf2-frame.h" #include "gdbtypes.h" +#include "prologue-value.h" #include "arm-tdep.h" #include "gdb/sim-arm.h" @@ -212,84 +213,156 @@ arm_smash_text_address (CORE_ADDR val) return val & ~1; } -/* A typical Thumb prologue looks like this: - push {r7, lr} - add sp, sp, #-28 - add r7, sp, #12 - Sometimes the latter instruction may be replaced by: - mov r7, sp - - or like this: - push {r7, lr} - mov r7, sp - sub sp, #12 - - or, on tpcs, like this: - sub sp,#16 - push {r7, lr} - (many instructions) - mov r7, sp - sub sp, #12 - - There is always one instruction of three classes: - 1 - push - 2 - setting of r7 - 3 - adjusting of sp - - When we have found at least one of each class we are done with the prolog. - Note that the "sub sp, #NN" before the push does not count. - */ +/* Analyze a Thumb prologue, looking for a recognizable stack frame + and frame pointer. Scan until we encounter a store that could + clobber the stack frame unexpectedly, or an unknown instruction. */ static CORE_ADDR -thumb_skip_prologue (CORE_ADDR pc, CORE_ADDR func_end) +thumb_analyze_prologue (struct gdbarch *gdbarch, + CORE_ADDR start, CORE_ADDR limit, + struct arm_prologue_cache *cache) { - CORE_ADDR current_pc; - /* findmask: - bit 0 - push { rlist } - bit 1 - mov r7, sp OR add r7, sp, #imm (setting of r7) - bit 2 - sub sp, #simm OR add sp, #simm (adjusting of sp) - */ - int findmask = 0; + int i; + pv_t regs[16]; + struct pv_area *stack; + struct cleanup *back_to; + CORE_ADDR offset; + + for (i = 0; i < 16; i++) + regs[i] = pv_register (i, 0); + stack = make_pv_area (ARM_SP_REGNUM); + back_to = make_cleanup_free_pv_area (stack); + + /* The call instruction saved PC in LR, and the current PC is not + interesting. Due to this file's conventions, we want the value + of LR at this function's entry, not at the call site, so we do + not record the save of the PC - when the ARM prologue analyzer + has also been converted to the pv mechanism, we could record the + save here and remove the hack in prev_register. */ + regs[ARM_PC_REGNUM] = pv_unknown (); - for (current_pc = pc; - current_pc + 2 < func_end && current_pc < pc + 40; - current_pc += 2) + while (start < limit) { - unsigned short insn = read_memory_unsigned_integer (current_pc, 2); + unsigned short insn; + + insn = read_memory_unsigned_integer (start, 2); if ((insn & 0xfe00) == 0xb400) /* push { rlist } */ { - findmask |= 1; /* push found */ + int regno; + int mask; + int stop = 0; + + /* Bits 0-7 contain a mask for registers R0-R7. Bit 8 says + whether to save LR (R14). */ + mask = (insn & 0xff) | ((insn & 0x100) << 6); + + /* Calculate offsets of saved R0-R7 and LR. */ + for (regno = ARM_LR_REGNUM; regno >= 0; regno--) + if (mask & (1 << regno)) + { + if (pv_area_store_would_trash (stack, regs[ARM_SP_REGNUM])) + { + stop = 1; + break; + } + + regs[ARM_SP_REGNUM] = pv_add_constant (regs[ARM_SP_REGNUM], + -4); + pv_area_store (stack, regs[ARM_SP_REGNUM], 4, regs[regno]); + } + + if (stop) + break; } else if ((insn & 0xff00) == 0xb000) /* add sp, #simm OR sub sp, #simm */ { - if ((findmask & 1) == 0) /* before push ? */ - continue; + offset = (insn & 0x7f) << 2; /* get scaled offset */ + if (insn & 0x80) /* Check for SUB. */ + regs[ARM_SP_REGNUM] = pv_add_constant (regs[ARM_SP_REGNUM], + -offset); else - findmask |= 4; /* add/sub sp found */ + regs[ARM_SP_REGNUM] = pv_add_constant (regs[ARM_SP_REGNUM], + offset); } else if ((insn & 0xff00) == 0xaf00) /* add r7, sp, #imm */ - { - findmask |= 2; /* setting of r7 found */ - } - else if (insn == 0x466f) /* mov r7, sp */ - { - findmask |= 2; /* setting of r7 found */ + regs[THUMB_FP_REGNUM] = pv_add_constant (regs[ARM_SP_REGNUM], + (insn & 0xff) << 2); + else if ((insn & 0xff00) == 0x4600) /* mov hi, lo or mov lo, hi */ + { + int dst_reg = (insn & 0x7) + ((insn & 0x80) >> 4); + int src_reg = (insn & 0x78) >> 3; + regs[dst_reg] = regs[src_reg]; + } + else if ((insn & 0xf800) == 0x9000) /* str rd, [sp, #off] */ + { + /* Handle stores to the stack. Normally pushes are used, + but with GCC -mtpcs-frame, there may be other stores + in the prologue to create the frame. */ + int regno = (insn >> 8) & 0x7; + pv_t addr; + + offset = (insn & 0xff) << 2; + addr = pv_add_constant (regs[ARM_SP_REGNUM], offset); + + if (pv_area_store_would_trash (stack, addr)) + break; + + pv_area_store (stack, addr, 4, regs[regno]); } - else if (findmask == (4+2+1)) + else { - /* We have found one of each type of prologue instruction */ + /* We don't know what this instruction is. We're finished + scanning. NOTE: Recognizing more safe-to-ignore + instructions here will improve support for optimized + code. */ break; } - else - /* Something in the prolog that we don't care about or some - instruction from outside the prolog scheduled here for - optimization. */ - continue; + + start += 2; + } + + if (cache == NULL) + { + do_cleanups (back_to); + return start; } - return current_pc; + /* frameoffset is unused for this unwinder. */ + cache->frameoffset = 0; + + if (pv_is_register (regs[ARM_FP_REGNUM], ARM_SP_REGNUM)) + { + /* Frame pointer is fp. Frame size is constant. */ + cache->framereg = ARM_FP_REGNUM; + cache->framesize = -regs[ARM_FP_REGNUM].k; + } + else if (pv_is_register (regs[THUMB_FP_REGNUM], ARM_SP_REGNUM)) + { + /* Frame pointer is r7. Frame size is constant. */ + cache->framereg = THUMB_FP_REGNUM; + cache->framesize = -regs[THUMB_FP_REGNUM].k; + } + else if (pv_is_register (regs[ARM_SP_REGNUM], ARM_SP_REGNUM)) + { + /* Try the stack pointer... this is a bit desperate. */ + cache->framereg = ARM_SP_REGNUM; + cache->framesize = -regs[ARM_SP_REGNUM].k; + } + else + { + /* We're just out of luck. We don't know where the frame is. */ + cache->framereg = -1; + cache->framesize = 0; + } + + for (i = 0; i < 16; i++) + if (pv_area_find_reg (stack, gdbarch, i, &offset)) + cache->saved_regs[i].addr = offset; + + do_cleanups (back_to); + return start; } /* Advance the PC across any function entry prologue instructions to @@ -337,10 +410,6 @@ arm_skip_prologue (CORE_ADDR pc) } } - /* Check if this is Thumb code. */ - if (arm_pc_is_thumb (pc)) - return thumb_skip_prologue (pc, func_end); - /* Can't find the prologue end in the symbol table, try it the hard way by disassembling the instructions. */ @@ -348,6 +417,10 @@ arm_skip_prologue (CORE_ADDR pc) if (func_end == 0 || func_end > pc + 64) func_end = pc + 64; + /* Check if this is Thumb code. */ + if (arm_pc_is_thumb (pc)) + return thumb_analyze_prologue (current_gdbarch, pc, func_end, NULL); + for (skip_pc = pc; skip_pc < func_end; skip_pc += 4) { inst = read_memory_unsigned_integer (skip_pc, 4); @@ -462,86 +535,8 @@ thumb_scan_prologue (CORE_ADDR prev_pc, prologue_end = min (prologue_end, prev_pc); - /* Initialize the saved register map. When register H is copied to - register L, we will put H in saved_reg[L]. */ - for (i = 0; i < 16; i++) - saved_reg[i] = i; - - /* Search the prologue looking for instructions that set up the - frame pointer, adjust the stack pointer, and save registers. - Do this until all basic prolog instructions are found. */ - - cache->framesize = 0; - for (current_pc = prologue_start; - (current_pc < prologue_end) && ((findmask & 7) != 7); - current_pc += 2) - { - unsigned short insn; - int regno; - int offset; - - insn = read_memory_unsigned_integer (current_pc, 2); - - if ((insn & 0xfe00) == 0xb400) /* push { rlist } */ - { - int mask; - findmask |= 1; /* push found */ - /* Bits 0-7 contain a mask for registers R0-R7. Bit 8 says - whether to save LR (R14). */ - mask = (insn & 0xff) | ((insn & 0x100) << 6); - - /* Calculate offsets of saved R0-R7 and LR. */ - for (regno = ARM_LR_REGNUM; regno >= 0; regno--) - if (mask & (1 << regno)) - { - cache->framesize += 4; - cache->saved_regs[saved_reg[regno]].addr = -cache->framesize; - /* Reset saved register map. */ - saved_reg[regno] = regno; - } - } - else if ((insn & 0xff00) == 0xb000) /* add sp, #simm OR - sub sp, #simm */ - { - if ((findmask & 1) == 0) /* before push? */ - continue; - else - findmask |= 4; /* add/sub sp found */ - - offset = (insn & 0x7f) << 2; /* get scaled offset */ - if (insn & 0x80) /* is it signed? (==subtracting) */ - { - cache->frameoffset += offset; - offset = -offset; - } - cache->framesize -= offset; - } - else if ((insn & 0xff00) == 0xaf00) /* add r7, sp, #imm */ - { - findmask |= 2; /* setting of r7 found */ - cache->framereg = THUMB_FP_REGNUM; - /* get scaled offset */ - cache->frameoffset = (insn & 0xff) << 2; - } - else if (insn == 0x466f) /* mov r7, sp */ - { - findmask |= 2; /* setting of r7 found */ - cache->framereg = THUMB_FP_REGNUM; - cache->frameoffset = 0; - saved_reg[THUMB_FP_REGNUM] = ARM_SP_REGNUM; - } - else if ((insn & 0xffc0) == 0x4640) /* mov r0-r7, r8-r15 */ - { - int lo_reg = insn & 7; /* dest. register (r0-r7) */ - int hi_reg = ((insn >> 3) & 7) + 8; /* source register (r8-15) */ - saved_reg[lo_reg] = hi_reg; /* remember hi reg was saved */ - } - else - /* Something in the prolog that we don't care about or some - instruction from outside the prolog scheduled here for - optimization. */ - continue; - } + thumb_analyze_prologue (current_gdbarch, prologue_start, prologue_end, + cache); } /* This function decodes an ARM function prologue to determine: Index: testsuite/gdb.arch/thumb-prologue.c =================================================================== RCS file: testsuite/gdb.arch/thumb-prologue.c diff -N testsuite/gdb.arch/thumb-prologue.c --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ testsuite/gdb.arch/thumb-prologue.c 21 Nov 2006 15:45:15 -0000 @@ -0,0 +1,96 @@ +/* Unwinder test program. + + Copyright 2006 Free Software Foundation, Inc. + + This file is part of GDB. + + 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 2 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, write to the Free Software + Foundation, Inc., 59 Temple Place - Suite 330, + Boston, MA 02111-1307, USA. */ + +void tpcs_frame (void); + +int +main (void) +{ + tpcs_frame (); + return 0; +} + +/* Normally Thumb functions use r7 as the frame pointer. However, + with the GCC option -mtpcs-frame, they may use fp instead. */ + +asm(".text\n" + " .align 2\n" + " .thumb_func\n" + " .code 16\n" + "tpcs_frame_1:\n" + " sub sp, #16\n" + " push {r7}\n" + " add r7, sp, #20\n" + " str r7, [sp, #8]\n" + " mov r7, pc\n" + " str r7, [sp, #16]\n" + " mov r7, fp\n" + " str r7, [sp, #4]\n" + " mov r7, lr\n" + " str r7, [sp, #12]\n" + " add r7, sp, #16\n" + " mov fp, r7\n" + " mov r7, sl\n" + " push {r7}\n" + + /* Trap. */ + " .short 0xdffe\n" + + " pop {r2}\n" + " mov sl, r2\n" + " pop {r7}\n" + " pop {r1, r2}\n" + " mov fp, r1\n" + " mov sp, r2\n" + " bx lr\n" + + " .align 2\n" + " .thumb_func\n" + " .code 16\n" + "tpcs_frame:\n" + " sub sp, #16\n" + " push {r7}\n" + " add r7, sp, #20\n" + " str r7, [sp, #8]\n" + " mov r7, pc\n" + " str r7, [sp, #16]\n" + " mov r7, fp\n" + " str r7, [sp, #4]\n" + " mov r7, lr\n" + " str r7, [sp, #12]\n" + " add r7, sp, #16\n" + " mov fp, r7\n" + " mov r7, sl\n" + " push {r7}\n" + + /* Clobber saved regs. */ + " mov r7, #0\n" + " mov lr, r7\n" + " bl tpcs_frame_1\n" + + " pop {r2}\n" + " mov sl, r2\n" + " pop {r7}\n" + " pop {r1, r2}\n" + " mov fp, r1\n" + " mov sp, r2\n" + " bx lr\n" +); Index: testsuite/gdb.arch/thumb-prologue.exp =================================================================== RCS file: testsuite/gdb.arch/thumb-prologue.exp diff -N testsuite/gdb.arch/thumb-prologue.exp --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ testsuite/gdb.arch/thumb-prologue.exp 21 Nov 2006 18:41:27 -0000 @@ -0,0 +1,60 @@ +# Copyright 2006 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 2 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, write to the Free Software +# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. + +# Test ARM/Thumb prologue analyzer. + +if {![istarget arm*-*]} then { + verbose "Skipping ARM prologue tests." + return +} + +set testfile "thumb-prologue" +set srcfile ${testfile}.c +set binfile ${objdir}/${subdir}/${testfile} + +# Don't use "debug", so that we don't have line information for the assembly +# fragments. +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {"additional_flags=-mthumb"}] != "" } { + untested "ARM prologue tests" + return -1 +} + + +gdb_exit +gdb_start +gdb_reinitialize_dir $srcdir/$subdir +gdb_load ${binfile} + +# +# Run to `main' where we begin our tests. +# + +if ![runto_main] then { + untested "ARM prologue tests" + return -1 +} + +# Testcase for TPCS prologue. + +gdb_test "continue" "Program received signal SIG.*" "continue to TPCS" + +gdb_test "backtrace 10" \ + "#0\[ \t\]*$hex in tpcs_frame_1 .*\r\n#1\[ \t\]*$hex in tpcs_frame .*\r\n#2\[ \t\]*$hex in main.*" \ + "backtrace in TPCS" + +gdb_test "info frame" \ + ".*Saved registers:.*r7 at.*r10 at.*r11 at.*lr at.*pc at .*" \ + "saved registers in TPCS"