diff mbox

[PING] Improve prologue skipping on PPC32 for variadic functions

Message ID 55722498.3050208@codesourcery.com
State New
Headers show

Commit Message

Kwok Cheung Yeung June 5, 2015, 10:37 p.m. UTC
This is a ping for some PowerPC patches that I submitted two years ago at:

http://sourceware.org/ml/gdb-patches/2013-05/msg00532.html
http://sourceware.org/ml/gdb-patches/2013-05/msg00553.html

These fix issues that occur when trying to set a breakpoint on variadic 
functions such as printf() on 32-bit PowerPC.

I have now combined and updated these patches for current GDB trunk. Note that 
the first part of the first patch (for skipping the prologue in variadic 
functions with debug info enabled) is no longer strictly necessary as newer 
versions of GCC (GCC 4.8.3 has the problem, GCC 4.9.1 does not) will no longer 
emit line information after the branch for skipping the saving of FP registers, 
therefore the problem with losing control does not occur. It may still be useful 
for older compiles though. The rest is still relevant on modern toolchains.

Kwok

2015-06-05  Kwok Cheung Yeung  <kcy@codesourcery.com>

	* rs6000-tdep.c (store_param_on_stack_p): Track register
	assigned to r0.  Track saved registers.  Return false if a
	register has already been saved.  Fix range used for parameters
	passed in floating-point registers.
	(skip_prologue): Add variables used to keep track of saved
	registers and pass to store_param_on_stack_p.  Add test for
	instruction used to skip saving of the floating-point registers.
	(GET_DEST_REG): New macro.
	(BNE_CR1_MASK, BNE_CR1_INSTRUCTION, BNE_TARGET_MASK, STF_MASK,
	STF_INSTRUCTION): New defines.
	(skip_prologue_fp_reg_save): New.
	(rs6000_skip_prologue): Use result of skip_prologue_fp_reg_save
	when using SAL information to skip prologue.

        else
@@ -1210,45 +1214,45 @@ store_param_on_stack_p (unsigned long op, int framep, 
int *r0_contains_arg)
      }

    /* Save a General Purpose Register on stack.  */
+  if ((op & 0xfc1f0003) == 0xf8010000 ||                /* std Rx,NUM(r1) */
+      (framep && (op & 0xfc1f0000) == 0x901f0000) ||    /* stw Rx,NUM(r31) */
+      (framep && (op & 0xfc1f0000) == 0x981f0000))      /* stb Rx,NUM(r31) */

-  if ((op & 0xfc1f0003) == 0xf8010000 ||       /* std  Rx,NUM(r1) */
-      (op & 0xfc1f0000) == 0xd8010000)         /* stfd Rx,NUM(r1) */
-    {
-      /* Rx: Only r3 - r10 are used for parameter passing.  */
-      const int rx_regno = GET_SRC_REG (op);
-
-      return (rx_regno >= 3 && rx_regno <= 10);
-    }
-
-  /* Save a General Purpose Register on stack via the Frame Pointer.  */
-
-  if (framep &&
-      ((op & 0xfc1f0000) == 0x901f0000 ||     /* st rx,NUM(r31) */
-       (op & 0xfc1f0000) == 0x981f0000 ||     /* stb Rx,NUM(r31) */
-       (op & 0xfc1f0000) == 0xd81f0000))      /* stfd Rx,NUM(r31) */
      {
        /* Rx: Usually, only r3 - r10 are used for parameter passing.
           However, the compiler sometimes uses r0 to hold an argument.  */
-      const int rx_regno = GET_SRC_REG (op);
+      int rx_regno = GET_SRC_REG (op);

-      return ((rx_regno >= 3 && rx_regno <= 10)
-              || (rx_regno == 0 && *r0_contains_arg));
-    }
+      if (rx_regno == 0 && *r0_contains_arg)
+        rx_regno = *r0_contains_arg;

-  if ((op & 0xfc1f0000) == 0xfc010000)         /* frsp, fp?,NUM(r1) */
-    {
-      /* Only f2 - f8 are used for parameter passing.  */
-      const int src_regno = GET_SRC_REG (op);
-
-      return (src_regno >= 2 && src_regno <= 8);
+      if (rx_regno >= 3 && rx_regno <= 10 &&
+          !(*saved_gp_params & (1 << rx_regno)))
+        {
+          *saved_gp_params |= 1 << rx_regno;
+          return 1;
+        }
+      else
+        return 0;
      }

