* [rfa] Fix software-watchpoint failures by adding epilogue detection
@ 2010-09-22 19:20 Ulrich Weigand
2010-09-22 20:01 ` Daniel Jacobowitz
0 siblings, 1 reply; 12+ messages in thread
From: Ulrich Weigand @ 2010-09-22 19:20 UTC (permalink / raw)
To: gdb-patches, rearnsha
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).
This fixes the following test case failures:
FAIL: gdb.base/recurse.exp: continue to second instance watchpoint, second time
FAIL: gdb.base/recurse.exp: second instance watchpoint deleted when leaving scope (the program exited)
FAIL: gdb.base/recurse.exp: continue to first instance watchpoint, second time (the program is no longer running)
FAIL: gdb.base/recurse.exp: first instance watchpoint deleted when leaving scope (the program is no longer running)
FAIL: gdb.base/watch-cond.exp: watchpoint with local expression, local condition evaluates in correct frame
and adds:
XPASS: gdb.mi/mi-watch.exp: sw: watchpoint trigger
XPASS: gdb.mi/mi2-watch.exp: sw: watchpoint trigger
Tested on armv7l-linux-gnueabi with no regressions.
OK for mainline?
Bye,
Ulrich
ChangeLog:
* arm-tdep.c (arm_in_function_epilogue_p): New function.
(arm_gdbarch_init): Install it.
=== modified file 'gdb/arm-tdep.c'
--- gdb/arm-tdep.c 2010-08-30 15:33:03 +0000
+++ gdb/arm-tdep.c 2010-09-22 17:07:18 +0000
@@ -1686,6 +1686,87 @@
}
}
+/* Return true if we are in the function's epilogue, i.e. after the
+ instruction that destroyed the function's stack frame.
+
+ We consider this to be the case if, starting from the current
+ instruction, we have a sequence of:
+
+ - [optional] setting SP from the frame pointer
+ - restoring registers from SP [may include PC]
+ - a return-type instruction [if PC wasn't already restored]
+
+ If we find anything else, we assume we're not in the 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);
+
+ if (arm_pc_is_thumb (pc))
+ {
+ for (;;)
+ {
+ unsigned int insn;
+ gdb_byte buf[2];
+
+ if (target_read_memory (pc, buf, 2))
+ return 0;
+
+ pc += 2;
+ insn = extract_unsigned_integer (buf, 2, byte_order_for_code);
+
+ if (insn == 0x4770) /* bx lr */
+ return 1;
+ else if (insn == 0x46f7) /* mov pc, lr */
+ return 1;
+ else if (insn == 0x46bd) /* mov sp, r7 */
+ ;
+ else if ((insn & 0xfe00) == 0xbc00) /* pop <registers> */
+ {
+ if (insn & 0x0100) /* <registers> include PC. */
+ return 1;
+ }
+ else if ((insn & 0xe000) == 0xe000) /* 32-bit Thumb-2 instruction */
+ {
+ unsigned int insn2;
+
+ if (target_read_memory (pc, buf, 2))
+ return 0;
+
+ pc += 2;
+ insn2 = extract_unsigned_integer (buf, 2, byte_order_for_code);
+
+ if ((insn & 0xffdf) == 0xe89d) /* ldm.w sp{!}, <registers> */
+ {
+ if (insn2 & 0x8000) /* <registers> include PC. */
+ return 1;
+ }
+ else if (insn == 0xf85d /* ldr.w <Rt>, [sp], #4 */
+ && (insn2 & 0x0fff) == 0x0d04)
+ {
+ if ((insn2 & 0xf000) == 0xf000) /* <Rt> is PC. */
+ return 1;
+ }
+ else if ((insn & 0xffbf) == 0xecbd /* vldm sp!, <list> */
+ && (insn2 & 0x0e00) == 0x0a00)
+ ;
+ else
+ return 0;
+ }
+ else
+ return 0;
+ }
+ }
+ else
+ {
+ /* FIXME: We don't handle ARM for now. */
+ }
+
+ 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. */
@@ -6818,6 +6899,9 @@
/* 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);
--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [rfa] Fix software-watchpoint failures by adding epilogue detection 2010-09-22 19:20 [rfa] Fix software-watchpoint failures by adding epilogue detection Ulrich Weigand @ 2010-09-22 20:01 ` Daniel Jacobowitz 2010-09-23 16:13 ` Ulrich Weigand 0 siblings, 1 reply; 12+ messages in thread From: Daniel Jacobowitz @ 2010-09-22 20:01 UTC (permalink / raw) To: Ulrich Weigand; +Cc: gdb-patches, rearnsha 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 <dan@codesourcery.com> * 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); ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [rfa] Fix software-watchpoint failures by adding epilogue detection 2010-09-22 20:01 ` Daniel Jacobowitz @ 2010-09-23 16:13 ` Ulrich Weigand 2010-09-24 16:11 ` Ulrich Weigand 0 siblings, 1 reply; 12+ messages in thread From: Ulrich Weigand @ 2010-09-23 16:13 UTC (permalink / raw) To: Daniel Jacobowitz; +Cc: gdb-patches, rearnsha Dan Jacobowitz wrote: > 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. Thanks! Unfortunately it doesn't fix the problem with our current ARM compiler, because your heuristics doesn't recognize the sequence we get as epilogue: 83c8: f107 0710 add.w r7, r7, #16 83cc: 46bd mov sp, r7 83ce: bd80 pop {r7, pc} On the other hand, your patch does indeed accept a couple of additonal instructions compared to mine. I'll come up with a merged approach ... > 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. Hmm, this test is passing now. We did have problems with backtraces out of abort, but this was due to a compiler problem -- it simply would not save the return address at all for no-return functions. This is now fixed in the compiler ... Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [rfa] Fix software-watchpoint failures by adding epilogue detection 2010-09-23 16:13 ` Ulrich Weigand @ 2010-09-24 16:11 ` Ulrich Weigand 2010-09-28 20:23 ` Daniel Jacobowitz 0 siblings, 1 reply; 12+ messages in thread From: Ulrich Weigand @ 2010-09-24 16:11 UTC (permalink / raw) To: Daniel Jacobowitz, gdb-patches, rearnsha I wrote: > Dan Jacobowitz wrote: > > > 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. > > Thanks! Unfortunately it doesn't fix the problem with our current > ARM compiler, because your heuristics doesn't recognize the sequence > we get as epilogue: > > 83c8: f107 0710 add.w r7, r7, #16 > 83cc: 46bd mov sp, r7 > 83ce: bd80 pop {r7, pc} > > On the other hand, your patch does indeed accept a couple of additonal > instructions compared to mine. I'll come up with a merged approach ... I've compared the Thumb cases of both patches with the following results: 1. Some epilogue sequences accepted by your patch were not accepted by mine: - My patch only accepted "bx lr" as return, while yours accepts any "bx". - I had a typo in one of the instruction opcodes. 2. Some epilogue sequences accepted by my patch were not accepted by yours: - I'm allowing "mov sp, r7" and "vldm" instructions, as well as certain additional cases of "ldm.w". - I'm accepting more diverse sequences due to forward-scanning for multiple instructions, and not requiring backward-scanning. I've now merged the two by basically keeping with the main structure of my patch for Thumb, and fixing the two issues identified under 1. For ARM, I'm simply using your patch as-is; I've confirmed that it fixes all the same testsuite failures when running with -marm as the Thumb patch fixes with -mthumb. The more limited scanning doesn't appear to hurt for ARM as the epilogues tend to be very short anyway. Retested with no regressions on armv7l-linux-gnueabi both with -marm and with (implied) -mthumb. Does this look good to you? Bye, Ulrich 2010-09-24 Ulrich Weigand <uweigand@de.ibm.com> Daniel Jacobowitz <dan@codesourcery.com> * 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. 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 23 Sep 2010 18:43:29 -0000 @@ -1706,6 +1706,150 @@ arm_dwarf2_frame_init_reg (struct gdbarc } } +/* 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); + + /* The epilogue is a sequence of instructions along the following lines: + + - [optionally] set SP from the frame pointer + - restore registers from SP [may include PC] + - a return-type instruction [if PC wasn't already restored] + + If, scanning forward from the current PC, the instructions we find + are compatible with this sequence (ending in a return), we consider + the current PC to be part of the epilogue. */ + + for (;;) + { + unsigned int insn; + gdb_byte buf[2]; + + if (target_read_memory (pc, buf, 2)) + return 0; + + pc += 2; + insn = extract_unsigned_integer (buf, 2, byte_order_for_code); + + if ((insn & 0xff80) == 0x4700) /* bx <Rm> */ + return 1; + else if (insn == 0x46f7) /* mov pc, lr */ + return 1; + else if (insn == 0x46bd) /* mov sp, r7 */ + ; + else if ((insn & 0xfe00) == 0xbc00) /* pop <registers> */ + { + if (insn & 0x0100) /* <registers> include PC. */ + return 1; + } + else if ((insn & 0xe000) == 0xe000) /* 32-bit Thumb-2 instruction */ + { + unsigned int insn2; + + if (target_read_memory (pc, buf, 2)) + return 0; + + pc += 2; + insn2 = extract_unsigned_integer (buf, 2, byte_order_for_code); + + if ((insn & 0xffdf) == 0xe89d) /* ldm.w sp{!}, <registers> */ + { + if (insn2 & 0x8000) /* <registers> include PC. */ + return 1; + } + else if (insn == 0xf85d /* ldr.w <Rt>, [sp], #4 */ + && (insn2 & 0x0fff) == 0x0b04) + { + if ((insn2 & 0xf000) == 0xf000) /* <Rt> is PC. */ + return 1; + } + else if ((insn & 0xffbf) == 0xecbd /* vldm sp!, <list> */ + && (insn2 & 0x0e00) == 0x0a00) + ; + else + return 0; + } + else + 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. */ @@ -6881,6 +7025,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); -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [rfa] Fix software-watchpoint failures by adding epilogue detection 2010-09-24 16:11 ` Ulrich Weigand @ 2010-09-28 20:23 ` Daniel Jacobowitz 2010-09-28 21:47 ` Ulrich Weigand 0 siblings, 1 reply; 12+ messages in thread From: Daniel Jacobowitz @ 2010-09-28 20:23 UTC (permalink / raw) To: Ulrich Weigand; +Cc: gdb-patches, rearnsha On Fri, Sep 24, 2010 at 02:39:05PM +0200, Ulrich Weigand wrote: > 1. Some epilogue sequences accepted by your patch were not accepted by mine: > - My patch only accepted "bx lr" as return, while yours accepts any "bx". > - I had a typo in one of the instruction opcodes. > > 2. Some epilogue sequences accepted by my patch were not accepted by yours: > - I'm allowing "mov sp, r7" and "vldm" instructions, as well as certain > additional cases of "ldm.w". These all sound good. > - I'm accepting more diverse sequences due to forward-scanning for multiple > instructions, and not requiring backward-scanning. This I'm worried about. From my patch: + /* 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). This is definitely an epilogue: pop { r4, r5, r6, lr } bx lr This could be an epilogue, but it could also be an indirect call: bx lr If it's an indirect call there would be a mov lr, pc before it. If it's an indirect tail call, then it's an epilogue, and the return address won't be saved. If there's no stack adjustment, then gdbarch_in_function_epilogue_p does not need to return 1; the predicate really means "we can not check for watchpoints because the frame might be in an inconsistent state". Is it safe for this predicate to return 1 around something that is not an epilogue? Given that definition of the predicate, the backwards scan is appropriate; without a backwards scan, we can only answer "is there an epilogue after this point", not "are we already inside an epilogue". Of course, if it turns out harmless to return false positives... I'm not sure. -- Daniel Jacobowitz CodeSourcery ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [rfa] Fix software-watchpoint failures by adding epilogue detection 2010-09-28 20:23 ` Daniel Jacobowitz @ 2010-09-28 21:47 ` Ulrich Weigand 2010-09-28 21:49 ` Daniel Jacobowitz 2010-09-29 14:43 ` [rfa] Fix software-watchpoint failures " Yao Qi 0 siblings, 2 replies; 12+ messages in thread From: Ulrich Weigand @ 2010-09-28 21:47 UTC (permalink / raw) To: Daniel Jacobowitz; +Cc: gdb-patches, rearnsha Daniel Jacobowitz wrote: > On Fri, Sep 24, 2010 at 02:39:05PM +0200, Ulrich Weigand wrote: > > - I'm accepting more diverse sequences due to forward-scanning for multiple > > instructions, and not requiring backward-scanning. > > This I'm worried about. From my patch: > > + /* 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). > > This is definitely an epilogue: > > pop { r4, r5, r6, lr } > bx lr > > This could be an epilogue, but it could also be an indirect call: > > bx lr > > If it's an indirect call there would be a mov lr, pc before it. > If it's an indirect tail call, then it's an epilogue, and the return > address won't be saved. 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? My original patch accepted only specifically "bx lr"; yours also accepts different registers for bx. If we have a bx with a different register, this may of course well be an indirect 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". > If there's no stack adjustment, then gdbarch_in_function_epilogue_p > does not need to return 1; the predicate really means "we can not > check for watchpoints because the frame might be in an inconsistent > state". > > Is it safe for this predicate to return 1 around something that is not > an epilogue? > > Given that definition of the predicate, the backwards scan is > appropriate; without a backwards scan, we can only answer "is there an > epilogue after this point", not "are we already inside an epilogue". > > Of course, if it turns out harmless to return false positives... I'm > not sure. It seems to me that it is relatively harmless to return a false positive; the only thing that happens is that the check for watchpoint hits is delayed until the next instruction. In particular, returning true in the epilogue of a frameless functions should definitely be harmless. (Returning true on a bx that implements a function call might in rare cases lead to a watchpoint hit being detected on the first instruction of the called function instead ...) Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [rfa] Fix software-watchpoint failures by adding epilogue detection 2010-09-28 21:47 ` Ulrich Weigand @ 2010-09-28 21:49 ` Daniel Jacobowitz 2010-09-29 15:24 ` Richard Earnshaw 2010-10-07 16:12 ` [rfa, v3] Fix software-watchpoint failures on ARM " Ulrich Weigand 2010-09-29 14:43 ` [rfa] Fix software-watchpoint failures " Yao Qi 1 sibling, 2 replies; 12+ messages in thread From: Daniel Jacobowitz @ 2010-09-28 21:49 UTC (permalink / raw) To: Ulrich Weigand; +Cc: gdb-patches, rearnsha 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. But I did get this out of RealView: void (*foo)(); void bar() { foo(); } bar PROC LDR r0,|L1.12| LDR r0,[r0,#0] ; foo BX r0 ENDP > It seems to me that it is relatively harmless to return a false positive; > the only thing that happens is that the check for watchpoint hits is > delayed until the next instruction. In particular, returning true in > the epilogue of a frameless functions should definitely be harmless. > (Returning true on a bx that implements a function call might in rare > cases lead to a watchpoint hit being detected on the first instruction > of the called function instead ...) Yes, that sounds like the case I was worried about. Of course, it's relatively harmless either way; nothing in GDB absolutely relies on this hook. So I won't object to the patch as-is. This would be a nice thing to clean up some day. -- Daniel Jacobowitz CodeSourcery ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [rfa] Fix software-watchpoint failures by adding epilogue detection 2010-09-28 21:49 ` Daniel Jacobowitz @ 2010-09-29 15:24 ` Richard Earnshaw 2010-10-07 16:12 ` [rfa, v3] Fix software-watchpoint failures on ARM " Ulrich Weigand 1 sibling, 0 replies; 12+ messages in thread From: Richard Earnshaw @ 2010-09-29 15:24 UTC (permalink / raw) To: Daniel Jacobowitz; +Cc: Ulrich Weigand, gdb-patches On Tue, 2010-09-28 at 12:32 -0400, 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. But I did get this out of RealView: > You can never use LR for an indirect tailcall as it must contain the return address from the outer function (at least, in any ABI compliant code). The mcount code does something a bit funky here, but that's a special case. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [rfa, v3] Fix software-watchpoint failures on ARM by adding epilogue detection 2010-09-28 21:49 ` Daniel Jacobowitz 2010-09-29 15:24 ` Richard Earnshaw @ 2010-10-07 16:12 ` Ulrich Weigand 2010-10-08 13:01 ` Richard Earnshaw 1 sibling, 1 reply; 12+ messages in thread From: Ulrich Weigand @ 2010-10-07 16:12 UTC (permalink / raw) To: Daniel Jacobowitz, rearnsha; +Cc: gdb-patches 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 <uweigand@de.ibm.com> Daniel Jacobowitz <dan@codesourcery.com> * 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. 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 <Rm> */ + 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 <registers> */ + { + found_stack_adjust = 1; + if (insn & 0x0100) /* <registers> 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!, <registers> */ + { + found_stack_adjust = 1; + if (insn2 & 0x8000) /* <registers> include PC. */ + found_return = 1; + } + else if (insn == 0xf85d /* ldr.w <Rt>, [sp], #4 */ + && (insn2 & 0x0fff) == 0x0b04) + { + found_stack_adjust = 1; + if ((insn2 & 0xf000) == 0xf000) /* <Rt> is PC. */ + found_return = 1; + } + else if ((insn & 0xffbf) == 0xecbd /* vldm sp!, <list> */ + && (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 <registers> without PC */ + found_stack_adjust = 1; + else if (insn == 0xe8bd) /* ldm.w sp!, <registers> */ + found_stack_adjust = 1; + else if (insn == 0xf85d /* ldr.w <Rt>, [sp], #4 */ + && (insn2 & 0x0fff) == 0x0b04) + found_stack_adjust = 1; + else if ((insn & 0xffbf) == 0xecbd /* vldm sp!, <list> */ + && (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); -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [rfa, v3] Fix software-watchpoint failures on ARM by adding epilogue detection 2010-10-07 16:12 ` [rfa, v3] Fix software-watchpoint failures on ARM " Ulrich Weigand @ 2010-10-08 13:01 ` Richard Earnshaw 2010-10-08 13:27 ` Ulrich Weigand 0 siblings, 1 reply; 12+ messages in thread From: Richard Earnshaw @ 2010-10-08 13:01 UTC (permalink / raw) To: Ulrich Weigand; +Cc: Daniel Jacobowitz, gdb-patches 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 <uweigand@de.ibm.com> > Daniel Jacobowitz <dan@codesourcery.com> > > * 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 <Rm> */ > + 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 <registers> */ > + { > + found_stack_adjust = 1; > + if (insn & 0x0100) /* <registers> 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!, <registers> */ > + { > + found_stack_adjust = 1; > + if (insn2 & 0x8000) /* <registers> include PC. */ > + found_return = 1; > + } > + else if (insn == 0xf85d /* ldr.w <Rt>, [sp], #4 */ > + && (insn2 & 0x0fff) == 0x0b04) > + { > + found_stack_adjust = 1; > + if ((insn2 & 0xf000) == 0xf000) /* <Rt> is PC. */ > + found_return = 1; > + } > + else if ((insn & 0xffbf) == 0xecbd /* vldm sp!, <list> */ > + && (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 <registers> without PC */ > + found_stack_adjust = 1; > + else if (insn == 0xe8bd) /* ldm.w sp!, <registers> */ > + found_stack_adjust = 1; > + else if (insn == 0xf85d /* ldr.w <Rt>, [sp], #4 */ > + && (insn2 & 0x0fff) == 0x0b04) > + found_stack_adjust = 1; > + else if ((insn & 0xffbf) == 0xecbd /* vldm sp!, <list> */ > + && (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); > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [rfa, v3] Fix software-watchpoint failures on ARM by adding epilogue detection 2010-10-08 13:01 ` Richard Earnshaw @ 2010-10-08 13:27 ` Ulrich Weigand 0 siblings, 0 replies; 12+ messages in thread From: Ulrich Weigand @ 2010-10-08 13:27 UTC (permalink / raw) To: Richard Earnshaw; +Cc: Daniel Jacobowitz, gdb-patches Richard Earnshaw wrote: > On Thu, 2010-10-07 at 18:12 +0200, Ulrich Weigand wrote: > > 2010-10-07 Ulrich Weigand <uweigand@de.ibm.com> > > Daniel Jacobowitz <dan@codesourcery.com> > > > > * 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. Thanks for the review; I've checked the patch in now. We can always go back and add more instructions later ... Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [rfa] Fix software-watchpoint failures by adding epilogue detection 2010-09-28 21:47 ` Ulrich Weigand 2010-09-28 21:49 ` Daniel Jacobowitz @ 2010-09-29 14:43 ` Yao Qi 1 sibling, 0 replies; 12+ messages in thread From: Yao Qi @ 2010-09-29 14:43 UTC (permalink / raw) To: gdb-patches On Tue, Sep 28, 2010 at 06:04:14PM +0200, Ulrich Weigand wrote: > Daniel Jacobowitz wrote: > > > On Fri, Sep 24, 2010 at 02:39:05PM +0200, Ulrich Weigand wrote: > > > - I'm accepting more diverse sequences due to forward-scanning for multiple > > > instructions, and not requiring backward-scanning. > > > > This I'm worried about. From my patch: > > > > + /* 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). > > > > This is definitely an epilogue: > > > > pop { r4, r5, r6, lr } > > bx lr > > > > This could be an epilogue, but it could also be an indirect call: > > > > bx lr > > > > If it's an indirect call there would be a mov lr, pc before it. > > If it's an indirect tail call, then it's an epilogue, and the return > > address won't be saved. > > 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? > > My original patch accepted only specifically "bx lr"; yours also > accepts different registers for bx. If we have a bx with a > different register, this may of course well be an indirect 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". > I haven't seen any examples GCC uses bx with other registers so far, but I noticed that some one is thinking of use 'bx r3' for optimization purpose. http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40887 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19599 The patch for PR19599 hasn't been committed yet, so I still can't find a real example that using 'bx r3' to return. In short, there *might* be some cases now or in the future, that registers other than lr are used with bx for return. -- Yao Qi CodeSourcery yao@codesourcery.com (650) 331-3385 x739 ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2010-10-08 13:27 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-09-22 19:20 [rfa] Fix software-watchpoint failures by adding epilogue detection Ulrich Weigand 2010-09-22 20:01 ` Daniel Jacobowitz 2010-09-23 16:13 ` Ulrich Weigand 2010-09-24 16:11 ` Ulrich Weigand 2010-09-28 20:23 ` Daniel Jacobowitz 2010-09-28 21:47 ` Ulrich Weigand 2010-09-28 21:49 ` Daniel Jacobowitz 2010-09-29 15:24 ` Richard Earnshaw 2010-10-07 16:12 ` [rfa, v3] Fix software-watchpoint failures on ARM " Ulrich Weigand 2010-10-08 13:01 ` Richard Earnshaw 2010-10-08 13:27 ` Ulrich Weigand 2010-09-29 14:43 ` [rfa] Fix software-watchpoint failures " Yao Qi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox