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

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

[Wei-cheng Wang]

2014-12-06  Wei-cheng Wang  <cole945@gmail.com>

	* ppc-linux-tdep.c (powerpc_linux_in_dynsym_resolve_code):
	Scan PLT stub backward for reverse debugging.
	(ppc_linux_init_abi): set powerpc_linux_in_dynsym_resolve_code
	for both 32-bit and 64-bit.

---
  gdb/ppc-linux-tdep.c | 35 ++++++++++++++++++++++++++---------
  1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
index 0a2afa7..f8971a3 100644
--- a/gdb/ppc-linux-tdep.c
+++ b/gdb/ppc-linux-tdep.c
@@ -51,6 +51,7 @@
  #include "linux-tdep.h"
  #include "linux-record.h"
  #include "record-full.h"
+#include "infrun.h"

  #include "stap-probe.h"
  #include "ax.h"
@@ -300,6 +301,22 @@ powerpc_linux_in_dynsym_resolve_code (CORE_ADDR pc)
  	  || strcmp (MSYMBOL_LINKAGE_NAME (sym.minsym),
  		     "__glink_PLTresolve") == 0))
      return 1;
+  else if (execution_direction == EXEC_REVERSE)
+    {
+      int i;
+      struct frame_info *frame = get_current_frame ();
+      struct gdbarch *gdbarch = get_frame_arch (frame);
+      const int plt_stub_len = 4;
+
+      /* Scan backward to chech whether we are in the middle
+	 of PLT stub.  */
+      for (i = 0; i < plt_stub_len; i++)
+	{
+	  if (gdbarch_skip_trampoline_code (gdbarch, frame, pc))
+	    return 1;
+	  pc -= 4;
+	}
+    }

    return 0;
  }
@@ -1698,15 +1715,6 @@ ppc_linux_init_abi (struct gdbarch_info info,
        else
  	set_gdbarch_gcore_bfd_target (gdbarch, "elf32-powerpc");

-      if (powerpc_so_ops.in_dynsym_resolve_code == NULL)
-	{
-	  powerpc_so_ops = svr4_so_ops;
-	  /* Override dynamic resolve function.  */
-	  powerpc_so_ops.in_dynsym_resolve_code =
-	    powerpc_linux_in_dynsym_resolve_code;
-	}
-      set_solib_ops (gdbarch, &powerpc_so_ops);
-
        set_gdbarch_skip_solib_resolver (gdbarch, glibc_skip_solib_resolver);
      }

@@ -1751,6 +1759,15 @@ ppc_linux_init_abi (struct gdbarch_info info,
  	set_gdbarch_gcore_bfd_target (gdbarch, "elf64-powerpc");
      }

+  if (powerpc_so_ops.in_dynsym_resolve_code == NULL)
+    {
+      powerpc_so_ops = svr4_so_ops;
+      /* Override dynamic resolve function.  */
+      powerpc_so_ops.in_dynsym_resolve_code =
+	powerpc_linux_in_dynsym_resolve_code;
+    }
+  set_solib_ops (gdbarch, &powerpc_so_ops);
+
    /* PPC32 uses a different prpsinfo32 compared to most other Linux
       archs.  */
    if (tdep->wordsize == 4)
-- 
1.9.1

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

[Ulrich Weigand]

Wei-cheng Wang wrote:

> 	* ppc-linux-tdep.c (powerpc_linux_in_dynsym_resolve_code):
> 	Scan PLT stub backward for reverse debugging.
> 	(ppc_linux_init_abi): set powerpc_linux_in_dynsym_resolve_code
> 	for both 32-bit and 64-bit.

As I said in the reply to the 0/3 mail, I really think it would be
better to handle this within the PPC skip_trampoline_code implementation,
instead of having in_dynsym_resolve_code suddenly also cover the
trampolines ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

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

[Wei-cheng Wang]

On 2014/12/9 上午 03:13, Ulrich Weigand wrote:
> Wei-cheng Wang wrote:
>> 	* ppc-linux-tdep.c (powerpc_linux_in_dynsym_resolve_code):
>> 	Scan PLT stub backward for reverse debugging.
>> 	(ppc_linux_init_abi): set powerpc_linux_in_dynsym_resolve_code
>> 	for both 32-bit and 64-bit.
>
> As I said in the reply to the 0/3 mail, I really think it would be
> better to handle this within the PPC skip_trampoline_code implementation,
> instead of having in_dynsym_resolve_code suddenly also cover the
> trampolines ...

   Hi,

   See the new patch, I moved the for-loop into skip_trampoline_code,
   and removed in_dynsym_resolve_code.

