Re: [rfa/breakpoint] Use a frame ID instead of a frame

[Andrew Cagney <ac131313@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 22 bytes --]

Ok to commit?

Andrew

[-- Attachment #2: mailbox-message://ac131313@movemail/fsf/gdb/patches#13800491 --]
[-- Type: message/rfc822, Size: 15262 bytes --]

[-- Attachment #2.1.1: Type: text/plain, Size: 1150 bytes --]

Hello,

The attached patch pushes the `struct frame_id' through the breakpoint 
code.  Unlike a simple frame base, a frame ID can be used to 
differentiate between frameless frames

It makes the following changes:

- modifies `struct breakpoint' so that it uses a `struct frame_id 
frame_id' instead of a `CORE_ADDR frame' when identifying / locating a 
frame.

At present, given frameless function calls, the code will always find 
the inner most function.

- modifies the methods set_momentary_breakpoint() and 
set_longjmp_resume_breakpoint() so that they take a frame ID instead of 
a `struct frame_info'.

At present, on at least one occasion, GDB fakes up a `struct frame_info' 
just so that it can call one of these functions where that function just 
uses the `struct frame_info' to obtain the frame ID.  This eliminates 
the need to do that unnecessary operation.

One note.  While the frame ID can be used to differentiate between 
frameless frames, the current code does not.  I intend posting the patch 
to do that after all this infrastructure is in place (I've tested it and 
the i386 didn't show regressions :-).

ok to commit,
Andrew

[-- Attachment #2.1.2: diffs --]
[-- Type: text/plain, Size: 10980 bytes --]

2002-12-03  Andrew Cagney  <ac131313@redhat.com>

	* breakpoint.h (struct breakpoint): Replace frame with frame_id.
	(set_momentary_breaokpoint): Replace `struct frame_info' parameter
	with `struct frame_id'.
	(set_longjmp_resume_breakpoint): Ditto.
	* infrun.c (handle_inferior_event): Update.
	* breakpoint.c (watch_command_1, until_break_command): Update.
	* infrun.c (handle_inferior_event, check_sigtramp2): Update.
	(handle_inferior_event, step_over_function): Update.
	* breakpoint.c (bpstat_stop_status, print_one_breakpoint): Update.
	(set_raw_breakpoint, set_longjmp_resume_breakpoint): Update.
	(set_momentary_breakpoint, deprecated_frame_in_dummy): Update.
	* infcmd.c (finish_command, run_stack_dummy): Update.

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.99
diff -u -r1.99 breakpoint.c
--- breakpoint.c	4 Dec 2002 00:05:53 -0000	1.99
+++ breakpoint.c	4 Dec 2002 02:30:05 -0000
@@ -1704,7 +1704,7 @@
   ALL_BREAKPOINTS (b)
   {
     if (b->type == bp_call_dummy
-	&& b->frame == frame->frame
+	&& frame_id_eq (b->frame_id, get_frame_id (frame))
     /* We need to check the PC as well as the frame on the sparc,
        for signals.exp in the testsuite.  */
 	&& (frame->pc
@@ -2727,8 +2727,8 @@
 	real_breakpoint = 1;
       }
 
-    if (b->frame &&
-       b->frame != (get_current_frame ())->frame)
+    if (frame_id_p (b->frame_id)
+	&& !frame_id_eq (b->frame_id, get_frame_id (get_current_frame ())))
       bs->stop = 0;
     else
       {
@@ -3417,11 +3417,13 @@
   
   ui_out_text (uiout, "\n");
   
-  if (b->frame)
+  if (frame_id_p (b->frame_id))
     {
       annotate_field (6);
       ui_out_text (uiout, "\tstop only in stack frame at ");
-      ui_out_field_core_addr (uiout, "frame", b->frame);
+      /* FIXME: cagney/2002-12-01: Shouldn't be poeking around inside
+         the frame ID.  */
+      ui_out_field_core_addr (uiout, "frame", b->frame_id.base);
       ui_out_text (uiout, "\n");
     }
   
@@ -3842,7 +3844,7 @@
   b->silent = 0;
   b->ignore_count = 0;
   b->commands = NULL;
-  b->frame = 0;
+  b->frame_id = null_frame_id;
   b->dll_pathname = NULL;
   b->triggered_dll_pathname = NULL;
   b->forked_inferior_pid = 0;
@@ -4310,7 +4312,7 @@
    that gets deleted automatically... */
 
 void
-set_longjmp_resume_breakpoint (CORE_ADDR pc, struct frame_info *frame)
+set_longjmp_resume_breakpoint (CORE_ADDR pc, struct frame_id frame_id)
 {
   register struct breakpoint *b;
 
@@ -4319,10 +4321,7 @@
     {
       b->address = pc;
       b->enable_state = bp_enabled;
-      if (frame != NULL)
-	b->frame = frame->frame;
-      else
-	b->frame = 0;
+      b->frame_id = frame_id;
       check_duplicates (b);
       return;
     }
@@ -4374,14 +4373,14 @@
    Restrict it to frame FRAME if FRAME is nonzero.  */
 
 struct breakpoint *
-set_momentary_breakpoint (struct symtab_and_line sal, struct frame_info *frame,
+set_momentary_breakpoint (struct symtab_and_line sal, struct frame_id frame_id,
 			  enum bptype type)
 {
   register struct breakpoint *b;
   b = set_raw_breakpoint (sal, type);
   b->enable_state = bp_enabled;
   b->disposition = disp_donttouch;
-  b->frame = (frame ? frame->frame : 0);
+  b->frame_id = frame_id;
 
   /* If we're debugging a multi-threaded program, then we
      want momentary breakpoints to be active in only a 
@@ -5424,7 +5423,7 @@
 	  scope_breakpoint->disposition = disp_del;
 
 	  /* Only break in the proper frame (help with recursion).  */
-	  scope_breakpoint->frame = prev_frame->frame;
+	  scope_breakpoint->frame_id = get_frame_id (prev_frame);
 
 	  /* Set the address at which we will stop.  */
 	  scope_breakpoint->address = get_frame_pc (prev_frame);
@@ -5612,7 +5611,9 @@
 
   resolve_sal_pc (&sal);
 
-  breakpoint = set_momentary_breakpoint (sal, deprecated_selected_frame, bp_until);
+  breakpoint = 
+    set_momentary_breakpoint (sal,get_frame_id (deprecated_selected_frame),
+			      bp_until);
 
   if (!event_loop_p || !target_can_async_p ())
     old_chain = make_cleanup_delete_breakpoint (breakpoint);
@@ -5646,7 +5647,8 @@
     {
       sal = find_pc_line (prev_frame->pc, 0);
       sal.pc = prev_frame->pc;
-      breakpoint = set_momentary_breakpoint (sal, prev_frame, bp_until);
+      breakpoint = set_momentary_breakpoint (sal, get_frame_id (prev_frame),
+					     bp_until);
       if (!event_loop_p || !target_can_async_p ())
 	make_cleanup_delete_breakpoint (breakpoint);
       else
Index: breakpoint.h
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.h,v
retrieving revision 1.15
diff -u -r1.15 breakpoint.h
--- breakpoint.h	18 Nov 2002 22:19:26 -0000	1.15
+++ breakpoint.h	4 Dec 2002 02:30:05 -0000
@@ -237,7 +237,7 @@
     struct command_line *commands;
     /* Stack depth (address of frame).  If nonzero, break only if fp
        equals this.  */
-    CORE_ADDR frame;
+    struct frame_id frame_id;
     /* Conditional.  Break only if this expression's value is nonzero.  */
     struct expression *cond;
 
@@ -521,9 +521,6 @@
 
 /* Prototypes for breakpoint-related functions.  */
 
-/* Forward declarations for prototypes */
-struct frame_info;
-
 extern enum breakpoint_here breakpoint_here_p (CORE_ADDR);
 
 extern int breakpoint_inserted_here_p (CORE_ADDR);
@@ -532,6 +529,7 @@
    implements a functional superset of this function.  The only reason
    it hasn't been removed is because some architectures still don't
    use the new framework.  Once they have been fixed, this can go.  */
+struct frame_info;
 extern int deprecated_frame_in_dummy (struct frame_info *);
 
 extern int breakpoint_thread_match (CORE_ADDR, ptid_t);
@@ -545,7 +543,7 @@
 extern int ep_is_exception_catchpoint (struct breakpoint *);
 
 extern struct breakpoint *set_momentary_breakpoint
-  (struct symtab_and_line, struct frame_info *, enum bptype);
+  (struct symtab_and_line, struct frame_id, enum bptype);
 
 extern void set_ignore_count (int, int, int);
 
@@ -619,7 +617,7 @@
 extern void enable_overlay_breakpoints (void);
 extern void disable_overlay_breakpoints (void);
 
-extern void set_longjmp_resume_breakpoint (CORE_ADDR, struct frame_info *);
+extern void set_longjmp_resume_breakpoint (CORE_ADDR, struct frame_id);
 /* These functions respectively disable or reenable all currently
    enabled watchpoints.  When disabled, the watchpoints are marked
    call_disabled.  When reenabled, they are marked enabled.
Index: infcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/infcmd.c,v
retrieving revision 1.65
diff -u -r1.65 infcmd.c
--- infcmd.c	3 Dec 2002 12:25:11 -0000	1.65
+++ infcmd.c	4 Dec 2002 02:30:05 -0000
@@ -1018,8 +1018,11 @@
 
          addr is the address of the call dummy plus the CALL_DUMMY_START_OFFSET,
          so we need to subtract the CALL_DUMMY_START_OFFSET.  */
+      /* FIXME: cagney/2002-12-01: Rather than pass in curent frame,
+         why not just create, and then pass in a frame ID.  This would
+         make it possible to eliminate set_current_frame().  */
       bpt = set_momentary_breakpoint (sal,
-				      get_current_frame (),
+				      get_frame_id (get_current_frame ()),
 				      bp_call_dummy);
       bpt->disposition = disp_del;
 
@@ -1284,7 +1287,7 @@
   sal = find_pc_line (frame->pc, 0);
   sal.pc = frame->pc;
 
-  breakpoint = set_momentary_breakpoint (sal, frame, bp_finish);
+  breakpoint = set_momentary_breakpoint (sal, get_frame_id (frame), bp_finish);
 
   if (!event_loop_p || !target_can_async_p ())
     old_chain = make_cleanup_delete_breakpoint (breakpoint);
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.81
diff -u -r1.81 infrun.c
--- infrun.c	4 Dec 2002 00:05:53 -0000	1.81
+++ infrun.c	4 Dec 2002 02:30:06 -0000
@@ -2243,7 +2243,7 @@
 	  set_longjmp_resume_breakpoint (jmp_buf_pc, get_current_frame ());
 	else
 #endif /* 0 */
-	  set_longjmp_resume_breakpoint (jmp_buf_pc, NULL);
+	  set_longjmp_resume_breakpoint (jmp_buf_pc, null_frame_id);
 	ecs->handling_longjmp = 1;	/* FIXME */
 	keep_going (ecs);
 	return;
@@ -2536,7 +2536,7 @@
 
 	  check_for_old_step_resume_breakpoint ();
 	  step_resume_breakpoint =
-	    set_momentary_breakpoint (sr_sal, NULL, bp_step_resume);
+	    set_momentary_breakpoint (sr_sal, null_frame_id, bp_step_resume);
 	  if (breakpoints_inserted)
 	    insert_breakpoints ();
 	}
@@ -2593,7 +2593,7 @@
 	       try it.  */
 	    check_for_old_step_resume_breakpoint ();
 	    step_resume_breakpoint =
-	      set_momentary_breakpoint (sr_sal, NULL, bp_step_resume);
+	      set_momentary_breakpoint (sr_sal, null_frame_id, bp_step_resume);
 	    if (breakpoints_inserted)
 	      insert_breakpoints ();
 	  }
@@ -2702,7 +2702,7 @@
 	      xxx.section = find_pc_overlay (xxx.pc);
 	      check_for_old_step_resume_breakpoint ();
 	      step_resume_breakpoint =
-		set_momentary_breakpoint (xxx, NULL, bp_step_resume);
+		set_momentary_breakpoint (xxx, null_frame_id, bp_step_resume);
 	      insert_breakpoints ();
 	      keep_going (ecs);
 	      return;
@@ -2780,7 +2780,7 @@
 	     is where the new fp value is established.  */
 	  check_for_old_step_resume_breakpoint ();
 	  step_resume_breakpoint =
-	    set_momentary_breakpoint (sr_sal, NULL, bp_step_resume);
+	    set_momentary_breakpoint (sr_sal, null_frame_id, bp_step_resume);
 	  if (breakpoints_inserted)
 	    insert_breakpoints ();
 
@@ -2894,7 +2894,7 @@
       /* We perhaps could set the frame if we kept track of what the
          frame corresponding to prev_pc was.  But we don't, so don't.  */
       through_sigtramp_breakpoint =
-	set_momentary_breakpoint (sr_sal, NULL, bp_through_sigtramp);
+	set_momentary_breakpoint (sr_sal, null_frame_id, bp_through_sigtramp);
       if (breakpoints_inserted)
 	insert_breakpoints ();
 
@@ -2952,7 +2952,7 @@
          established.  */
       check_for_old_step_resume_breakpoint ();
       step_resume_breakpoint =
-	set_momentary_breakpoint (sr_sal, NULL, bp_step_resume);
+	set_momentary_breakpoint (sr_sal, null_frame_id, bp_step_resume);
       if (breakpoints_inserted)
 	insert_breakpoints ();
 
@@ -2985,10 +2985,15 @@
 
   check_for_old_step_resume_breakpoint ();
   step_resume_breakpoint =
-    set_momentary_breakpoint (sr_sal, get_current_frame (), bp_step_resume);
+    set_momentary_breakpoint (sr_sal, get_frame_id (get_current_frame ()),
+			      bp_step_resume);
 
   if (step_frame_address && !IN_SOLIB_DYNSYM_RESOLVE_CODE (sr_sal.pc))
-    step_resume_breakpoint->frame = step_frame_address;
+    /* Match any frame with the specified address but wild card on the
+       func (someone should change step_frame_address to
+       step_frame_id).  */
+    step_resume_breakpoint->frame_id =
+      frame_id_build (step_frame_address, 0);
 
   if (breakpoints_inserted)
     insert_breakpoints ();