Re: [PATCH 2/3 v4] Process record support for PowerPC

[Wei-cheng Wang <cole945 at gmail dot com>]

On 2015/1/19 äå 03:48, Joel Brobecker wrote:
> > Using %s+paddress instead of %lx+long in this patch.
> The change looks fine to me, but there is no ChangeLog.

Hi Joel,

ChangeLog is in my last mail.  Re-post below for your review.

2015-01-18  Wei-cheng Wang  <cole945@gmail.com>

	* rs6000-tdep.c (ppc_process_record_op4, ppc_process_record_op19,
	ppc_process_record_op31, ppc_process_record_op59,
	ppc_process_record_op60, ppc_process_record_op63,
	ppc_process_record): Fix -Wformat warning.
	* rs6000-tdep.c (rs6000_epilogue_frame_cache, ppc_process_record_op60):
	Remove unused variables.

> Also, I ended glancing at some of the patches you recently pushed,
> and noticed that you added a lot of new functions for which there
> is no introductory comment describing the function's behavior and
> arguments. It is now mandatory that all new functions, no matter
> how trivial, have such introductory comment. Would you mind going
> back over your patches, and send a patch that adds the missing
> comments, please?

I've added the missing comments.  See below.

Thanks,
Wei-cheng


2015-01-18  Wei-cheng Wang  <cole945@gmail.com>

	* ppc-linux-tdep.c (ppc_skip_trampoline_code,
	ppc_canonicalize_syscall, ppc_linux_syscall_record,
	ppc_linux_record_signal, ppc_init_linux_record_tdep): Add comments.
	* ppc64-tdep.c (ppc64_skip_trampoline_code): Likewise.
	* rs6000-tdep.c (rs6000_epilogue_frame_cache,
	rs6000_epilogue_frame_this_id, rs6000_epilogue_frame_prev_register,
	rs6000_epilogue_frame_sniffer, ppc_record_vsr, ppc_process_record_op4,
	ppc_process_record_op19, ppc_process_record_op31,
	ppc_process_record_op59, ppc_process_record_op60,
	ppc_process_record_op63): Likewise.


From 26f1310335aace089f686a11a5e100ad166d4235 Mon Sep 17 00:00:00 2001
From: Wei-cheng Wang <cole945@gmail.com>
Date: Mon, 19 Jan 2015 23:34:07 +0800
Subject: [PATCH 2/2] Add missing comments in rs6000-tdep.c, ppc64-tdep.c and
 ppc-linux-tdep.c.

---
 gdb/ppc-linux-tdep.c | 26 ++++++++++++++++++++++----
 gdb/ppc64-tdep.c     |  5 ++++-
 gdb/rs6000-tdep.c    | 39 +++++++++++++++++++++++++++++++--------
 4 files changed, 70 insertions(+), 13 deletions(-)

diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
index 13bb479..4a37a27 100644
--- a/gdb/ppc-linux-tdep.c
+++ b/gdb/ppc-linux-tdep.c
@@ -305,7 +305,12 @@ powerpc_linux_in_dynsym_resolve_code (CORE_ADDR pc)
   return 0;
 }

-/* Follow PLT stub to actual routine.  */
+/* Follow PLT stub to actual routine.
+
+   When the execution direction is EXEC_REVERSE, scan backward to
+   check whether we are in the middle of a PLT stub.  Currently,
+   we only look-behind at most 4 instructions (the max length of PLT
+   stub sequence.  */

 static CORE_ADDR
 ppc_skip_trampoline_code (struct frame_info *frame, CORE_ADDR pc)