> It seems odd to have in_dynsym_resolve_code call into
> skip_trampoline_code.  Is there a reason why the skip_trampoline_code
> implementation cannot accept a PC in the middle of the sequence?

   I thought skip-trampoline is used to find the target address for
   inserting step-resume breakpoint, so it doesn't make sense for
   reverse-stepping, because we are just stepping from the target address.
   And for forward-stepping, when we reach the very first instruction of
   trampoline code, we can just skip the resolving code by inserting
   a step-resume breakpoint, so we don't have to step through the resolving
   code and check whether we are in the middle of trampoline code.

   in_dynsym_resolve_code is used to check whether we are in (the middle of)
   the resolving code, so we can keep going to step though the resolving code.
   Therefor I thought in_dynsym_resolve_code is the proper place to implement.

   Since skip-trampoline-code had implemented the plt-stub pattern-match code
   we need, by calling it in in-dynsym-resolve-code, we don't need to have
   duplicate pattern-match code in both functions.

Wei-cheng,
Thanks

--

2014-12-06  Wei-cheng Wang  <cole945@gmail.com>

	* ppc-linux-tdep.c (ppc_skip_trampoline_code):
	Scan PLT stub backward for reverse debugging.
	* ppc64-tdep.c (ppc64_skip_trampoline_code): Likewise.


---
  gdb/ppc-linux-tdep.c | 58 ++++++++++++++++++++++------------
  gdb/ppc64-tdep.c     | 88 +++++++++++++++++++++++++++++++++++-----------------
  2 files changed, 99 insertions(+), 47 deletions(-)

diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
index fa41d34..b57f41d 100644
--- a/gdb/ppc-linux-tdep.c
+++ b/gdb/ppc-linux-tdep.c
@@ -51,6 +51,7 @@
  #include "linux-tdep.h"
  #include "linux-record.h"
  #include "record-full.h"
+#include "infrun.h"

  #include "stap-probe.h"
  #include "ax.h"
@@ -314,31 +315,50 @@ ppc_skip_trampoline_code (struct frame_info *frame, CORE_ADDR pc)
    struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
    enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
    CORE_ADDR target = 0;
+  int scan_limit, i;

-  if (ppc_insns_match_pattern (frame, pc, powerpc32_plt_stub, insnbuf))
-    {
-      /* Insn pattern is
-		lis   r11, xxxx
-		lwz   r11, xxxx(r11)
-	 Branch target is in r11.  */
-
-      target = (ppc_insn_d_field (insnbuf[0]) << 16)
-	| ppc_insn_d_field (insnbuf[1]);
-      target = read_memory_unsigned_integer (target, 4, byte_order);
-    }
+  scan_limit = 1;
+  /* When reverse-debugging, scan backward to check whether we are
+     in the middle of trampoline code.  */
+  if (execution_direction == EXEC_REVERSE)
+    scan_limit = 4;	/* At more 4 instructions.  */

-  if (ppc_insns_match_pattern (frame, pc, powerpc32_plt_stub_so, insnbuf))
+  for (i = 0; i < scan_limit; i++)
      {
-      /* Insn pattern is
-		lwz   r11, xxxx(r30)
-	 Branch target is in r11.  */
+      if (ppc_insns_match_pattern (frame, pc, powerpc32_plt_stub, insnbuf))
+	{
+	  /* Insn pattern is
+	     lis   r11, xxxx
+	     lwz   r11, xxxx(r11)
+	     Branch target is in r11.  */
+
+	  target = (ppc_insn_d_field (insnbuf[0]) << 16)
+		   | ppc_insn_d_field (insnbuf[1]);
+	  target = read_memory_unsigned_integer (target, 4, byte_order);
+	}
+      else if (ppc_insns_match_pattern (frame, pc, powerpc32_plt_stub_so,
+					insnbuf))
+	{
+	  /* Insn pattern is
+	     lwz   r11, xxxx(r30)
+	     Branch target is in r11.  */
+
+	  target = get_frame_register_unsigned (frame,
+						tdep->ppc_gp0_regnum + 30)
+		   + ppc_insn_d_field (insnbuf[0]);
+	  target = read_memory_unsigned_integer (target, 4, byte_order);
+	}
+      else
+	{
+	  /* Scan backward one more instructions if doesn't match.  */
+	  pc -= 4;
+	  continue;
+	}

-      target = get_frame_register_unsigned (frame, tdep->ppc_gp0_regnum + 30)
-	       + ppc_insn_d_field (insnbuf[0]);
-      target = read_memory_unsigned_integer (target, 4, byte_order);
+      return target;
      }

