Re: [Patch,ARM] Next pc of sigreturn/rt_sigreturn syscall

Yao Qi
2010-08-26T08:51:09Z

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

Daniel Jacobowitz wrote:
> Generally looks OK.
>
> On Tue, Aug 24, 2010 at 08:05:55PM -0700, Yao Qi wrote:
>> Here is the updated patch, in which:
>> 1. Add arm_linux_syscall_next_pc, similar to
>> mips_linux_syscall_next_pc. Compute the return address of SWI in both
>> ARM mode and Thumb mode.
>> 2. Extract some common code from arm_linux_copy_svc.
>
> A valid return address won't be zero, but it's still confusing.
> Please do this the way that e.g. mips_linux_get_longjmp_target does;
> return 0 or 1, and have a CORE_ADDR * parameter.
>
OK, done as you suggested.

>> +static int
>> +arm_linux_sigreturn_return_addr(struct frame_info *frame,
>> + unsigned long svc_number)
>
> Space before "(". Same problem in other places, too.
>

Done.

>> +/* When FRAME is at a syscall instruction, return the PC of the next
>> + instruction to be executed. */
>> +
>> +static CORE_ADDR
>> +arm_linux_syscall_next_pc (struct frame_info *frame)
>> +{
>> + CORE_ADDR pc = get_frame_pc (frame);
>> + CORE_ADDR return_addr = 0;
>> + return_addr = arm_linux_sigreturn_return_addr(frame,
>> + get_frame_register_unsigned (frame, 7));
>
> Line too long; move the read of r7 to another temporary.
>

Done.

> What about non-EABI? We shouldn't wire this up for non-EABI binaries,
> because the syscall number won't be in r7.

I've added code for non-EABI to decode SWI instruction for syscall
number. See details in my patch.

>
>> @@ -2808,7 +2808,16 @@ thumb_get_next_pc_raw (struct frame_info *frame, CORE_ADDR pc, int insert_bkpt)
>> else if ((inst1 & 0xf000) == 0xd000) /* conditional branch */
>> {
>> unsigned long cond = bits (inst1, 8, 11);
>> - if (cond != 0x0f && condition_true (cond, status)) /* 0x0f = SWI */
>> + if (cond == 0x0f) /* 0x0f = SWI */
>
> Why did you remove the condition_true check?

Because I noticed that condition_true always returns true if cond is 0xf
(INST_NV).

>> + {
>> + struct gdbarch_tdep *tdep;
>> + tdep = gdbarch_tdep (get_frame_arch (frame));
>
> You can just use gdbarch_tdep (gdbarch).

OK, done in my patch.

--
Yao Qi
CodeSourcery
yao@codesourcery.com
(650) 331-3385 x739

[-- Attachment #2: return_addr_sigreturn.patch --]
[-- Type: text/x-patch, Size: 7073 bytes --]

2010-08-26 Yao Qi <yao@codesourcery.com>

* arm-linux-tdep.c (arm_linux_sigreturn_return_addr): New.
(arm_linux_syscall_next_pc): New.
(arm_linux_copy_svc): Use arm_linux_sigreturn_return_addr instead.
(arm_linux_init_abi): Initialize syscall_next_pc.
* arm-tdep.c (thumb_get_next_pc_raw): Get next pc of SWI in Thumb mode.
(arm_get_next_pc_raw): Get next pc of SWI in ARM mode.
* arm-tdep.h (struct gdbarch_tdep): Add a function pointer syscall_next_pc.
Declare arm_frame_is_thumb.

diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
index 682054c..f0e1dfa 100644
--- a/gdb/arm-linux-tdep.c
+++ b/gdb/arm-linux-tdep.c
@@ -630,6 +630,82 @@ arm_linux_regset_from_core_section (struct gdbarch *gdbarch,
return NULL;
}

+/* Copy the value of next pc of sigreturn and rt_sigrturn into PC,
+ and return 1. Return 0 if it is not a rt_sigreturn/sigreturn
+ syscall. */
+static int
+arm_linux_sigreturn_return_addr (struct frame_info *frame,
+ unsigned long svc_number,
+ CORE_ADDR *pc)
+{
+ /* Is this a sigreturn or rt_sigreturn syscall? */
+ if (svc_number == 119 || svc_number == 173)
+ {
+ if (get_frame_type (frame) == SIGTRAMP_FRAME)
+ {
+ *pc = frame_unwind_caller_pc (frame);
+ return 1;
+ }
+ }
+ return 0;
+}
+
+/* When FRAME is at a syscall instruction, return the PC of the next
+ instruction to be executed. */
+
+static CORE_ADDR
+arm_linux_syscall_next_pc (struct frame_info *frame)
+{
+ CORE_ADDR pc = get_frame_pc (frame);
+ CORE_ADDR return_addr = 0;
+ int is_thumb = arm_frame_is_thumb (frame);
+ ULONGEST svc_number = 0;
+ int is_sigreturn = 0;
+
+ if (is_thumb)
+ {
+ svc_number = get_frame_register_unsigned (frame, 7);
+ }
+ else
+ {
+ struct gdbarch *gdbarch = get_frame_arch (frame);
+ enum bfd_endian byte_order_for_code =
+ gdbarch_byte_order_for_code (gdbarch);
+ unsigned long this_instr =
+ read_memory_unsigned_integer (pc, 4, byte_order_for_code);
+
+ unsigned long svc_operand = (0x00ffffff & this_instr);
+ if (svc_operand) /* OABI. */
+ {
+ svc_number = svc_operand - 0x900000;
+ }
+ else /* EABI. */
+ {
+ svc_number = get_frame_register_unsigned (frame, 7);
+ }
+ }
+
+ is_sigreturn = arm_linux_sigreturn_return_addr (frame, svc_number,
+ &return_addr);
+
+ if (is_sigreturn)
+ return return_addr;
+
+ if (is_thumb)
+ {
+ return_addr = pc + 2;
+ /* Addresses for calling Thumb functions have the bit 0 set. */
+ return_addr |= 1;
+ }
+ else
+ {
+ return_addr = pc + 4;
+ }
+
+ return return_addr;
+}
+
+
/* Insert a single step breakpoint at the next executed instruction. */

static int
@@ -688,8 +764,11 @@ arm_linux_copy_svc (struct gdbarch *gdbarch, uint32_t insn, CORE_ADDR to,
struct regcache *regs, struct displaced_step_closure *dsc)
{
CORE_ADDR from = dsc->insn_addr;
+ CORE_ADDR return_to = 0;
+
struct frame_info *frame;
unsigned int svc_number = displaced_read_reg (regs, from, 7);
+ int is_sigreturn = 0;

if (debug_displaced)
fprintf_unfiltered (gdb_stdlog, "displaced: copying Linux svc insn %.8lx\n",
@@ -697,13 +776,10 @@ arm_linux_copy_svc (struct gdbarch *gdbarch, uint32_t insn, CORE_ADDR to,

frame = get_current_frame ();

- /* Is this a sigreturn or rt_sigreturn syscall? Note: these are only useful
- for EABI. */
- if (svc_number == 119 || svc_number == 173)
+ is_sigreturn = arm_linux_sigreturn_return_addr(frame, svc_number,
+ &return_to);
+ if (is_sigreturn)
{
- if (get_frame_type (frame) == SIGTRAMP_FRAME)
- {
- CORE_ADDR return_to;
struct symtab_and_line sal;

if (debug_displaced)
@@ -711,7 +787,6 @@ arm_linux_copy_svc (struct gdbarch *gdbarch, uint32_t insn, CORE_ADDR to,
"sigreturn/rt_sigreturn SVC call. PC in frame = %lx\n",
(unsigned long) get_frame_pc (frame));

- return_to = frame_unwind_caller_pc (frame);
if (debug_displaced)
fprintf_unfiltered (gdb_stdlog, "displaced: unwind pc = %lx. "
"Setting momentary breakpoint.\n", (unsigned long) return_to);
@@ -743,7 +818,7 @@ arm_linux_copy_svc (struct gdbarch *gdbarch, uint32_t insn, CORE_ADDR to,
else if (debug_displaced)
fprintf_unfiltered (gdb_stdlog, "displaced: sigreturn/rt_sigreturn "
"SVC call not in signal trampoline frame\n");
- }
+

/* Preparation: If we detect sigreturn, set momentary breakpoint at resume
location, else nothing.
@@ -946,6 +1021,9 @@ arm_linux_init_abi (struct gdbarch_info info,
set_gdbarch_displaced_step_free_closure (gdbarch,
simple_displaced_step_free_closure);
set_gdbarch_displaced_step_location (gdbarch, displaced_step_at_entry_point);
+
+
+ tdep->syscall_next_pc = arm_linux_syscall_next_pc;
}

/* Provide a prototype to silence -Wmissing-prototypes. */
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 1ac8817..36b46b5 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -257,7 +257,7 @@ int arm_apcs_32 = 1;

/* Determine if FRAME is executing in Thumb mode. */

-static int
+int
arm_frame_is_thumb (struct frame_info *frame)
{
CORE_ADDR cpsr;
@@ -2808,7 +2808,16 @@ thumb_get_next_pc_raw (struct frame_info *frame, CORE_ADDR pc, int insert_bkpt)
else if ((inst1 & 0xf000) == 0xd000) /* conditional branch */
{
unsigned long cond = bits (inst1, 8, 11);
- if (cond != 0x0f && condition_true (cond, status)) /* 0x0f = SWI */
+ if (cond == 0x0f) /* 0x0f = SWI */
+ {
+ struct gdbarch_tdep *tdep;
+ tdep = gdbarch_tdep (gdbarch);
+
+ if (tdep->syscall_next_pc != NULL)
+ nextpc = tdep->syscall_next_pc (frame);
+
+ }
+ else if (cond != 0x0f && condition_true (cond, status))
nextpc = pc_val + (sbits (inst1, 0, 7) << 1);
}
else if ((inst1 & 0xf800) == 0xe000) /* unconditional branch */
@@ -3257,7 +3266,16 @@ arm_get_next_pc_raw (struct frame_info *frame, CORE_ADDR pc, int insert_bkpt)
case 0xc:
case 0xd:
case 0xe: /* coproc ops */
+ break;
case 0xf: /* SWI */
+ {
+ struct gdbarch_tdep *tdep;
+ tdep = gdbarch_tdep (gdbarch);
+
+ if (tdep->syscall_next_pc != NULL)
+ nextpc = tdep->syscall_next_pc (frame);
+
+ }
break;

default:
diff --git a/gdb/arm-tdep.h b/gdb/arm-tdep.h
index b6283ef..2f4e99b 100644
--- a/gdb/arm-tdep.h
+++ b/gdb/arm-tdep.h
@@ -193,6 +193,10 @@ struct gdbarch_tdep
struct type *arm_ext_type;
struct type *neon_double_type;
struct type *neon_quad_type;
+
+ /* Return the expected next PC if FRAME is stopped at a syscall
+ instruction. */
+ CORE_ADDR (*syscall_next_pc) (struct frame_info *frame);
};

/* Structures used for displaced stepping. */
@@ -294,6 +298,7 @@ extern void
CORE_ADDR arm_skip_stub (struct frame_info *, CORE_ADDR);
CORE_ADDR arm_get_next_pc (struct frame_info *, CORE_ADDR);
int arm_software_single_step (struct frame_info *);
+int arm_frame_is_thumb (struct frame_info *frame);

extern struct displaced_step_closure *
arm_displaced_step_copy_insn (struct gdbarch *, CORE_ADDR, CORE_ADDR,