@@ -791,11 +796,16 @@ ppc_linux_get_syscall_number (struct gdbarch *gdbarch,
 static struct linux_record_tdep ppc_linux_record_tdep;
 static struct linux_record_tdep ppc64_linux_record_tdep;

+/* ppc_canonicalize_syscall maps from the native PowerPC Linux set of
+   syscall ids into a canonical set of syscall ids used by process
+   record.  (See arch/powerpc/include/uapi/asm/unistd.h in kernel tree.)
+   Return -1 if this system call is not supported by process record.
+   Otherwise, return the syscall number for preocess reocrd of given
+   SYSCALL.  */
+
 static enum gdb_syscall
 ppc_canonicalize_syscall (int syscall)
 {
-  /* See arch/powerpc/include/uapi/asm/unistd.h */
-
   if (syscall <= 165)
     return syscall;
   else if (syscall >= 167 && syscall <= 190)	/* Skip query_module 166 */
@@ -821,6 +831,9 @@ ppc_canonicalize_syscall (int syscall)
   return -1;
 }

+/* Record registers which might be clobbered during system call.
+   Return 0 if succeed.  */
+
 static int
 ppc_linux_syscall_record (struct regcache *regcache)
 {
@@ -901,6 +914,9 @@ ppc_linux_syscall_record (struct regcache *regcache)
   return 0;
 }

+/* Record registers which might be clobbered during sinal handling.
+   Return 0 if succeed.  */
+
 static int
 ppc_linux_record_signal (struct gdbarch *gdbarch, struct regcache *regcache,
 			 enum gdb_signal signal)
@@ -1417,7 +1433,9 @@ static const struct frame_unwind ppu2spu_unwind = {
   ppu2spu_prev_arch,
 };

-/* Initialize linux_record_tdep if not initialized yet.  */
+/* Initialize linux_record_tdep if not initialized yet.
+   WORDSIZE is 4 or 8 for 32- or 64-bit PowerPC Linux respectively.
+   Sizes of data structures are initilzied accordingly.  */

 static void
 ppc_init_linux_record_tdep (struct linux_record_tdep *record_tdep,
diff --git a/gdb/ppc64-tdep.c b/gdb/ppc64-tdep.c
index 66d7b23..bb23b6a 100644
--- a/gdb/ppc64-tdep.c
+++ b/gdb/ppc64-tdep.c
@@ -449,7 +449,10 @@ ppc64_standard_linkage4_target (struct frame_info *frame,


 /* Given that we've begun executing a call trampoline at PC, return
-   the entry point of the function the trampoline will go to.  */
+   the entry point of the function the trampoline will go to.
+
+   When the execution direction is EXEC_REVERSE, scan backward to
+   check whether we are in the middle of a PLT stub.  */

 CORE_ADDR
 ppc64_skip_trampoline_code (struct frame_info *frame, CORE_ADDR pc)
diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index 1dd6165..0a24881 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -3349,6 +3349,9 @@ static const struct frame_unwind rs6000_frame_unwind =
   default_frame_sniffer
 };

+/* Allocate and initialize a frame cache for an epilogue frame.
+   SP is restored and prev-PC is stored in LR.  */
+
 static struct rs6000_frame_cache *
 rs6000_epilogue_frame_cache (struct frame_info *this_frame, void **this_cache)
 {
@@ -3385,6 +3388,9 @@ rs6000_epilogue_frame_cache (struct frame_info *this_frame, void **this_cache)
   return cache;
 }

+/* Implementation of frame_unwind.this_id, as defined in frame_unwind.h.
+   Return the frame ID of an epilogue frame.  */
+
 static void
 rs6000_epilogue_frame_this_id (struct frame_info *this_frame,
 			       void **this_cache, struct frame_id *this_id)
@@ -3400,6 +3406,9 @@ rs6000_epilogue_frame_this_id (struct frame_info *this_frame,
     (*this_id) = frame_id_build (info->base, pc);
 }

+/* Implementation of frame_unwind.prev_register, as defined in frame_unwind.h.
+   Return the register value of REGNUM in previous frame.  */
+
 static struct value *
 rs6000_epilogue_frame_prev_register (struct frame_info *this_frame,
 				     void **this_cache, int regnum)
@@ -3409,6 +3418,9 @@ rs6000_epilogue_frame_prev_register (struct frame_info *this_frame,
   return trad_frame_get_prev_register (this_frame, info->saved_regs, regnum);
 }

+/* Implementation of frame_unwind.sniffer, as defined in frame_unwind.h.
+   Check whether this an epilogue frame.  */
+
 static int
 rs6000_epilogue_frame_sniffer (const struct frame_unwind *self,
 			       struct frame_info *this_frame,
@@ -3422,6 +3434,9 @@ rs6000_epilogue_frame_sniffer (const struct frame_unwind *self,
     return 0;
 }

+/* Frame unwinder for epilogue frame.  This is required for reverse step-over
+   a function without debug information.  */
+
 static const struct frame_unwind rs6000_epilogue_frame_unwind =
 {
   NORMAL_FRAME,
@@ -3668,7 +3683,9 @@ bfd_uses_spe_extensions (bfd *abfd)
 #define PPC_XT(insn)	((PPC_TX (insn) << 5) | PPC_T (insn))
 #define PPC_XER_NB(xer)	(xer & 0x7f)

-/* Record Vector-Scalar Registers.  */
+/* Record Vector-Scalar Registers.
+   For VSR less than 32, it's represented by an FPR and an VSR-upper register.
+   Otherwise, it's just a VR register.  Record them accordingly.  */

 static int
 ppc_record_vsr (struct regcache *regcache, struct gdbarch_tdep *tdep, int vsr)
@@ -3693,11 +3710,12 @@ ppc_record_vsr (struct regcache *regcache, struct gdbarch_tdep *tdep, int vsr)
   return 0;
 }

-/* Parse instructions of primary opcode-4.  */
+/* Parse and record instructions primary opcode-4 at ADDR.
+   Return 0 if succeed.  */

 static int
 ppc_process_record_op4 (struct gdbarch *gdbarch, struct regcache *regcache,
-			   CORE_ADDR addr, uint32_t insn)
+			CORE_ADDR addr, uint32_t insn)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   int ext = PPC_FIELD (insn, 21, 11);
@@ -3961,7 +3979,8 @@ ppc_process_record_op4 (struct gdbarch *gdbarch, struct regcache *regcache,
   return -1;
 }

-/* Parse instructions of primary opcode-19.  */
+/* Parse and record instructions of primary opcode-19 at ADDR.
+   Return 0 if succeed.  */

 static int
 ppc_process_record_op19 (struct gdbarch *gdbarch, struct regcache *regcache,
@@ -4004,7 +4023,8 @@ ppc_process_record_op19 (struct gdbarch *gdbarch, struct regcache *regcache,
   return -1;
 }

-/* Parse instructions of primary opcode-31.  */
+/* Parse and record instructions of primary opcode-31 at ADDR.
+   Return 0 if succeed.  */

 static int
 ppc_process_record_op31 (struct gdbarch *gdbarch, struct regcache *regcache,
@@ -4482,7 +4502,8 @@ UNKNOWN_OP:
   return -1;
 }

-/* Parse instructions of primary opcode-59.  */
+/* Parse and record instructions of primary opcode-59 at ADDR.
+   Return 0 if succeed.  */

 static int
 ppc_process_record_op59 (struct gdbarch *gdbarch, struct regcache *regcache,
@@ -4574,7 +4595,8 @@ ppc_process_record_op59 (struct gdbarch *gdbarch, struct regcache *regcache,
   return -1;
 }

-/* Parse instructions of primary opcode-60.  */
+/* Parse and record instructions of primary opcode-60 at ADDR.
+   Return 0 if succeed.  */

 static int
 ppc_process_record_op60 (struct gdbarch *gdbarch, struct regcache *regcache,
@@ -4856,7 +4878,8 @@ ppc_process_record_op60 (struct gdbarch *gdbarch, struct regcache *regcache,
   return -1;
 }

-/* Parse instructions of primary opcode-63.  */
+/* Parse and record instructions of primary opcode-63 at ADDR.
+   Return 0 if succeed.  */

 static int
 ppc_process_record_op63 (struct gdbarch *gdbarch, struct regcache *regcache,
--
1.9.1