-  return target;
+  return 0;
  }

  /* Wrappers to handle Linux-only registers.  */
diff --git a/gdb/ppc64-tdep.c b/gdb/ppc64-tdep.c
index 8acd754..25a0816 100644
--- a/gdb/ppc64-tdep.c
+++ b/gdb/ppc64-tdep.c
@@ -20,6 +20,7 @@
  #include "defs.h"
  #include "frame.h"
  #include "gdbcore.h"
+#include "infrun.h"
  #include "ppc-tdep.h"
  #include "ppc64-tdep.h"
  #include "elf-bfd.h"
@@ -464,35 +465,66 @@ ppc64_skip_trampoline_code (struct frame_info *frame, CORE_ADDR pc)
  				    ARRAY_SIZE (ppc64_standard_linkage8))))
  		     - 1];
    CORE_ADDR target;
+  int scan_limit, i;

-  if (ppc_insns_match_pattern (frame, pc, ppc64_standard_linkage8, insns))
-    pc = ppc64_standard_linkage4_target (frame, pc, insns);
-  else if (ppc_insns_match_pattern (frame, pc, ppc64_standard_linkage7, insns))
-    pc = ppc64_standard_linkage3_target (frame, pc, insns);
-  else if (ppc_insns_match_pattern (frame, pc, ppc64_standard_linkage6, insns))
-    pc = ppc64_standard_linkage4_target (frame, pc, insns);
-  else if (ppc_insns_match_pattern (frame, pc, ppc64_standard_linkage5, insns)
-	   && (insns[8] != 0 || insns[9] != 0))
-    pc = ppc64_standard_linkage3_target (frame, pc, insns);
-  else if (ppc_insns_match_pattern (frame, pc, ppc64_standard_linkage4, insns)
-	   && (insns[9] != 0 || insns[10] != 0))
-    pc = ppc64_standard_linkage4_target (frame, pc, insns);
-  else if (ppc_insns_match_pattern (frame, pc, ppc64_standard_linkage3, insns)
-	   && (insns[8] != 0 || insns[9] != 0))
-    pc = ppc64_standard_linkage3_target (frame, pc, insns);
-  else if (ppc_insns_match_pattern (frame, pc, ppc64_standard_linkage2, insns)
-	   && (insns[10] != 0 || insns[11] != 0))
-    pc = ppc64_standard_linkage2_target (frame, pc, insns);
-  else if (ppc_insns_match_pattern (frame, pc, ppc64_standard_linkage1, insns))
-    pc = ppc64_standard_linkage1_target (frame, pc, insns);
-  else
-    return 0;
-
-  /* The PLT descriptor will either point to the already resolved target
-     address, or else to a glink stub.  As the latter carry synthetic @plt
-     symbols, find_solib_trampoline_target should be able to resolve them.  */
-  target = find_solib_trampoline_target (frame, pc);
-  return target ? target : pc;
+  scan_limit = 1;
+  /* When reverse-debugging, scan backward to check whether we are
+     in the middle of trampoline code.  */
+  if (execution_direction == EXEC_REVERSE)
+    scan_limit = ARRAY_SIZE (insns) - 1;
+
+  for (i = 0; i < scan_limit; i++)
+    {
+      if (i < ARRAY_SIZE (ppc64_standard_linkage8) - 1
+	  && ppc_insns_match_pattern (frame, pc, ppc64_standard_linkage8, insns))
+	pc = ppc64_standard_linkage4_target (frame, pc, insns);
+      else if (i < ARRAY_SIZE (ppc64_standard_linkage7) - 1
+	       && ppc_insns_match_pattern (frame, pc, ppc64_standard_linkage7,
+					   insns))
+	pc = ppc64_standard_linkage3_target (frame, pc, insns);
+      else if (i < ARRAY_SIZE (ppc64_standard_linkage6) - 1
+	       && ppc_insns_match_pattern (frame, pc, ppc64_standard_linkage6,
+					   insns))
+	pc = ppc64_standard_linkage4_target (frame, pc, insns);
+      else if (i < ARRAY_SIZE (ppc64_standard_linkage5) - 1
+	       && ppc_insns_match_pattern (frame, pc, ppc64_standard_linkage5,
+					   insns)
+	       && (insns[8] != 0 || insns[9] != 0))
+	pc = ppc64_standard_linkage3_target (frame, pc, insns);
+      else if (i < ARRAY_SIZE (ppc64_standard_linkage4) - 1
+	       && ppc_insns_match_pattern (frame, pc, ppc64_standard_linkage4,
+					   insns)
+	       && (insns[9] != 0 || insns[10] != 0))
+	pc = ppc64_standard_linkage4_target (frame, pc, insns);
+      else if (i < ARRAY_SIZE (ppc64_standard_linkage3) - 1
+	       && ppc_insns_match_pattern (frame, pc, ppc64_standard_linkage3,
+					   insns)
+	       && (insns[8] != 0 || insns[9] != 0))
+	pc = ppc64_standard_linkage3_target (frame, pc, insns);
+      else if (i < ARRAY_SIZE (ppc64_standard_linkage2) - 1
+	       && ppc_insns_match_pattern (frame, pc, ppc64_standard_linkage2,
+					   insns)
+	       && (insns[10] != 0 || insns[11] != 0))
+	pc = ppc64_standard_linkage2_target (frame, pc, insns);
+      else if (i < ARRAY_SIZE (ppc64_standard_linkage1) - 1
+	       && ppc_insns_match_pattern (frame, pc, ppc64_standard_linkage1,
+					   insns))
+	pc = ppc64_standard_linkage1_target (frame, pc, insns);
+      else
+	{
+	  /* Scan backward one more instructions if doesn't match.  */
+	  pc -= 4;
+	  continue;
+	}
+
+      /* The PLT descriptor will either point to the already resolved target
+         address, or else to a glink stub.  As the latter carry synthetic @plt
+         symbols, find_solib_trampoline_target should be able to resolve them.  */
+      target = find_solib_trampoline_target (frame, pc);
+      return target ? target : pc;
+  }
+
+  return 0;
  }

  /* Support for convert_from_func_ptr_addr (ARCH, ADDR, TARG) on PPC64
-- 
1.9.1

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

[Ulrich Weigand]

Wei-cheng Wang wrote:

>    I thought skip-trampoline is used to find the target address for
>    inserting step-resume breakpoint, so it doesn't make sense for
>    reverse-stepping, because we are just stepping from the target address.
>    And for forward-stepping, when we reach the very first instruction of
>    trampoline code, we can just skip the resolving code by inserting
>    a step-resume breakpoint, so we don't have to step through the resolving
>    code and check whether we are in the middle of trampoline code.
> 
>    in_dynsym_resolve_code is used to check whether we are in (the middle of)
>    the resolving code, so we can keep going to step though the resolving code.
>    Therefor I thought in_dynsym_resolve_code is the proper place to implement.
> 
>    Since skip-trampoline-code had implemented the plt-stub pattern-match code
>    we need, by calling it in in-dynsym-resolve-code, we don't need to have
>    duplicate pattern-match code in both functions.

Well, on other platforms skip_trampoline also works when the PC is in the
middle of the trampoline instead of the beginning, so it doesn't hurt anyway
to make PowerPC match that behavior.  In general, skip_trampoline is supposed
to handle the linker-generated short stubs, while in_dynsym_resolve_code is
supposed to handle the main dynamic linker code itself.  Of course that's
not always a strict distinction, but I still prefer this second patch.

> 2014-12-06  Wei-cheng Wang  <cole945@gmail.com>
> 
> 	* ppc-linux-tdep.c (ppc_skip_trampoline_code):
> 	Scan PLT stub backward for reverse debugging.
> 	* ppc64-tdep.c (ppc64_skip_trampoline_code): Likewise.

This is OK.

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com