-  if (framep && ((op & 0xfc1f0000) == 0xfc1f0000))  /* frsp, fp?,NUM(r31) */
+  /* Save a Floating Point Register on stack.  */
+  if ((op & 0xfc1f0000) == 0xd8010000 ||              /* stfd Fx,NUM(r1) */
+      (op & 0xfc1f0000) == 0xfc010000 ||              /* frsp Fx,NUM(r1) */
+      (framep && (op & 0xfc1f0000) == 0xd81f0000) ||  /* stfd Fx,NUM(r31) */
+      (framep && (op & 0xfc1f0000) == 0xfc1f0000))    /* frsp Fx,NUM(r31) */
      {
-      /* Only f2 - f8 are used for parameter passing.  */
+      /* Only f1 - f8 are used for parameter passing.  */
        const int src_regno = GET_SRC_REG (op);

-      return (src_regno >= 2 && src_regno <= 8);
+      if (src_regno >= 1 && src_regno <= 8 &&
+          !(*saved_fp_params & (1 << src_regno)))
+        {
+          *saved_fp_params |= 1 << src_regno;
+          return 1;
+        }
+      else
+        return 0;
      }

    /* Not an insn that saves a parameter on stack.  */
@@ -1523,6 +1527,8 @@ skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc, 
CORE_ADDR lim_pc,
    int prev_insn_was_prologue_insn = 1;
    int num_skip_non_prologue_insns = 0;
    int r0_contains_arg = 0;
+  unsigned int saved_gp_params = 0;
+  unsigned int saved_fp_params = 0;
    const struct bfd_arch_info *arch_info = gdbarch_bfd_arch_info (gdbarch);
    struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
    enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -1776,6 +1782,10 @@ skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc, 
CORE_ADDR lim_pc,
  	  fdata->used_bl = 1;
  	  continue;
  	}
+      /* The prologue of variadic functions may contain a branch instruction
+         to skip the saving of floating-point registers.  */
+      else if ((op & 0xffff0003) == 0x40860000)   /* bne cr1,SIMM */
+        continue;
        /* update stack pointer */
        else if ((op & 0xfc1f0000) == 0x94010000)
  	{		/* stu rX,NUM(r1) ||  stwu rX,NUM(r1) */
@@ -1836,7 +1846,9 @@ skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc, 
CORE_ADDR lim_pc,
  	  /* store parameters in stack */
  	}
        /* Move parameters from argument registers to temporary register.  */
-      else if (store_param_on_stack_p (op, framep, &r0_contains_arg))
+      else if (store_param_on_stack_p (op, framep, &r0_contains_arg,
+                                       &saved_gp_params,
+                                       &saved_fp_params))
          {
  	  continue;

@@ -2113,6 +2125,95 @@ skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc, 
CORE_ADDR lim_pc,
    return last_prologue_pc;
  }

