This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v6 3/7] Refactor arm_software_single_step to use regcache.




On 12/07/2015 09:32 AM, Yao Qi wrote:
Antoine Tremblay <antoine.tremblay@ericsson.com> writes:

+/* Calculate the offset from stack pointer of the pc register on the stack
+   in the case of a sigreturn or sigreturn_rt syscall.  */
+static int
+arm_linux_sigreturn_next_pc_offset (unsigned long sp,
+				    unsigned long sp_data,
+				    unsigned long svc_number)
+{
+  /* Offset of R0 register.  */
+  int r0_offset = 0;
+  /* Offset of PC register.  */
+  int pc_offset = 0;
+
+  gdb_assert (svc_number == 119 || svc_number == 173);
+
+  /* sigreturn.  */
+  if (svc_number == 119)

Can we get rid of these magic numbers?


Yes I can find a place to put a define, good idea.

+    {
+      if (sp_data == ARM_NEW_SIGFRAME_MAGIC)
+	r0_offset = ARM_UCONTEXT_SIGCONTEXT + ARM_SIGCONTEXT_R0;
+      else
+	r0_offset = ARM_SIGCONTEXT_R0;
+    }
+  /* rt_sigreturn. */
+  else if (svc_number == 173)
+    {
+      if (sp_data == sp + ARM_OLD_RT_SIGFRAME_SIGINFO)
+	r0_offset = ARM_OLD_RT_SIGFRAME_UCONTEXT + ARM_UCONTEXT_SIGCONTEXT
+	  + ARM_SIGCONTEXT_R0;
+      else
+	r0_offset = ARM_NEW_RT_SIGFRAME_UCONTEXT + ARM_UCONTEXT_SIGCONTEXT
+	  + ARM_SIGCONTEXT_R0;
+    }
+
+  pc_offset = r0_offset + 4 * 15;
+
+  return pc_offset;
+}
+

+
  /* At a ptrace syscall-stop, return the syscall number.  This either
     comes from the SWI instruction (OABI) or from r7 (EABI).

@@ -862,21 +924,21 @@ arm_linux_get_syscall_number (struct gdbarch *gdbarch,
     instruction to be executed.  */

  static CORE_ADDR
-arm_linux_syscall_next_pc (struct frame_info *frame)
+arm_linux_syscall_next_pc (struct regcache *regcache)
  {
-  CORE_ADDR pc = get_frame_pc (frame);
-  CORE_ADDR return_addr = 0;
-  int is_thumb = arm_frame_is_thumb (frame);
+  CORE_ADDR pc = regcache_read_pc (regcache);
+  CORE_ADDR next_pc = 0;
+  int is_thumb = arm_is_thumb (regcache);

Nit: looks you rename return_addr to next_pc.  If you don't that, the patch
can be shorter.  On the other hand, return_addr sounds a good variable
name to me, which means the address after syscall returns.


Yes, I do since the function is called arm_linux_syscall*_next_pc*
and that is called from the get_next_pcs context. It seems more consistent to me to refer to this address as the next program counter in that context.

Also I wanted to mark the difference between arm_linux_sigreturn_return_addr and arm_linux_sigreturn_next_pc.

Calling return_addr = arm_linux_sigreturn_next_pc seemed weird when there was return_addr = arm_linux_sigreturn_return_addr before.

In short I prefer to keep the function name consistent with the return value name.

Regards,
Antoine


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]