From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16991 invoked by alias); 8 Oct 2010 13:01:28 -0000 Received: (qmail 16974 invoked by uid 22791); 8 Oct 2010 13:01:26 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from cam-admin0.cambridge.arm.com (HELO cam-admin0.cambridge.arm.com) (217.140.96.50) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 08 Oct 2010 13:01:21 +0000 Received: from cam-owa1.Emea.Arm.com (cam-owa1.emea.arm.com [10.1.255.62]) by cam-admin0.cambridge.arm.com (8.12.6/8.12.6) with ESMTP id o98CvSF9000443; Fri, 8 Oct 2010 13:57:28 +0100 (BST) Received: from [10.1.67.34] ([10.1.255.212]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.0); Fri, 8 Oct 2010 14:01:05 +0100 Subject: Re: [rfa, v3] Fix software-watchpoint failures on ARM by adding epilogue detection From: Richard Earnshaw To: Ulrich Weigand Cc: Daniel Jacobowitz , gdb-patches@sourceware.org In-Reply-To: <201010071612.o97GCDt7024334@d12av02.megacenter.de.ibm.com> References: <201010071612.o97GCDt7024334@d12av02.megacenter.de.ibm.com> Content-Type: text/plain Date: Fri, 08 Oct 2010 13:01:00 -0000 Message-Id: <1286542864.29651.13.camel@e102346-lin.cambridge.arm.com> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit 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: 2010-10/txt/msg00133.txt.bz2 On Thu, 2010-10-07 at 18:12 +0200, Ulrich Weigand wrote: > Daniel Jacobowitz wrote: > > On Tue, Sep 28, 2010 at 06:04:14PM +0200, Ulrich Weigand wrote: > > > I'm wondering how "bx lr" could be an indirect call; for a call, > > > lr would have to point to the return address, so it couldn't also > > > contain the target address ... Am I missing something here? > > > > Bah, you are correct. Poor choice of example. bx ip is a better > > example; that can be an indirect call, a return, or a tail call. > > > > > As far as I can see, GCC never uses bx with any other register but > > > lr to implement a return instruction. Do you know whether this is > > > also true for other compilers? If so, maybe the easiest fix would > > > be to change this back to only accepting "bx lr". > > > > Sorry, I don't know :-( Does GCC also only use lr for an indirect > > tail call? I can't tell - I couldn't get GCC to issue an indirect > > tail call. > > OK, so it looks like we do have to add a backward scanning pass in order > to reliably distinguish whether a BX is an indirect call or an indirect > tail call (which GCC doesn't issue today, but probably could in the > future, and RealView already does). > > Fortunately, this is relatively straightforward, since we at most need > to scan back one instruction. *Every* instruction in the default > epilogue sequence modifies SP, except possibly the return itself. > Therefore, we either found some instruction(s) modifying SP during > the forward-scanning pass before hitting the return, or else we > check back one instruction. > > The following patch implements this approach for Thumb (the ARM part > remains unchanged from your version). > > Re-tested (with the same results) on armv7l-linux-gnueabi. > > Richard, would this be OK with you? > > Bye, > Ulrich > > > 2010-10-07 Ulrich Weigand > Daniel Jacobowitz > > * arm-tdep.c (thumb_in_function_epilogue_p) > (arm_in_function_epilogue_p): New. > (arm_gdbarch_init): Install arm_in_function_epilogue_p as > gdbarch_in_function_epilogue_p callback. > This is OK. There are some sequences that could be generated that this can't handle, and I don't think we'd ever get a stack decrement in an epilogue sequence. An example I don't think your code can handle is stack adjustments that are not valid immediates for ADD SP, #n. So very large stack adjusts my fail. These sequences will load or create an immediate in a register (particularly on Thumb1) and then add that to SP. R. > > Index: gdb/arm-tdep.c > =================================================================== > RCS file: /cvs/src/src/gdb/arm-tdep.c,v > retrieving revision 1.307 > diff -u -p -r1.307 arm-tdep.c > --- gdb/arm-tdep.c 30 Aug 2010 15:26:28 -0000 1.307 > +++ gdb/arm-tdep.c 7 Oct 2010 11:55:02 -0000 > @@ -1706,6 +1706,203 @@ arm_dwarf2_frame_init_reg (struct gdbarc > } > } > > +/* Return true if we are in the function's epilogue, i.e. after the > + instruction that destroyed the function's stack frame. */ > + > +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 insn, insn2; > + int found_return = 0, found_stack_adjust = 0; > + CORE_ADDR func_start, func_end; > + CORE_ADDR scan_pc; > + gdb_byte buf[4]; > + > + if (!find_pc_partial_function (pc, NULL, &func_start, &func_end)) > + return 0; > + > + /* The epilogue is a sequence of instructions along the following lines: > + > + - add stack frame size to SP or FP > + - [if frame pointer used] restore SP from FP > + - restore registers from SP [may include PC] > + - a return-type instruction [if PC wasn't already restored] > + > + In a first pass, we scan forward from the current PC and verify the > + instructions we find as compatible with this sequence, ending in a > + return instruction. > + > + However, this is not sufficient to distinguish indirect function calls > + within a function from indirect tail calls in the epilogue in some cases. > + Therefore, if we didn't already find any SP-changing instruction during > + forward scan, we add a backward scanning heuristic to ensure we actually > + are in the epilogue. */ > + > + scan_pc = pc; > + while (scan_pc < func_end && !found_return) > + { > + if (target_read_memory (scan_pc, buf, 2)) > + break; > + > + scan_pc += 2; > + insn = extract_unsigned_integer (buf, 2, byte_order_for_code); > + > + if ((insn & 0xff80) == 0x4700) /* bx */ > + found_return = 1; > + else if (insn == 0x46f7) /* mov pc, lr */ > + found_return = 1; > + else if (insn == 0x46bd) /* mov sp, r7 */ > + found_stack_adjust = 1; > + else if ((insn & 0xff00) == 0xb000) /* add sp, imm or sub sp, imm */ > + found_stack_adjust = 1; > + else if ((insn & 0xfe00) == 0xbc00) /* pop */ > + { > + found_stack_adjust = 1; > + if (insn & 0x0100) /* include PC. */ > + found_return = 1; > + } > + else if ((insn & 0xe000) == 0xe000) /* 32-bit Thumb-2 instruction */ > + { > + if (target_read_memory (scan_pc, buf, 2)) > + break; > + > + scan_pc += 2; > + insn2 = extract_unsigned_integer (buf, 2, byte_order_for_code); > + > + if (insn == 0xe8bd) /* ldm.w sp!, */ > + { > + found_stack_adjust = 1; > + if (insn2 & 0x8000) /* include PC. */ > + found_return = 1; > + } > + else if (insn == 0xf85d /* ldr.w , [sp], #4 */ > + && (insn2 & 0x0fff) == 0x0b04) > + { > + found_stack_adjust = 1; > + if ((insn2 & 0xf000) == 0xf000) /* is PC. */ > + found_return = 1; > + } > + else if ((insn & 0xffbf) == 0xecbd /* vldm sp!, */ > + && (insn2 & 0x0e00) == 0x0a00) > + found_stack_adjust = 1; > + else > + break; > + } > + else > + break; > + } > + > + if (!found_return) > + return 0; > + > + /* Since any instruction in the epilogue sequence, with the possible > + exception of return itself, updates the stack pointer, we need to > + scan backwards for at most one instruction. Try either a 16-bit or > + a 32-bit instruction. This is just a heuristic, so we do not worry > + too much about false positives.*/ > + > + if (!found_stack_adjust) > + { > + if (pc - 4 < func_start) > + return 0; > + if (target_read_memory (pc - 4, buf, 4)) > + return 0; > + > + insn = extract_unsigned_integer (buf, 2, byte_order_for_code); > + insn2 = extract_unsigned_integer (buf + 2, 2, byte_order_for_code); > + > + if (insn2 == 0x46bd) /* mov sp, r7 */ > + found_stack_adjust = 1; > + else if ((insn2 & 0xff00) == 0xb000) /* add sp, imm or sub sp, imm */ > + found_stack_adjust = 1; > + else if ((insn2 & 0xff00) == 0xbc00) /* pop without PC */ > + found_stack_adjust = 1; > + else if (insn == 0xe8bd) /* ldm.w sp!, */ > + found_stack_adjust = 1; > + else if (insn == 0xf85d /* ldr.w , [sp], #4 */ > + && (insn2 & 0x0fff) == 0x0b04) > + found_stack_adjust = 1; > + else if ((insn & 0xffbf) == 0xecbd /* vldm sp!, */ > + && (insn2 & 0x0e00) == 0x0a00) > + found_stack_adjust = 1; > + } > + > + return found_stack_adjust; > +} > + > +/* Return true if we are in the function's epilogue, i.e. after the > + instruction that destroyed the function's stack frame. */ > + > +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. */ > > @@ -6881,6 +7078,9 @@ arm_gdbarch_init (struct gdbarch_info in > /* Advance PC across function entry code. */ > set_gdbarch_skip_prologue (gdbarch, arm_skip_prologue); > > + /* Detect whether PC is in function epilogue. */ > + set_gdbarch_in_function_epilogue_p (gdbarch, arm_in_function_epilogue_p); > + > /* Skip trampolines. */ > set_gdbarch_skip_trampoline_code (gdbarch, arm_skip_stub); > >