diff mbox

PowerPC PLT stub matching

Message ID 20180124062015.GN20622@bubble.grove.modra.org
State New
Headers show

Commit Message

Alan Modra Jan. 24, 2018, 6:20 a.m. UTC
This patch fixes a number of bugs in ppc32 plt stub matching code.
1) The 4-insn stub for shared libs and PIEs wasn't matched.
2) The executable stub miscalculated PLT entry address by oring a
   sign-extended quantity rather than adding.
3) Comments were not accurate.

In addition, the insn arrays are made const.  Regression tested
powerpc64-linux, powerpc-linux and powerpc64le-linux.  OK?

	* ppc-linux-tdep.c (powerpc32_plt_stub): Make const.
	(powerpc32_plt_stub_so_1): Rename from powerpc32_plt_stub_so.
	Remove nop.  Make const.  Comment.
	(powerpc32_plt_stub_so_2): New.
	(POWERPC32_PLT_CHECK_LEN): Rename from POWERPC32_PLT_STUB_LEN.
	Correct count.  Update uses.
	(ppc_skip_trampoline_code): Match powerpc32_plt_stub_so_2 too.
	Move common code reading PLT entry word.  Correct
	powerpc32_plt_stub PLT address calculation.
	* ppc64-tdep.c (ppc64_standard_linkage1): Make const.
	(ppc64_standard_linkage2, ppc64_standard_linkage3): Likewise.
	(ppc64_standard_linkage4, ppc64_standard_linkage5): Likewise.
	(ppc64_standard_linkage6, ppc64_standard_linkage7): Likewise.
	(ppc64_standard_linkage8): Likewise.
	* rs6000-tdep.c (ppc_insns_match_pattern): Make pattern const.
	Correct insns description.
	* ppc-tdep.h (ppc_insns_match_pattern): Update prototype.

Comments

Yao Qi Jan. 24, 2018, 4:06 p.m. UTC | #1
Alan Modra <amodra@gmail.com> writes:


Hi Alan,
Thanks for the clean up, patch is good to me.  Please push.

A general question below,

> -/* PLT stub in executable.  */
> -static struct ppc_insn_pattern powerpc32_plt_stub[] =
> +/* PLT stub in an executable.  */
> +static const struct ppc_insn_pattern powerpc32_plt_stub[] =
>    {
>      { 0xffff0000, 0x3d600000, 0 },	/* lis   r11, xxxx	 */
>      { 0xffff0000, 0x816b0000, 0 },	/* lwz   r11, xxxx(r11)  */
> @@ -266,16 +266,30 @@ static struct ppc_insn_pattern powerpc32_plt_stub[] =
>      {          0,          0, 0 }
>    };
>  
> -/* PLT stub in shared library.  */
> -static struct ppc_insn_pattern powerpc32_plt_stub_so[] =
> +/* PLT stubs in a shared library or PIE.
> +   The first variant is used when the PLT entry is within +/-32k of
> +   the GOT pointer (r30).  */
> +static const struct ppc_insn_pattern powerpc32_plt_stub_so_1[] =
>    {
>      { 0xffff0000, 0x817e0000, 0 },	/* lwz   r11, xxxx(r30)  */
>      { 0xffffffff, 0x7d6903a6, 0 },	/* mtctr r11		 */
>      { 0xffffffff, 0x4e800420, 0 },	/* bctr			 */
> -    { 0xffffffff, 0x60000000, 0 },	/* nop			 */
>      {          0,          0, 0 }
>    };
> -#define POWERPC32_PLT_STUB_LEN 	ARRAY_SIZE (powerpc32_plt_stub)
> +
> +/* The second variant is used when the PLT entry is more than +/-32k
> +   from the GOT pointer (r30).  */
> +static const struct ppc_insn_pattern powerpc32_plt_stub_so_2[] =
> +  {
> +    { 0xffff0000, 0x3d7e0000, 0 },	/* addis r11, r30, xxxx  */
> +    { 0xffff0000, 0x816b0000, 0 },	/* lwz   r11, xxxx(r11)  */
> +    { 0xffffffff, 0x7d6903a6, 0 },	/* mtctr r11		 */
> +    { 0xffffffff, 0x4e800420, 0 },	/* bctr			 */
> +    {          0,          0, 0 }
> +  };

