public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [patch] Fix PR tdep/12352: Handle str pc, [Rd, #imm] in displaced stepping
@ 2010-12-28 17:24 Yao Qi
  2011-01-06 14:03 ` [Ping : patch] " Yao Qi
  2011-01-22 23:44 ` [patch] " Richard Earnshaw
  0 siblings, 2 replies; 14+ messages in thread
From: Yao Qi @ 2010-12-28 17:24 UTC (permalink / raw)
  To: gdb-patches

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

When I read arm-tdep.c:copy_ldr_str_ldrb_strb, I feel pretty hard to
understand two lines of code,

      dsc->modinsn[0] = 0xe58ff014;  /* str pc, [pc, #20].  */
      dsc->modinsn[1] = 0xe59f4010;  /* ldr r4, [pc, #16].  */

Shall program get SIGSEGV when executing `str pc, [pc, #20]' during
displaced stepping?  A simple test case confirmed my guess, which is
included in arm-disp-step.S in this patch.

If it is a bug here, this patch is to address it.  These two lines of
code is to compute the offset of `str pc'.  In this patch, we can do
this in a different way,

	str pc, [sp, #-4]
	ldr r4, [sp, #-4]

OK for mainline and 7.2 branch?

-- 
Yao (齐尧)

[-- Attachment #2: pr12352.patch --]
[-- Type: text/x-patch, Size: 3332 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.

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..ad9b1f4 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -5152,7 +5152,7 @@ copy_ldr_str_ldrb_strb (struct gdbarch *gdbarch, uint32_t insn,
      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>
+     temp can be [sp, -4]
 
      Otherwise we don't know what value to write for PC, since the offset is
      architecture-dependent (sometimes PC+8, sometimes PC+12).  */
@@ -5177,8 +5177,8 @@ 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] = 0xe58ff014;  /* str pc, [pc, #20].  */
-      dsc->modinsn[1] = 0xe59f4010;  /* ldr r4, [pc, #16].  */
+      dsc->modinsn[0] = 0xe50df004;  /* str pc, [sp, #-4]*/
+      dsc->modinsn[1] = 0xe51d4004;  /* ldr r4, [sp, #-4]*/
       dsc->modinsn[2] = 0xe044400f;  /* sub r4, r4, pc.  */
       dsc->modinsn[3] = 0xe2844008;  /* add r4, r4, #8.  */
       dsc->modinsn[4] = 0xe0800004;  /* add r0, r0, r4.  */
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.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Ping : patch] Fix PR tdep/12352: Handle str pc, [Rd, #imm] in displaced stepping
  2010-12-28 17:24 [patch] Fix PR tdep/12352: Handle str pc, [Rd, #imm] in displaced stepping Yao Qi
@ 2011-01-06 14:03 ` Yao Qi
  2011-01-19 16:17   ` [Ping 2: " Yao Qi
  2011-01-22 23:44 ` [patch] " Richard Earnshaw
  1 sibling, 1 reply; 14+ messages in thread
From: Yao Qi @ 2011-01-06 14:03 UTC (permalink / raw)
  To: gdb-patches

On 12/29/2010 12:41 AM, Yao Qi wrote:
> 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.

Ping.
http://sourceware.org/ml/gdb-patches/2010-12/msg00514.html

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Ping 2: patch] Fix PR tdep/12352: Handle str pc, [Rd, #imm] in displaced stepping
  2011-01-06 14:03 ` [Ping : patch] " Yao Qi
@ 2011-01-19 16:17   ` Yao Qi
  0 siblings, 0 replies; 14+ messages in thread
From: Yao Qi @ 2011-01-19 16:17 UTC (permalink / raw)
  To: gdb-patches

On 01/06/2011 07:03 AM, Yao Qi wrote:
> On 12/29/2010 12:41 AM, Yao Qi wrote:
>> 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.
>
> Ping.
> http://sourceware.org/ml/gdb-patches/2010-12/msg00514.html
>

Ping.

-- 
Yao Qi

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch] Fix PR tdep/12352: Handle str pc, [Rd, #imm] in displaced stepping
  2010-12-28 17:24 [patch] Fix PR tdep/12352: Handle str pc, [Rd, #imm] in displaced stepping Yao Qi
  2011-01-06 14:03 ` [Ping : patch] " Yao Qi
@ 2011-01-22 23:44 ` Richard Earnshaw
  2011-01-24 13:22   ` Yao Qi
  2011-01-31  0:31   ` Yao Qi
  1 sibling, 2 replies; 14+ messages in thread
From: Richard Earnshaw @ 2011-01-22 23:44 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches, julian

On 28/12/10 16:41, Yao Qi wrote:
> When I read arm-tdep.c:copy_ldr_str_ldrb_strb, I feel pretty hard to
> understand two lines of code,
> 
>       dsc->modinsn[0] = 0xe58ff014;  /* str pc, [pc, #20].  */
>       dsc->modinsn[1] = 0xe59f4010;  /* ldr r4, [pc, #16].  */

Hmm, looks like a rather convoluted way of moving the PC into R4.
Julian, I think this patch was yours... can you remember why  "MOV R4,
PC" wasn't sufficient?

> 
> Shall program get SIGSEGV when executing `str pc, [pc, #20]' during
> displaced stepping?  A simple test case confirmed my guess, which is
> included in arm-disp-step.S in this patch.
> 
> If it is a bug here, this patch is to address it.  These two lines of
> code is to compute the offset of `str pc'.  In this patch, we can do
> this in a different way,
> 
> 	str pc, [sp, #-4]
> 	ldr r4, [sp, #-4]
> 
> OK for mainline and 7.2 branch?
> 

No, code must not write below the stack -- the value can get corrupted
if an interrupt occurs.  (I'm not sure if that's possible in this
specific case as the debugger ought to be in control; but it's bad
practice to violate the ABI in this way).

R.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch] Fix PR tdep/12352: Handle str pc, [Rd, #imm] in displaced stepping
  2011-01-22 23:44 ` [patch] " Richard Earnshaw
@ 2011-01-24 13:22   ` Yao Qi
  2011-01-31  0:31   ` Yao Qi
  1 sibling, 0 replies; 14+ messages in thread
From: Yao Qi @ 2011-01-24 13:22 UTC (permalink / raw)
  To: Richard Earnshaw; +Cc: gdb-patches, julian

On 01/22/2011 11:07 PM, Richard Earnshaw wrote:
> On 28/12/10 16:41, Yao Qi wrote:
>> When I read arm-tdep.c:copy_ldr_str_ldrb_strb, I feel pretty hard to
>> understand two lines of code,
>>
>>       dsc->modinsn[0] = 0xe58ff014;  /* str pc, [pc, #20].  */
>>       dsc->modinsn[1] = 0xe59f4010;  /* ldr r4, [pc, #16].  */
> 
> Hmm, looks like a rather convoluted way of moving the PC into R4.
> Julian, I think this patch was yours... can you remember why  "MOV R4,
> PC" wasn't sufficient?
> 

IIUC, these two instructions together with 'sub r4, r4, pc' are used to
calculate the offset of 'str pc ADDR', which is 8 or
12(implementation-dependent, but consistent for a given device).
AFAIK, 'mov r4, pc' doesn't help.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch] Fix PR tdep/12352: Handle str pc, [Rd, #imm] in displaced stepping
  2011-01-22 23:44 ` [patch] " Richard Earnshaw
  2011-01-24 13:22   ` Yao Qi
@ 2011-01-31  0:31   ` Yao Qi
  2011-01-31 15:56     ` Ulrich Weigand
  1 sibling, 1 reply; 14+ messages in thread
From: Yao Qi @ 2011-01-31  0:31 UTC (permalink / raw)
  To: Richard Earnshaw; +Cc: gdb-patches, julian

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

On 01/22/2011 11:07 PM, Richard Earnshaw wrote:
>> > If it is a bug here, this patch is to address it.  These two lines of
>> > code is to compute the offset of `str pc'.  In this patch, we can do
>> > this in a different way,
>> > 
>> > 	str pc, [sp, #-4]
>> > 	ldr r4, [sp, #-4]
>> > 
>> > 
> No, code must not write below the stack -- the value can get corrupted
> if an interrupt occurs.  (I'm not sure if that's possible in this
> specific case as the debugger ought to be in control; but it's bad
> practice to violate the ABI in this way).

When these two instructions are running, debugger is not in control.

How about this insn sequence, which should comply with ABI?

	sub sp,	#4
	str pc, [sp]
	ldr r4, [sp]
	add sp, #4

Tested new patch in ARM native GDB with arm-disp-step.exp.  No failures.

-- 
Yao (齐尧)

[-- Attachment #2: pr12352-0127.patch --]
[-- Type: text/x-patch, Size: 4818 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..a37ab15 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -5152,7 +5152,7 @@ copy_ldr_str_ldrb_strb (struct gdbarch *gdbarch, uint32_t insn,
      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>
+     temp can be [sp, -4]
 
      Otherwise we don't know what value to write for PC, since the offset is
      architecture-dependent (sometimes PC+8, sometimes PC+12).  */
@@ -5176,23 +5176,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.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch] Fix PR tdep/12352: Handle str pc, [Rd, #imm] in displaced stepping
  2011-01-31  0:31   ` Yao Qi
@ 2011-01-31 15:56     ` Ulrich Weigand
  2011-02-09  6:15       ` Yao Qi
  0 siblings, 1 reply; 14+ messages in thread
From: Ulrich Weigand @ 2011-01-31 15:56 UTC (permalink / raw)
  To: Yao Qi; +Cc: Richard Earnshaw, gdb-patches, julian

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?

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] 14+ messages in thread

* Re: [patch] Fix PR tdep/12352: Handle str pc, [Rd, #imm] in displaced stepping
  2011-01-31 15:56     ` Ulrich Weigand
@ 2011-02-09  6:15       ` Yao Qi
  2011-02-09 13:51         ` Ulrich Weigand
  0 siblings, 1 reply; 14+ messages in thread
From: Yao Qi @ 2011-02-09  6:15 UTC (permalink / raw)
  To: Ulrich Weigand; +Cc: Richard Earnshaw, gdb-patches, julian

[-- 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.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch] Fix PR tdep/12352: Handle str pc, [Rd, #imm] in displaced stepping
  2011-02-09  6:15       ` Yao Qi
@ 2011-02-09 13:51         ` Ulrich Weigand
  2011-02-10  6:19           ` Yao Qi
  0 siblings, 1 reply; 14+ messages in thread
From: Ulrich Weigand @ 2011-02-09 13:51 UTC (permalink / raw)
  To: Yao Qi; +Cc: Richard Earnshaw, gdb-patches, julian

Yao Qi wrote:

> 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

Yes, I'm aware of that.  However, my understanding is that this special
definition of STR PC applies to *all* variants of STR, including PUSH
(PUSH { PC } is just another mnemonic for STR PC, [ SP, #-4 ]).

If you look at the formal semantics definition in the ARM reference
manual, all variants of STR (including PUSH) use the pseudo-code macro
"PCStoreValue" to implement storing of the PC, which is defined to
include the implementation-defined constant ...

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] 14+ messages in thread

* Re: [patch] Fix PR tdep/12352: Handle str pc, [Rd, #imm] in displaced stepping
  2011-02-09 13:51         ` Ulrich Weigand
@ 2011-02-10  6:19           ` Yao Qi
  2011-02-14 14:39             ` Ulrich Weigand
  0 siblings, 1 reply; 14+ messages in thread
From: Yao Qi @ 2011-02-10  6:19 UTC (permalink / raw)
  To: Ulrich Weigand; +Cc: Richard Earnshaw, gdb-patches, julian

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

On 02/09/2011 09:51 PM, Ulrich Weigand wrote:
> Yes, I'm aware of that.  However, my understanding is that this special
> definition of STR PC applies to *all* variants of STR, including PUSH
> (PUSH { PC } is just another mnemonic for STR PC, [ SP, #-4 ]).
> 

Oh, I understand your points now.  You are right.  I didn't realize that
before.

> If you look at the formal semantics definition in the ARM reference
> manual, all variants of STR (including PUSH) use the pseudo-code macro
> "PCStoreValue" to implement storing of the PC, which is defined to
> include the implementation-defined constant ...

A new patch is attached in which PUSH/POP is used.  I also noticed that
Insn4 should be 'add r4, r4, #16' rather than 'add r4, r4, #8',
explained in the comments.  After that, the offset of my board is 8.

Run patched arm-disp-step.exp on native GDB configured as
armv7l-unknown-linux-gnueabi.  No failures.

-- 
Yao (齐尧)

[-- Attachment #2: pr12352-0210.patch --]
[-- Type: text/x-patch, Size: 4356 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.

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..7591b62 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>
+     Insn1: push {pc} Write address of STR instruction + offset on stack
+     Insn2: pop  {r4} Read it back from stack, r4 = addr(Insn1) + offset
+     Insn3: sub r4, r4, pc   r4 = addr(Insn1) + offset - pc
+                                = addr(Insn1) + offset - addr(Insn3) - 8
+                                = offset - 16
+     Insn4: add r4, r4, #16  r4 = offset
+     Insn5: add r0, r0, r4
+     Insn6: 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,11 +5179,10 @@ 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] = 0xe58ff014;  /* str pc, [pc, #20].  */
-      dsc->modinsn[1] = 0xe59f4010;  /* ldr r4, [pc, #16].  */
+      dsc->modinsn[0] = 0xe92d8000;  /* push {pc} */
+      dsc->modinsn[1] = 0xe8bd0010;  /* pop  {r4} */
       dsc->modinsn[2] = 0xe044400f;  /* sub r4, r4, pc.  */
-      dsc->modinsn[3] = 0xe2844008;  /* add r4, r4, #8.  */
+      dsc->modinsn[3] = 0xe2844010;  /* add r4, r4, #16.  */
       dsc->modinsn[4] = 0xe0800004;  /* add r0, r0, r4.  */
 
       /* As above.  */
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.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch] Fix PR tdep/12352: Handle str pc, [Rd, #imm] in displaced stepping
  2011-02-10  6:19           ` Yao Qi
@ 2011-02-14 14:39             ` Ulrich Weigand
  2011-02-15 10:55               ` Yao Qi
  0 siblings, 1 reply; 14+ messages in thread
From: Ulrich Weigand @ 2011-02-14 14:39 UTC (permalink / raw)
  To: Yao Qi; +Cc: Richard Earnshaw, gdb-patches, julian

Yao Qi wrote:

> A new patch is attached in which PUSH/POP is used.  I also noticed that
> Insn4 should be 'add r4, r4, #16' rather than 'add r4, r4, #8',
> explained in the comments.  After that, the offset of my board is 8.

I think this is wrong: the "pipeline offset" of 8 bytes is already
added to the PC value when it is retrieved by displaced_read_reg
(this applies to any use of PC as source operand in any instruction).

The special case relating to STR PC is about an optional *additional*
offset of 4 bytes (such that PC + 12 instead of PC + 8 is stored);
the "offset" value computed by this routine should therefore be
0 or 4 (not 8 or 12).

> Run patched arm-disp-step.exp on native GDB configured as
> armv7l-unknown-linux-gnueabi.  No failures.

Maybe it would be good to add a test that verifies the value
stored under displaced stepping is identical to the value
stored when running the instruction natively?

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] 14+ messages in thread

* Re: [patch] Fix PR tdep/12352: Handle str pc, [Rd, #imm] in displaced stepping
  2011-02-14 14:39             ` Ulrich Weigand
@ 2011-02-15 10:55               ` Yao Qi
  2011-02-15 13:36                 ` Ulrich Weigand
  0 siblings, 1 reply; 14+ messages in thread
From: Yao Qi @ 2011-02-15 10:55 UTC (permalink / raw)
  To: Ulrich Weigand; +Cc: Richard Earnshaw, gdb-patches, julian

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

On 02/14/2011 10:16 PM, Ulrich Weigand wrote:
> I think this is wrong: the "pipeline offset" of 8 bytes is already
> added to the PC value when it is retrieved by displaced_read_reg
> (this applies to any use of PC as source operand in any instruction).
> 
> The special case relating to STR PC is about an optional *additional*
> offset of 4 bytes (such that PC + 12 instead of PC + 8 is stored);
> the "offset" value computed by this routine should therefore be
> 0 or 4 (not 8 or 12).

Thanks for the explanation.

> Maybe it would be good to add a test that verifies the value
> stored under displaced stepping is identical to the value
> stored when running the instruction natively?

In my new patch, the test case is revised to execute instructions below
twice,

       str     pc, [sp, #-4]
       ldr     rN, [sp, #-4]
       sub     rN, rN, pc

the first `str' instruction is executed with displaced stepping, while
the second `str' is executed without displaced stepping.  Then, values
of two registers are compared to make sure they should be the same.

-- 
Yao (齐尧)

[-- Attachment #2: pr12352-0215.patch --]
[-- Type: text/x-patch, Size: 5465 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.

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..440fda5 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -5146,16 +5146,24 @@ 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>
+     Before this sequence of instructions:
+     r0 is the PC value got from displaced_read_reg, so r0 = from + 8;
+     r2 is the Rn value got from dispalced_read_reg.
+
+     Insn1: push {pc} Write address of STR instruction + offset on stack
+     Insn2: pop  {r4} Read it back from stack, r4 = addr(Insn1) + offset
+     Insn3: sub r4, r4, pc   r4 = addr(Insn1) + offset - pc
+                                = addr(Insn1) + offset - addr(Insn3) - 8
+                                = offset - 16
+     Insn4: add r4, r4, #8   r4 = offset - 8
+     Insn5: add r0, r0, r4   r0 = from + 8 + offset - 8
+                                = from + offset
+     Insn6: 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,9 +5184,8 @@ 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] = 0xe58ff014;  /* str pc, [pc, #20].  */
-      dsc->modinsn[1] = 0xe59f4010;  /* ldr r4, [pc, #16].  */
+      dsc->modinsn[0] = 0xe92d8000;  /* push {pc} */
+      dsc->modinsn[1] = 0xe8bd0010;  /* pop  {r4} */
       dsc->modinsn[2] = 0xe044400f;  /* sub r4, r4, pc.  */
       dsc->modinsn[3] = 0xe2844008;  /* add r4, r4, #8.  */
       dsc->modinsn[4] = 0xe0800004;  /* add r0, r0, r4.  */
diff --git a/gdb/testsuite/gdb.arch/arm-disp-step.S b/gdb/testsuite/gdb.arch/arm-disp-step.S
index d748718..4b637d7 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,36 @@ 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]
+	ldr	r0, [sp, #-4]
+	sub	r0, r0, pc
+	/* compute offset again without displaced stepping.  */
+	str     pc, [sp, #-4]
+	ldr	r1, [sp, #-4]
+	sub	r1, r1, pc
+
+	/* r0 should be equal to r1.  */
+	cmp	r0, r1
+	bne	pc_offset_wrong
+
+	.global pc_offset_right
+pc_offset_right:
+	b	test_str_pc_end
+
+	.global pc_offset_wrong
+pc_offset_wrong:
+	nop
+
+	.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..80e4050 100644
--- a/gdb/testsuite/gdb.arch/arm-disp-step.exp
+++ b/gdb/testsuite/gdb.arch/arm-disp-step.exp
@@ -126,6 +126,42 @@ 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"
+
+    # Set breakpoint on both lables pc_offset_right and pc_offset_wrong
+    gdb_test "break *pc_offset_right" \
+	"Breakpoint.*at.* file .*$srcfile, line.*" \
+	"break pc_offset_right"
+    gdb_test "break *pc_offset_wrong" \
+	"Breakpoint.*at.* file .*$srcfile, line.*" \
+	"break pc_offset_wrong"
+
+    gdb_continue_to_breakpoint "continue to test_str_pc" \
+	".*str.*pc\,.*\[sp, #-4\].*"
+    # If breakpoint on lable pc_offset_wrong is hit, that means the offset
+    # computed in displaced stepping is different from offset computed
+    # without displaced stepping.  Report a failure.
+    gdb_continue_to_breakpoint "continue to pc_offset_right" \
+	".*b.*test_str_pc_end.*"
+    gdb_continue_to_breakpoint "continue to test_str_pc_end" \
+	".*bx lr.*"
+}
+
 # Get things started.
 
 clean_restart ${testfile}
@@ -165,6 +201,7 @@ test_ldr_from_pc
 
 test_ldm_stm_pc
 
+test_str_pc
 ##########################################
 
 # Done, run program to exit.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch] Fix PR tdep/12352: Handle str pc, [Rd, #imm] in displaced stepping
  2011-02-15 10:55               ` Yao Qi
@ 2011-02-15 13:36                 ` Ulrich Weigand
  2011-02-15 15:57                   ` Yao Qi
  0 siblings, 1 reply; 14+ messages in thread
From: Ulrich Weigand @ 2011-02-15 13:36 UTC (permalink / raw)
  To: Yao Qi; +Cc: Richard Earnshaw, gdb-patches, julian

Yao Qi wrote:

> 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.
> 
> gdb/testsuite/
>         PR tdep/12352
>         * gdb.arch/arm-disp-step.S : New test for str instruction.
>         * gdb.arch/arm-disp-step.exp : Likewise.

This is OK.

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch] Fix PR tdep/12352: Handle str pc, [Rd, #imm] in displaced stepping
  2011-02-15 13:36                 ` Ulrich Weigand
@ 2011-02-15 15:57                   ` Yao Qi
  0 siblings, 0 replies; 14+ messages in thread
From: Yao Qi @ 2011-02-15 15:57 UTC (permalink / raw)
  To: Ulrich Weigand; +Cc: Richard Earnshaw, gdb-patches, julian

On 02/15/2011 09:22 PM, Ulrich Weigand wrote:
> Yao Qi wrote:
> 
>> 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.
>>
>> gdb/testsuite/
>>         PR tdep/12352
>>         * gdb.arch/arm-disp-step.S : New test for str instruction.
>>         * gdb.arch/arm-disp-step.exp : Likewise.
> 
> This is OK.

Thanks for your patient review.  Applied.
Closed PR tdep/12352.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2011-02-15 15:32 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-12-28 17:24 [patch] Fix PR tdep/12352: Handle str pc, [Rd, #imm] in displaced stepping Yao Qi
2011-01-06 14:03 ` [Ping : patch] " Yao Qi
2011-01-19 16:17   ` [Ping 2: " Yao Qi
2011-01-22 23:44 ` [patch] " Richard Earnshaw
2011-01-24 13:22   ` Yao Qi
2011-01-31  0:31   ` Yao Qi
2011-01-31 15:56     ` Ulrich Weigand
2011-02-09  6:15       ` Yao Qi
2011-02-09 13:51         ` Ulrich Weigand
2011-02-10  6:19           ` Yao Qi
2011-02-14 14:39             ` Ulrich Weigand
2011-02-15 10:55               ` Yao Qi
2011-02-15 13:36                 ` Ulrich Weigand
2011-02-15 15:57                   ` Yao Qi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).