+#define GET_DEST_REG(x) (((x) >> 16) & 0x1f)
+
+#define BNE_CR1_MASK            0xffff0003
+#define BNE_CR1_INSTRUCTION     0x40860000
+#define BNE_TARGET_MASK         0x0000fffc
+#define STF_MASK                0xf4000000
+#define STF_INSTRUCTION         0xd0000000
+
+/* This function inspects the instructions between the start of a
+   function and PC, looking for code indicative of a floating-point
+   save sequence used by variadic functions.  If found, an address
+   that occurs after the save sequence will be returned, otherwise
+   PC is returned.  */
+
+static CORE_ADDR
+skip_prologue_fp_reg_save (struct gdbarch *gdbarch, CORE_ADDR pc)
+{
+  gdb_byte buf[PPC_INSN_SIZE];
+  unsigned long opcode;
+  CORE_ADDR branch_addr, addr;
+  CORE_ADDR target_addr = 0;
+  CORE_ADDR start_pc, end_pc;
+  struct symtab_and_line prologue_sal;
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+
+  find_pc_partial_function (pc, NULL, &start_pc, &end_pc);
+
+  /* Search for a bne cr1,SIMM instruction.  */
+  for (branch_addr = start_pc;
+       branch_addr < pc;
+       branch_addr += PPC_INSN_SIZE)
+    {
+      if (target_read_memory (branch_addr, buf, PPC_INSN_SIZE))
+        return pc;
+      opcode = extract_unsigned_integer (buf, PPC_INSN_SIZE, byte_order);
+
+      if ((opcode & BNE_CR1_MASK) == BNE_CR1_INSTRUCTION)
+        {
+          target_addr = branch_addr + (opcode & BNE_TARGET_MASK);
+          break;
+        }
+    }
+
+  if (target_addr <= pc)
+    return pc;
+
+  /* All instructions between BRANCH_ADDR + PPC_INSN_SIZE and
+     TARGET_ADDR - PPC_INSN_SIZE should be floating-point
+     store instructions that write to an address relative to
+     R1 or R31.  */
+  for (addr = branch_addr + PPC_INSN_SIZE;
+       addr < target_addr;
+       addr += PPC_INSN_SIZE)
+    {
+      unsigned int insn;
+
+      if (target_read_memory (addr, buf, PPC_INSN_SIZE))
+        return pc;
+      insn = extract_unsigned_integer (buf, PPC_INSN_SIZE, byte_order);
+      if ((insn & STF_MASK) != STF_INSTRUCTION
+          || (GET_DEST_REG (insn) != 1 && GET_DEST_REG (insn) != 31))
+        return pc;
+    }
+
+  prologue_sal = find_pc_line (target_addr, 0);
+
+  /* Look for the first line that starts on or after TARGET_ADDR.  */
+  if (prologue_sal.symtab->language != language_asm)
+    {
+      struct linetable *linetable = SYMTAB_LINETABLE (prologue_sal.symtab);
+      int idx = 0;
+
+      /* Skip any earlier lines, and any end-of-sequence marker
+         from a previous function.  */
+      while (linetable->item[idx].pc != prologue_sal.pc
+             || linetable->item[idx].line == 0)
+        idx++;
+
+      for (; idx < linetable->nitems; idx++)
+        {
+          if (linetable->item[idx].line != 0
+              && linetable->item[idx].pc >= target_addr)
+            return linetable->item[idx].pc;
+        }
+    }
+
+  return target_addr;
+}
+
  static CORE_ADDR
  rs6000_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
  {
@@ -2127,7 +2228,11 @@ rs6000_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
        CORE_ADDR post_prologue_pc
  	= skip_prologue_using_sal (gdbarch, func_addr);
        if (post_prologue_pc != 0)
-	return max (pc, post_prologue_pc);
+        {
+          post_prologue_pc = skip_prologue_fp_reg_save (gdbarch,
+                                                        post_prologue_pc);
+          return max (pc, post_prologue_pc);
+        }
      }

    /* Can't determine prologue from the symbol table, need to examine

Comments

Andreas Schwab June 6, 2015, 7:37 a.m. UTC | #1
Kwok Cheung Yeung <kcy@codesourcery.com> writes:

> @@ -1210,45 +1214,45 @@ store_param_on_stack_p (unsigned long op, int
> framep, int *r0_contains_arg)
>      }
>
>    /* Save a General Purpose Register on stack.  */
> +  if ((op & 0xfc1f0003) == 0xf8010000 ||                /* std Rx,NUM(r1) */
> +      (framep && (op & 0xfc1f0000) == 0x901f0000) ||    /* stw Rx,NUM(r31) */
> +      (framep && (op & 0xfc1f0000) == 0x981f0000))      /* stb Rx,NUM(r31) */

Please also fix the formatting (break before the operator, not after).

Andreas.
diff mbox

Patch

diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index eb40430..bec46db 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -1187,10 +1187,14 @@  static int max_skip_non_prologue_insns = 10;
  /* Return nonzero if the given instruction OP can be part of the prologue
     of a function and saves a parameter on the stack.  FRAMEP should be
     set if one of the previous instructions in the function has set the
-   Frame Pointer.  */
+   Frame Pointer.  If the parameter has already been saved on the stack by
+   a previous prologue instruction, then OP is not considered to be part
+   of the prologue.  */

  static int
-store_param_on_stack_p (unsigned long op, int framep, int *r0_contains_arg)
+store_param_on_stack_p (unsigned long op, int framep, int *r0_contains_arg,
+                        unsigned int *saved_gp_params,
+                        unsigned int *saved_fp_params)
  {
    /* Move parameters from argument registers to temporary register.  */
    if ((op & 0xfc0007fe) == 0x7c000378)         /* mr(.)  Rx,Ry */
@@ -1202,7 +1206,7 @@  store_param_on_stack_p (unsigned long op, int framep, int 
*r0_contains_arg)

        if (rx_regno == 0 && ry_regno >= 3 && ry_regno <= 10)
          {
-          *r0_contains_arg = 1;
+          *r0_contains_arg = ry_regno;
            return 1;
          }