Re: [PATCH] add 'rs6000_in_function_epilogue_p()' (Revised)

[Paul Gilliam <pgilliam at us dot ibm dot com>]

On Thursday 01 December 2005 17:07, Jim Blandy wrote:
> Paul wanted to fast-track this patch, in hopes it could get into the
> 6.4 release.  Joel, what are your thoughts?
> 
I guess this is now moot.  Oh well.

> I think the best alternative to deprecated_read_memory_nobpt would be
> to actually call current_frame (this will be cheap, since we just
> called it in watchpoint_check, before calling
> gdbarch_in_function_epilogue_p) and then call
> safe_frame_unwind_memory.
> 
I revised the patch using your alternative (see below).

> Whatever we do immediately, the underlying cause here is that
> unwinding isn't working at every instruction.  If it were, then the
> call to frame_find_by_id would find the frame containing the watched
> variable, and watchpoint_check would correctly conclude that the
> variable was still in scope.
> 
> Paul, does your test program have Dwarf CFI for the functions in question?
> 
> 
I have attached a couple of dumps (and a typescript show how they where made), the executable, and the source.

And here is the revised patch:

2005-12-02  Paul Gilliam  <pgilliam@us.ibm.com>

        * rs6000-tdep.c: Add new subroutine, 'rs6000_in_function_epilogue_p()'
        and put it into the architecture vector.

Index: rs6000-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v
retrieving revision 1.248
diff -a -u -p -r1.248 rs6000-tdep.c
--- rs6000-tdep.c       1 Nov 2005 19:32:36 -0000       1.248
+++ rs6000-tdep.c       2 Dec 2005 18:43:19 -0000
@@ -502,6 +502,69 @@ rs6000_skip_prologue (CORE_ADDR pc)
   return pc;
 }

+static int
+insn_changes_sp(unsigned long insn)
+{
+  int opcode  = (insn>>26) & 0x03f;
+  int sd      = (insn>>21) & 0x01f;
+  int a       = (insn>>16) & 0x01f;
+  int b       = (insn>>11) & 0x01f;
+  int subcode = (insn>> 1) & 0x3ff;
+  int rc      =  insn      & 0x001;
+
+  if (opcode == 31 && subcode == 444 && a == 1)
+    return 1;  /* mr R1,Rn */
+  if (opcode == 14 && sd == 1)
+    return 1;  /* addi R1,Rn,simm */
+  if (opcode == 58 && sd == 1)
+    return 1;  /* ld R1,ds(Rn) */
+
+  return 0;
+}
+
+/* Return true if we are in the functin's epilogue, i.e. after the
+   instruction that destroyed the function's stack frame.  */
+static int
+rs6000_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc)
+{
+  bfd_byte insn_buf[PPC_INSN_SIZE];
+  CORE_ADDR scan_pc, func_addr, func_end;
+  unsigned long insn;
+  struct frame_info *fr;
+
+  /* Find the search limits.  */
+  if (!find_pc_partial_function (pc, NULL, &func_addr, &func_end))
+    return 0;
+
+  /* Get the current frame.  This may be cheap, since we might have just called
+     it in watchpoint_check, before calling gdbarch_in_function_epilogue_p.  */
+
+  fr = get_current_frame ();
+
+  /* Scan forward untill next 'blr'.  */
+  for (scan_pc = pc; scan_pc < func_end; scan_pc += PPC_INSN_SIZE)
+    {
+      if (!safe_frame_unwind_memory (fr, scan_pc, insn_buf, PPC_INSN_SIZE))
+        return 0;
+      insn = extract_signed_integer (insn_buf, PPC_INSN_SIZE);
+      if (insn == 0x4e800020) break;
+      if (insn_changes_sp(insn))
+        return 0;
+    }
+
+  /* Scan backward untill adjustment to stack pointer (R1).  */
+  for (scan_pc=pc-PPC_INSN_SIZE; scan_pc>=func_addr; scan_pc-=PPC_INSN_SIZE)
+    {
+      if (!safe_frame_unwind_memory (fr, scan_pc, insn_buf, PPC_INSN_SIZE))
+        return 0;
+      insn = extract_signed_integer (insn_buf, PPC_INSN_SIZE);
+      if (insn_changes_sp(insn))
+        return 1;
+    }
+
+  return 0;
+}
+

 /* Fill in fi->saved_regs */

@@ -3342,6 +3405,8 @@ rs6000_gdbarch_init (struct gdbarch_info
   set_gdbarch_deprecated_extract_struct_value_address (gdbarch, rs6000_extract_struct_value_address);

   set_gdbarch_skip_prologue (gdbarch, rs6000_skip_prologue);
+  set_gdbarch_in_function_epilogue_p (gdbarch, rs6000_in_function_epilogue_p);
+
   set_gdbarch_inner_than (gdbarch, core_addr_lessthan);
   set_gdbarch_breakpoint_from_pc (gdbarch, rs6000_breakpoint_from_pc);

Attachment: dump.typescript
Description: Text document

Attachment: testwatch.readelf
Description: Text document

Attachment: testwatch
Description: application/executable

int  checkmeout = 0;
char *lookatme = "I have not changed";

void subr2(int * addr)
{
	*addr = 9;
	*addr = 1;
}


void subr1()
{
        int     watchthis = 9;

	subr2(&watchthis);

        checkmeout = 2;
        lookatme = "change is good";
        watchthis = 2;
}

int main()
{
	int	ret_val = 0;

	subr1();
}

Attachment: testwatch.objdmp
Description: Text document