These instruction sequences reflect what
bfd/elf32-ppc.c:write_glink_stub does, IIUC.  Is there any way we can
correlate these two sides, PLT stub generation and matching?  Ideally, I
want to such sequences are defined once, but shared by linker and debugger.
Alan Modra Jan. 26, 2018, 5:42 a.m. UTC | #2
On Wed, Jan 24, 2018 at 04:06:02PM +0000, Yao Qi wrote:
> These instruction sequences reflect what
> bfd/elf32-ppc.c:write_glink_stub does, IIUC.

Yes, and gold/powerpc.cc:Stub_table::do_write.

>  Is there any way we can
> correlate these two sides, PLT stub generation and matching?  Ideally, I
> want to such sequences are defined once, but shared by linker and debugger.

It might be possible, but I think the resulting data structure and
code would be a mess, particularly for ppc64 given the number of
variations of PLT stubs.

Thanks for the review!  Committed.
diff mbox

Patch

diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
index bbe9e89..ed0ea13 100644
--- a/gdb/ppc-linux-tdep.c
+++ b/gdb/ppc-linux-tdep.c
@@ -256,8 +256,8 @@  ppc_linux_return_value (struct gdbarch *gdbarch, struct value *function,
 				      readbuf, writebuf);
 }
 
-/* PLT stub in executable.  */
-static struct ppc_insn_pattern powerpc32_plt_stub[] =
+/* PLT stub in an executable.  */
+static const struct ppc_insn_pattern powerpc32_plt_stub[] =
   {
     { 0xffff0000, 0x3d600000, 0 },	/* lis   r11, xxxx	 */
     { 0xffff0000, 0x816b0000, 0 },	/* lwz   r11, xxxx(r11)  */
@@ -266,16 +266,30 @@  static struct ppc_insn_pattern powerpc32_plt_stub[] =
     {          0,          0, 0 }
   };
 
-/* PLT stub in shared library.  */
-static struct ppc_insn_pattern powerpc32_plt_stub_so[] =
+/* PLT stubs in a shared library or PIE.
+   The first variant is used when the PLT entry is within +/-32k of
+   the GOT pointer (r30).  */
+static const struct ppc_insn_pattern powerpc32_plt_stub_so_1[] =
   {
     { 0xffff0000, 0x817e0000, 0 },	/* lwz   r11, xxxx(r30)  */
     { 0xffffffff, 0x7d6903a6, 0 },	/* mtctr r11		 */
     { 0xffffffff, 0x4e800420, 0 },	/* bctr			 */
-    { 0xffffffff, 0x60000000, 0 },	/* nop			 */
     {          0,          0, 0 }
   };
