Re: [patch] Fix PR tdep/12352: Handle str pc, [Rd, #imm] in displaced stepping

[Yao Qi <yao@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 1384 bytes --]

On 01/31/2011 11:40 PM, Ulrich Weigand wrote:
> Yao Qi wrote.
> 
>> How about this insn sequence, which should comply with ABI?
>>
>> 	sub sp,	#4
>> 	str pc, [sp]
>> 	ldr r4, [sp]
>> 	add sp, #4
> 
> Why then not simply this:
> 
>    e92d8000        push    {pc}
>    e8bd0010        pop     {r4}
> 
> which should do the same but still is just two instructions?

[Sorry for the late reply.  Back from Chinese Spring Festival holiday.]

I am afraid they are not equal to each other.  The intention of this
complicated insn sequence is used to compute the implementation-defined
constant offset of `str pc'.  See more explanations below.

http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0204g/Cihbjifh.html

Section "Saving from r15"
[...]
If you do save from r15, the value saved is the address of the current
instruction, plus an implementation-defined constant. The constant is
always the same for a particular processor.
If your assembled code might be used on different processors, you can
find out what the constant is at runtime using code like the following:

    SUB R1, PC, #4 ; R1 = address of following STR instruction
    STR PC, [R0]   ; Store address of STR instruction + offset,
    LDR R0, [R0]   ; then reload it
    SUB R0, R0, R1 ; Calculate the offset as the difference

Some comments are added to explain this in new patch.

-- 
Yao (齐尧)

[-- Attachment #2: pr12352-0209.patch --]
[-- Type: text/x-patch, Size: 5628 bytes --]

gdb/
        PR tdep/12352
        * arm-tdep.c (copy_ldr_str_ldrb_strb): Replace PC with SP in
        order to store PC value on stack instead of text section.
        * arm-tdep.h (DISPLACED_MODIFIED_INSNS): Increase it to 10. 

gdb/testsuite/
        PR tdep/12352
        * gdb.arch/arm-disp-step.S : New test for str instruction.
        * gdb.arch/arm-disp-step.exp : Likewise.

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 1f05b7a..e665036 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -5146,16 +5146,19 @@ copy_ldr_str_ldrb_strb (struct gdbarch *gdbarch, uint32_t insn,
 
   /* To write PC we can do:
 
-     scratch+0:  str pc, temp  (*temp = scratch + 8 + offset)
-     scratch+4:  ldr r4, temp
-     scratch+8:  sub r4, r4, pc  (r4 = scratch + 8 + offset - scratch - 8 - 8)
-     scratch+12: add r4, r4, #8  (r4 = offset)
-     scratch+16: add r0, r0, r4
-     scratch+20: str r0, [r2, #imm] (or str r0, [r2, r3])
-     scratch+24: <temp>
+     sub sp, sp, #4  Make sure next insn operated on stack complies to ABI
+     str pc, [sp]    Write address of STR instruction + offset on stack
+     ldr r4, [sp]    Then, read it back from stack
+     add sp, sp, #4  Revert SP
+     sub r4, r4, pc
+     add r4, r4, #8  (r4 = offset)
+     add r0, r0, r4
+     str r0, [r2, #imm] (or str r0, [r2, r3])
 
      Otherwise we don't know what value to write for PC, since the offset is
-     architecture-dependent (sometimes PC+8, sometimes PC+12).  */
+     architecture-dependent (sometimes PC+8, sometimes PC+12).  More details
+     of this can be found in Section "Saving from r15" in
+     http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0204g/Cihbjifh.html */
 
   if (load || rt != 15)
     {
@@ -5176,23 +5179,25 @@ copy_ldr_str_ldrb_strb (struct gdbarch *gdbarch, uint32_t insn,
     {
       /* We need to use r4 as scratch.  Make sure it's restored afterwards.  */
       dsc->u.ldst.restore_r4 = 1;
+      dsc->modinsn[0] = 0xe24dd004;  /* sub sp, sp, #4 */
+      dsc->modinsn[1] = 0xe58df000;  /* str pc, [sp] */
+      dsc->modinsn[2] = 0xe59d4000;  /* ldr r4, [sp] */
+      dsc->modinsn[3] = 0xe28dd004;  /* add sp, sp, #4 */
 
-      dsc->modinsn[0] = 0xe58ff014;  /* str pc, [pc, #20].  */
-      dsc->modinsn[1] = 0xe59f4010;  /* ldr r4, [pc, #16].  */
-      dsc->modinsn[2] = 0xe044400f;  /* sub r4, r4, pc.  */
-      dsc->modinsn[3] = 0xe2844008;  /* add r4, r4, #8.  */
-      dsc->modinsn[4] = 0xe0800004;  /* add r0, r0, r4.  */
+      dsc->modinsn[4] = 0xe044400f;  /* sub r4, r4, pc.  */
+      dsc->modinsn[5] = 0xe2844008;  /* add r4, r4, #8.  */
+      dsc->modinsn[6] = 0xe0800004;  /* add r0, r0, r4.  */
 
       /* As above.  */
       if (immed)
-	dsc->modinsn[5] = (insn & 0xfff00fff) | 0x20000;
+	dsc->modinsn[7] = (insn & 0xfff00fff) | 0x20000;
       else
-	dsc->modinsn[5] = (insn & 0xfff00ff0) | 0x20003;
+	dsc->modinsn[7] = (insn & 0xfff00ff0) | 0x20003;
 
-      dsc->modinsn[6] = 0x0;  /* breakpoint location.  */
-      dsc->modinsn[7] = 0x0;  /* scratch space.  */
+      dsc->modinsn[8] = 0x0;  /* breakpoint location.  */
+      dsc->modinsn[9] = 0x0;  /* scratch space.  */
 
-      dsc->numinsns = 6;
+      dsc->numinsns = 8;
     }
 
   dsc->cleanup = load ? &cleanup_load : &cleanup_store;
diff --git a/gdb/arm-tdep.h b/gdb/arm-tdep.h
index 61cdb5d..dbf2c14 100644
--- a/gdb/arm-tdep.h
+++ b/gdb/arm-tdep.h
@@ -209,7 +209,7 @@ struct gdbarch_tdep
 /* The maximum number of modified instructions generated for one single-stepped
    instruction, including the breakpoint (usually at the end of the instruction
    sequence) and any scratch words, etc.  */
-#define DISPLACED_MODIFIED_INSNS	8
+#define DISPLACED_MODIFIED_INSNS	10
 
 struct displaced_step_closure
 {
diff --git a/gdb/testsuite/gdb.arch/arm-disp-step.S b/gdb/testsuite/gdb.arch/arm-disp-step.S
index d748718..1aa7df1 100644
--- a/gdb/testsuite/gdb.arch/arm-disp-step.S
+++ b/gdb/testsuite/gdb.arch/arm-disp-step.S
@@ -48,6 +48,10 @@ test_ret_end:
 	bl test_ldm_stm_pc
 #endif
 
+	/* Test str in ARM mode and Thumb-2 */
+#if !defined(__thumb__)
+	bl test_str_pc
+#endif
 	/* Return */
 	mov     sp, r7
 	sub     sp, sp, #4
@@ -118,3 +122,17 @@ test_ldm_stm_pc_ret:
 	.word	test_ldm_stm_pc_ret
 	.size test_ldm_stm_pc, .-test_ldm_stm_pc
 #endif
+
+#if !defined(__thumb__)
+#if defined (__thumb2__)
+	.code   16
+	.thumb_func
+#endif
+	.global test_str_pc
+	.type test_str_pc, %function
+test_str_pc:
+	str     pc, [sp, #-4]
+	.global test_str_pc_end
+test_str_pc_end:
+	bx lr
+#endif
diff --git a/gdb/testsuite/gdb.arch/arm-disp-step.exp b/gdb/testsuite/gdb.arch/arm-disp-step.exp
index 826f728..f2df388 100644
--- a/gdb/testsuite/gdb.arch/arm-disp-step.exp
+++ b/gdb/testsuite/gdb.arch/arm-disp-step.exp
@@ -126,6 +126,29 @@ proc test_ldr_from_pc {} {
 	".*bx lr.*"
 }
 
+###########################################
+
+proc test_str_pc {} {
+    global srcfile
+    gdb_test_multiple "break *test_str_pc" "break test_str_pc" {
+	-re "Breakpoint.*at.* file .*$srcfile, line.*" {
+	    pass "break test_str_pc"
+	}
+	-re "No symbol.*" {
+	    pass "break test_str_pc"
+	    return
+	}
+    }
+    gdb_test "break *test_str_pc_end" \
+	"Breakpoint.*at.* file .*$srcfile, line.*" \
+	"break test_str_pc_end"
+
+    gdb_continue_to_breakpoint "continue to test_str_pc" \
+	".*str.*pc\,.*\[sp, #-4\].*"
+    gdb_continue_to_breakpoint "continue to test_str_pc_end" \
+	".*bx lr.*"
+}
+
 # Get things started.
 
 clean_restart ${testfile}
@@ -165,6 +188,7 @@ test_ldr_from_pc
 
 test_ldm_stm_pc
 
+test_str_pc
 ##########################################
 
 # Done, run program to exit.