From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20664 invoked by alias); 22 Sep 2010 19:38:14 -0000 Received: (qmail 20456 invoked by uid 22791); 22 Sep 2010 19:38:12 -0000 X-SWARE-Spam-Status: No, hits=-2.1 required=5.0 tests=AWL,BAYES_00,TW_XB,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 22 Sep 2010 19:38:04 +0000 Received: (qmail 14058 invoked from network); 22 Sep 2010 19:38:02 -0000 Received: from unknown (HELO caradoc.them.org) (dan@127.0.0.2) by mail.codesourcery.com with ESMTPA; 22 Sep 2010 19:38:02 -0000 Date: Wed, 22 Sep 2010 20:01:00 -0000 From: Daniel Jacobowitz To: Ulrich Weigand Cc: gdb-patches@sourceware.org, rearnsha@arm.com Subject: Re: [rfa] Fix software-watchpoint failures by adding epilogue detection Message-ID: <20100922193753.GA25020@caradoc.them.org> Mail-Followup-To: Ulrich Weigand , gdb-patches@sourceware.org, rearnsha@arm.com References: <201009221846.o8MIklfl003240@d12av02.megacenter.de.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201009221846.o8MIklfl003240@d12av02.megacenter.de.ibm.com> User-Agent: Mutt/1.5.20 (2009-06-14) 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: 2010-09/txt/msg00391.txt.bz2 On Wed, Sep 22, 2010 at 08:46:47PM +0200, Ulrich Weigand wrote: > Hello, > > I've been seeing failures in software watchpoint tests on ARM (Thumb-2), > due to missing modeling of precise unwind states during function > epilogues. This problem is well known from other architectures, > and is usually fixed by skipping epilogue during sofware watchpoint > single-stepping via the gdbarch_in_function_epilogue_p callback. > > However, ARM currently does not define this routine. The following > patch adds an implementation that detects common Thumb epilogue > sequences (ARM is missing, but could be added in an analogous way). Hi Ulrich, One of the patches we haven't had a chance to submit took the same approach to solve these failures. I also did ARM mode, as well as some additional sequences - probably those used by RealView. I've attached it. We have at least one other patch related to ARM testsuite failures, specifically an ERROR (probably infinite backtrace) in gdb1250.exp. I can extract it for you if you'd like, but I'm not sure it's in condition to be merged. I haven't retested this patch against HEAD, but it's in all our builds. -- Daniel Jacobowitz CodeSourcery 2010-07-02 Daniel Jacobowitz * arm-tdep.c (thumb_in_function_epilogue_p) (arm_in_function_epilogue_p): New. (thumb_insn_size): Move higher. (arm_gdbarch_init): Install arm_in_function_epilogue_p as gdbarch_in_function_epilogue_p callback. --- /net/henry4/scratch/gcc/src/gdb-mainline/gdb/arm-tdep.c 2010-08-30 10:47:04.000000000 -0700 +++ gdb/arm-tdep.c 2010-09-13 12:32:50.000000000 -0700 @@ -1706,6 +1815,168 @@ arm_dwarf2_frame_init_reg (struct gdbarc } } +/* Return the size in bytes of the complete Thumb instruction whose + first halfword is INST1. */ + +static int +thumb_insn_size (unsigned short inst1) +{ + if ((inst1 & 0xe000) == 0xe000 && (inst1 & 0x1800) != 0) + return 4; + else + return 2; +} + +/* Return 1 if the stack frame has already been destroyed by the + function epilogue. */ + +static int +thumb_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc) +{ + enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch); + unsigned int inst1, inst2; + int found_return, found_stack_adjust; + CORE_ADDR func_start, func_end; + + if (!find_pc_partial_function (pc, NULL, &func_start, &func_end)) + return 0; + + /* We are in the epilogue if the previous instruction was a stack + adjustment and the next instruction is a possible return (bx, mov + pc, or pop). We could have to scan backwards to find the stack + adjustment, or forwards to find the return, but this is a decent + approximation. First scan forwards. */ + + found_return = 0; + inst1 = read_memory_unsigned_integer (pc, 2, byte_order_for_code); + + if ((inst1 & 0xff00) == 0xbc00 && pc + 2 < func_end) + /* POP (without PC). Potentially loading the return address for BX. */ + inst1 = read_memory_unsigned_integer (pc + 2, 2, byte_order_for_code); + + if ((inst1 & 0xff80) == 0x4700) + /* BX lr is typically used for returns. */ + found_return = 1; + else if ((inst1 & 0xff00) == 0xbd00) + /* POP, including PC. */ + found_return = 1; + else if (thumb_insn_size (inst1) == 4) + { + inst2 = read_memory_unsigned_integer (pc, 2, byte_order_for_code); + + if (inst1 == 0xe8bd && (inst2 & 0x8000) == 0x8000) + /* POP.W, including PC. */ + found_return = 1; + else if (inst1 == 0xf85d && inst2 == 0xfb04) + /* POP.W, only PC. */ + found_return = 1; + } + + if (!found_return) + return 0; + + /* Scan backwards. This is just a heuristic, so do not worry about + false positives from mode changes or 16-bit vs 32-bit instructions. */ + + if (pc < func_start + 4) + return 0; + + found_stack_adjust = 0; + inst1 = read_memory_unsigned_integer (pc - 4, 2, byte_order_for_code); + inst2 = read_memory_unsigned_integer (pc - 2, 2, byte_order_for_code); + if ((inst2 & 0xff00) == 0xbc00) + /* POP (without PC). */ + found_stack_adjust = 1; + else if (inst1 == 0xe8bd) + /* POP.W (register list). */ + found_stack_adjust = 1; + else if (inst1 == 0xf85d && (inst2 & 0x0fff) == 0x0b04) + /* POP.W (one register). */ + found_stack_adjust = 1; + else if ((inst2 & 0xff00) == 0xb000) + /* ADD SP, imm or SUB SP, imm. */ + found_stack_adjust = 1; + + /* There are many other stack adjusting instructions which could be + recognized here. */ + + if (found_stack_adjust) + return 1; + + return 0; +} + +/* Return 1 if the stack frame has already been destroyed by the + function epilogue. */ + +static int +arm_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc) +{ + enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch); + unsigned int insn; + int found_return, found_stack_adjust; + CORE_ADDR func_start, func_end; + + if (arm_pc_is_thumb (gdbarch, pc)) + return thumb_in_function_epilogue_p (gdbarch, pc); + + if (!find_pc_partial_function (pc, NULL, &func_start, &func_end)) + return 0; + + /* We are in the epilogue if the previous instruction was a stack + adjustment and the next instruction is a possible return (bx, mov + pc, or pop). We could have to scan backwards to find the stack + adjustment, or forwards to find the return, but this is a decent + approximation. First scan forwards. */ + + found_return = 0; + insn = read_memory_unsigned_integer (pc, 4, byte_order_for_code); + if (bits (insn, 28, 31) != INST_NV) + { + if ((insn & 0x0ffffff0) == 0x012fff10) + /* BX. */ + found_return = 1; + else if ((insn & 0x0ffffff0) == 0x01a0f000) + /* MOV PC. */ + found_return = 1; + else if ((insn & 0x0fff0000) == 0x08bd0000 + && (insn & 0x0000c000) != 0) + /* POP (LDMIA), including PC or LR. */ + found_return = 1; + } + + if (!found_return) + return 0; + + /* Scan backwards. This is just a heuristic, so do not worry about + false positives from mode changes. */ + + if (pc < func_start + 4) + return 0; + + insn = read_memory_unsigned_integer (pc - 4, 4, byte_order_for_code); + if (bits (insn, 28, 31) != INST_NV) + { + if ((insn & 0x0df0f000) == 0x0080d000) + /* ADD SP (register or immediate). */ + found_stack_adjust = 1; + else if ((insn & 0x0df0f000) == 0x0040d000) + /* SUB SP (register or immediate). */ + found_stack_adjust = 1; + else if ((insn & 0x0ffffff0) == 0x01a0d000) + /* MOV SP. */ + found_return = 1; + else if ((insn & 0x0fff0000) == 0x08bd0000) + /* POP (LDMIA). */ + found_stack_adjust = 1; + } + + if (found_stack_adjust) + return 1; + + return 0; +} + /* When arguments must be pushed onto the stack, they go on in reverse order. The code below implements a FILO (stack) to do this. */ @@ -2647,18 +2918,6 @@ bitcount (unsigned long val) return nbits; } -/* Return the size in bytes of the complete Thumb instruction whose - first halfword is INST1. */ - -static int -thumb_insn_size (unsigned short inst1) -{ - if ((inst1 & 0xe000) == 0xe000 && (inst1 & 0x1800) != 0) - return 4; - else - return 2; -} - static int thumb_advance_itstate (unsigned int itstate) { @@ -6881,6 +7160,7 @@ arm_gdbarch_init (struct gdbarch_info in /* Advance PC across function entry code. */ set_gdbarch_skip_prologue (gdbarch, arm_skip_prologue); + set_gdbarch_in_function_epilogue_p (gdbarch, arm_in_function_epilogue_p); /* Skip trampolines. */ set_gdbarch_skip_trampoline_code (gdbarch, arm_skip_stub);