-#define POWERPC32_PLT_STUB_LEN 	ARRAY_SIZE (powerpc32_plt_stub)
+
+/* The second variant is used when the PLT entry is more than +/-32k
+   from the GOT pointer (r30).  */
+static const struct ppc_insn_pattern powerpc32_plt_stub_so_2[] =
+  {
+    { 0xffff0000, 0x3d7e0000, 0 },	/* addis r11, r30, xxxx  */
+    { 0xffff0000, 0x816b0000, 0 },	/* lwz   r11, xxxx(r11)  */
+    { 0xffffffff, 0x7d6903a6, 0 },	/* mtctr r11		 */
+    { 0xffffffff, 0x4e800420, 0 },	/* bctr			 */
+    {          0,          0, 0 }
+  };
+
+/* The max number of insns we check using ppc_insns_match_pattern.  */
+#define POWERPC32_PLT_CHECK_LEN (ARRAY_SIZE (powerpc32_plt_stub) - 1)
 
 /* Check if PC is in PLT stub.  For non-secure PLT, stub is in .plt
    section.  For secure PLT, stub is in .text and we need to check
@@ -306,13 +320,13 @@  powerpc_linux_in_dynsym_resolve_code (CORE_ADDR pc)
 
    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
+   we only look-behind at most 4 instructions (the max length of a PLT
    stub sequence.  */
 
 static CORE_ADDR
 ppc_skip_trampoline_code (struct frame_info *frame, CORE_ADDR pc)
 {
-  unsigned int insnbuf[POWERPC32_PLT_STUB_LEN];
+  unsigned int insnbuf[POWERPC32_PLT_CHECK_LEN];
   struct gdbarch *gdbarch = get_frame_arch (frame);
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -323,40 +337,47 @@  ppc_skip_trampoline_code (struct frame_info *frame, CORE_ADDR pc)
   /* 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.  */
+    scan_limit = 4;	/* At most 4 instructions.  */
 
   for (i = 0; i < scan_limit; i++)
     {
       if (ppc_insns_match_pattern (frame, pc, powerpc32_plt_stub, insnbuf))
 	{
-	  /* Insn pattern is
+	  /* Calculate PLT entry address from
 	     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);
+	     lwz   r11, xxxx(r11).  */
+	  target = ((ppc_insn_d_field (insnbuf[0]) << 16)
+		    + ppc_insn_d_field (insnbuf[1]));
+	}
+      else if (i < ARRAY_SIZE (powerpc32_plt_stub_so_1) - 1
+	       && ppc_insns_match_pattern (frame, pc, powerpc32_plt_stub_so_1,
+					   insnbuf))
+	{
+	  /* Calculate PLT entry address from
+	     lwz   r11, xxxx(r30).  */
+	  target = (ppc_insn_d_field (insnbuf[0])
+		    + get_frame_register_unsigned (frame,
+						   tdep->ppc_gp0_regnum + 30));
 	}
-      else if (ppc_insns_match_pattern (frame, pc, powerpc32_plt_stub_so,
+      else if (ppc_insns_match_pattern (frame, pc, powerpc32_plt_stub_so_2,
 					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);
+	  /* Calculate PLT entry address from
+	     addis r11, r30, xxxx
+	     lwz   r11, xxxx(r11).  */
+	  target = ((ppc_insn_d_field (insnbuf[0]) << 16)
+		    + ppc_insn_d_field (insnbuf[1])
+		    + get_frame_register_unsigned (frame,
+						   tdep->ppc_gp0_regnum + 30));
 	}
       else
 	{
-	  /* Scan backward one more instructions if doesn't match.  */
+	  /* Scan backward one more instruction if it doesn't match.  */
 	  pc -= 4;
 	  continue;
 	}
 
+      target = read_memory_unsigned_integer (target, 4, byte_order);
       return target;
     }
 
diff --git a/gdb/ppc-tdep.h b/gdb/ppc-tdep.h
index 3004744..156f82d 100644
--- a/gdb/ppc-tdep.h
+++ b/gdb/ppc-tdep.h
@@ -330,7 +330,7 @@  struct ppc_insn_pattern
 };
 
 extern int ppc_insns_match_pattern (struct frame_info *frame, CORE_ADDR pc,
-				    struct ppc_insn_pattern *pattern,
+				    const struct ppc_insn_pattern *pattern,
 				    unsigned int *insns);
 extern CORE_ADDR ppc_insn_d_field (unsigned int insn);
 
diff --git a/gdb/ppc64-tdep.c b/gdb/ppc64-tdep.c
index 7417232..5d8ccb4 100644
--- a/gdb/ppc64-tdep.c
+++ b/gdb/ppc64-tdep.c
@@ -80,7 +80,7 @@  ppc64_plt_entry_point (struct frame_info *frame, CORE_ADDR plt_off)
 
 /* Old ELFv1 PLT call stub.  */
 
-static struct ppc_insn_pattern ppc64_standard_linkage1[] =
+static const struct ppc_insn_pattern ppc64_standard_linkage1[] =
   {
     /* addis r12, r2, <any> */
     { insn_d (-1, -1, -1, 0), insn_d (15, 12, 2, 0), 0 },
@@ -119,7 +119,7 @@  static struct ppc_insn_pattern ppc64_standard_linkage1[] =
    instructions following "cmpldi r2, 0", "bnectr+" and "b <glink_i>",
    but there isn't any need to match them.  */
 
-static struct ppc_insn_pattern ppc64_standard_linkage2[] =
+static const struct ppc_insn_pattern ppc64_standard_linkage2[] =
   {
     /* std r2, 40(r1) <optional> */
     { -1, insn_ds (62, 2, 1, 40, 0), 1 },
@@ -162,7 +162,7 @@  static struct ppc_insn_pattern ppc64_standard_linkage2[] =
 
 /* ELFv1 PLT call stub to access PLT entries within +/- 32k of r2.  */
 
-static struct ppc_insn_pattern ppc64_standard_linkage3[] =
+static const struct ppc_insn_pattern ppc64_standard_linkage3[] =
   {
     /* std r2, 40(r1) <optional> */
     { -1, insn_ds (62, 2, 1, 40, 0), 1 },
@@ -201,7 +201,7 @@  static struct ppc_insn_pattern ppc64_standard_linkage3[] =
    A more modern variant of ppc64_standard_linkage2 differing in
    register usage.  */
 
-static struct ppc_insn_pattern ppc64_standard_linkage4[] =
+static const struct ppc_insn_pattern ppc64_standard_linkage4[] =
   {
     /* std r2, 40(r1) <optional> */
     { -1, insn_ds (62, 2, 1, 40, 0), 1 },
@@ -243,7 +243,7 @@  static struct ppc_insn_pattern ppc64_standard_linkage4[] =
    A more modern variant of ppc64_standard_linkage3 differing in
    register usage.  */
 
-static struct ppc_insn_pattern ppc64_standard_linkage5[] =
+static const struct ppc_insn_pattern ppc64_standard_linkage5[] =
   {
     /* std r2, 40(r1) <optional> */
     { -1, insn_ds (62, 2, 1, 40, 0), 1 },
@@ -280,7 +280,7 @@  static struct ppc_insn_pattern ppc64_standard_linkage5[] =
 
 /* ELFv2 PLT call stub to access PLT entries more than +/- 32k from r2.  */
 
-static struct ppc_insn_pattern ppc64_standard_linkage6[] =
+static const struct ppc_insn_pattern ppc64_standard_linkage6[] =
   {
     /* std r2, 24(r1) <optional> */
     { -1, insn_ds (62, 2, 1, 24, 0), 1 },
@@ -302,7 +302,7 @@  static struct ppc_insn_pattern ppc64_standard_linkage6[] =
 
 /* ELFv2 PLT call stub to access PLT entries within +/- 32k of r2.  */
 
-static struct ppc_insn_pattern ppc64_standard_linkage7[] =
+static const struct ppc_insn_pattern ppc64_standard_linkage7[] =
   {
     /* std r2, 24(r1) <optional> */
     { -1, insn_ds (62, 2, 1, 24, 0), 1 },
@@ -322,7 +322,7 @@  static struct ppc_insn_pattern ppc64_standard_linkage7[] =
 /* ELFv2 PLT call stub to access PLT entries more than +/- 32k from r2,
    supporting fusion.  */
 
-static struct ppc_insn_pattern ppc64_standard_linkage8[] =
+static const struct ppc_insn_pattern ppc64_standard_linkage8[] =
   {
     /* std r2, 24(r1) <optional> */
     { -1, insn_ds (62, 2, 1, 24, 0), 1 },
diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index f81064e..3ec3817 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -6717,17 +6717,17 @@  read_insn (struct frame_info *frame, CORE_ADDR pc)
    'struct ppc_insn_pattern' objects, terminated by an entry whose
    mask is zero.
 
-   When the match is successful, fill INSN[i] with what PATTERN[i]
+   When the match is successful, fill INSNS[i] with what PATTERN[i]
    matched.  If PATTERN[i] is optional, and the instruction wasn't
-   present, set INSN[i] to 0 (which is not a valid PPC instruction).
-   INSN should have as many elements as PATTERN.  Note that, if
-   PATTERN contains optional instructions which aren't present in
-   memory, then INSN will have holes, so INSN[i] isn't necessarily the
-   i'th instruction in memory.  */
+   present, set INSNS[i] to 0 (which is not a valid PPC instruction).
+   INSNS should have as many elements as PATTERN, minus the terminator.
+   Note that, if PATTERN contains optional instructions which aren't
+   present in memory, then INSNS will have holes, so INSNS[i] isn't
+   necessarily the i'th instruction in memory.  */
 
 int
 ppc_insns_match_pattern (struct frame_info *frame, CORE_ADDR pc,
-			 struct ppc_insn_pattern *pattern,
+			 const struct ppc_insn_pattern *pattern,
 			 unsigned int *insns)
 {
   int i;