[commit] New method find_frame_sal()

[commit] New method find_frame_sal()

[Andrew Cagney]

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

Hello,

While cleaning up accesses to the `struct frame_info', I kept coming 
across code snippits like:

  sal = find_pc_line (fi->pc,
                      (fi->next != (struct frame_info *) NULL
                       && !(get_frame_type (fi->next) == SIGTRAMP_FRAME)
                       && !(get_frame_type (fi->next) == DUMMY_FRAME)));

(Originally it was worse, as there was also fi->signal_handler_caller 
and PC_IN_CALL_DUMMY (fi->pc, fi->frame, fi->frame).)

The attached patch adds the function:

	find_frame_sal(fi, &sal);

that encapsulates all of the above.

The sole motivation for this being to eliminate all those frame pointer 
dereferences.

committed,
Andrew

[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 7246 bytes --]

2002-11-28  Andrew Cagney  <ac131313@redhat.com>

	* frame.c (pc_notcurrent): New function.
	(find_frame_sal): New function.
	* frame.h (find_frame_sal): Declare.
	(struct symtab_and_line): Add opaque declaration.
	* stack.c (print_frame_info_base): Use find_pc_line instead of
	find_frame_sal.
	(frame_info): Ditto.
	* ada-lang.c (find_printable_frame): Ditto.

Index: tui/ChangeLog
2002-11-28  Andrew Cagney  <ac131313@redhat.com>

	* tuiStack.c (tuiShowFrameInfo): Use find_frame_sal instead of
	find_pc_line.

Index: ada-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/ada-lang.c,v
retrieving revision 1.14
diff -u -r1.14 ada-lang.c
--- ada-lang.c	24 Nov 2002 03:20:08 -0000	1.14
+++ ada-lang.c	29 Nov 2002 01:42:35 -0000
@@ -5032,20 +5032,7 @@
 
   for (; fi != NULL; level += 1, fi = get_prev_frame (fi))
     {
-      /* If fi is not the innermost frame, that normally means that
-         fi->pc points at the return instruction (which is *after* the
-         call instruction), and we want to get the line containing the
-         call (because the call is where the user thinks the program
-         is).  However, if the next frame is either a SIGTRAMP_FRAME
-         or a DUMMY_FRAME, then the next frame will contain a saved
-         interrupt PC and such a PC indicates the current (rather than
-         next) instruction/line, consequently, for such cases, want to
-         get the line containing fi->pc.  */
-      sal =
-	find_pc_line (fi->pc,
-		      fi->next != NULL
-		      && !(get_frame_type (fi->next) == SIGTRAMP_FRAME)
-		      && !(get_frame_type (fi->next) == DUMMY_FRAME));
+      find_frame_sal (fi, &sal);
       if (sal.symtab && !is_ada_runtime_file (sal.symtab->filename))
 	{
 #if defined(__alpha__) && defined(__osf__) && !defined(VXWORKS_TARGET)
Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.33
diff -u -r1.33 frame.c
--- frame.c	28 Nov 2002 21:38:43 -0000	1.33
+++ frame.c	29 Nov 2002 01:42:36 -0000
@@ -1021,6 +1021,29 @@
   return frame->pc;
 }
 
+static int
+pc_notcurrent (struct frame_info *frame)
+{
+  /* If FRAME is not the innermost frame, that normally means that
+     FRAME->pc points at the return instruction (which is *after* the
+     call instruction), and we want to get the line containing the
+     call (because the call is where the user thinks the program is).
+     However, if the next frame is either a SIGTRAMP_FRAME or a
+     DUMMY_FRAME, then the next frame will contain a saved interrupt
+     PC and such a PC indicates the current (rather than next)
+     instruction/line, consequently, for such cases, want to get the
+     line containing fi->pc.  */
+  struct frame_info *next = get_next_frame (frame);
+  int notcurrent = (next != NULL && get_frame_type (next) == NORMAL_FRAME);
+  return notcurrent;
+}
+
+void
+find_frame_sal (struct frame_info *frame, struct symtab_and_line *sal)
+{
+  (*sal) = find_pc_line (frame->pc, pc_notcurrent (frame));
+}
+
 /* Per "frame.h", return the ``address'' of the frame.  Code should
    really be using get_frame_id().  */
 CORE_ADDR
Index: frame.h
===================================================================
RCS file: /cvs/src/src/gdb/frame.h,v
retrieving revision 1.39
diff -u -r1.39 frame.h
--- frame.h	29 Nov 2002 00:15:48 -0000	1.39
+++ frame.h	29 Nov 2002 01:42:36 -0000
@@ -23,6 +23,8 @@
 #if !defined (FRAME_H)
 #define FRAME_H 1
 
+struct symtab_and_line;
+
 /* The frame object.  */
 
 struct frame_info;
@@ -98,6 +100,27 @@
 /* The frame's `resume' address.  Where the program will resume in
    this frame.  */
 extern CORE_ADDR get_frame_pc (struct frame_info *);
+
+/* Closely related to the resume address, various symbol table
+   attributes that are determined by the PC.  Note that for a normal
+   frame, the PC refers to the resume address after the return, and
+   not the call instruction.  In such a case, the address is adjusted
+   so that it (approximatly) identifies the call site (and not return
+   site).
+
+   NOTE: cagney/2002-11-28: The frame cache could be used to cache the
+   computed value.  Working on the assumption that the bottle-neck is
+   in the single step code, and that code causes the frame cache to be
+   constantly flushed, caching things in a frame is probably of little
+   benefit.  As they say `show us the numbers'.
+
+   NOTE: cagney/2002-11-28: Plenty more where this one came from:
+   find_frame_block(), find_frame_partial_function(),
+   find_frame_symtab(), find_frame_function().  Each will need to be
+   carefully considered to determine if the real intent was for it to
+   apply to the PC or the adjusted PC.  */
+extern void find_frame_sal (struct frame_info *frame,
+			    struct symtab_and_line *sal);
 
 /* Return the frame address from FI.  Except in the machine-dependent
    *FRAME* macros, a frame address has no defined meaning other than
Index: stack.c
===================================================================
RCS file: /cvs/src/src/gdb/stack.c,v
retrieving revision 1.53
diff -u -r1.53 stack.c
--- stack.c	29 Nov 2002 00:15:48 -0000	1.53
+++ stack.c	29 Nov 2002 01:42:37 -0000
@@ -356,11 +356,7 @@
      frame is a SIGTRAMP_FRAME or a DUMMY_FRAME, then the next frame
      was not entered as the result of a call, and we want to get the
      line containing fi->pc.  */
-  sal =
-    find_pc_line (fi->pc,
-		  fi->next != NULL
-		  && !(get_frame_type (fi->next) == SIGTRAMP_FRAME)
-		  && !(get_frame_type (fi->next) == DUMMY_FRAME));
+  find_frame_sal (fi, &sal);
 
   location_print = (source == LOCATION 
 		    || source == LOC_AND_ADDRESS
@@ -757,11 +753,10 @@
   if (fi == NULL)
     error ("Invalid frame specified.");
 
-  sal = find_pc_line (fi->pc,
-		      fi->next != NULL
-		      && !(get_frame_type (fi->next) == SIGTRAMP_FRAME)
-		      && !(get_frame_type (fi->next) == DUMMY_FRAME));
+  find_frame_sal (fi, &sal);
   func = get_frame_function (fi);
+  /* FIXME: cagney/2002-11-28: Why bother?  Won't sal.symtab contain
+     the same value.  */
   s = find_pc_symtab (fi->pc);
   if (func)
     {
@@ -858,7 +853,8 @@
   if (fi->next || calling_frame_info)
     puts_filtered ("\n");
   if (s)
-    printf_filtered (" source language %s.\n", language_str (s->language));
+    printf_filtered (" source language %s.\n",
+		     language_str (s->language));
 
 #ifdef PRINT_EXTRA_FRAME_INFO
   PRINT_EXTRA_FRAME_INFO (fi);
Index: tui/tuiStack.c
===================================================================
RCS file: /cvs/src/src/gdb/tui/tuiStack.c,v
retrieving revision 1.24
diff -u -r1.24 tuiStack.c
--- tui/tuiStack.c	24 Nov 2002 03:20:09 -0000	1.24
+++ tui/tuiStack.c	29 Nov 2002 01:42:38 -0000
@@ -349,10 +349,7 @@
       int sourceAlreadyDisplayed;
       struct symtab_and_line sal;
 
-      sal = find_pc_line (fi->pc,
-                          (fi->next != (struct frame_info *) NULL
-			   && !(get_frame_type (fi->next) == SIGTRAMP_FRAME)
-			   && !(get_frame_type (fi->next) == DUMMY_FRAME)));
+      find_frame_sap (fi, &sal);
 
       sourceAlreadyDisplayed = sal.symtab != 0
         && tuiSourceIsDisplayed (sal.symtab->filename);

Re: [commit] New method find_frame_sal()

[Eli Zaretskii]

On Thu, 28 Nov 2002, Andrew Cagney wrote:

> While cleaning up accesses to the `struct frame_info', I kept coming 
> across code snippits like:
> 
>   sal = find_pc_line (fi->pc,
>                       (fi->next != (struct frame_info *) NULL
>                        && !(get_frame_type (fi->next) == SIGTRAMP_FRAME)
>                        && !(get_frame_type (fi->next) == DUMMY_FRAME)));
> 
> (Originally it was worse, as there was also fi->signal_handler_caller 
> and PC_IN_CALL_DUMMY (fi->pc, fi->frame, fi->frame).)
> 
> The attached patch adds the function:
> 
> 	find_frame_sal(fi, &sal);
> 
> that encapsulates all of the above.

Should this method be documented in gdbint.texinfo perhaps?

Re: [commit] New method find_frame_sal()

[Andrew Cagney]

> On Thu, 28 Nov 2002, Andrew Cagney wrote:
> 
> 
>> While cleaning up accesses to the `struct frame_info', I kept coming 
>> across code snippits like:
>> 
>>   sal = find_pc_line (fi->pc,
>>                       (fi->next != (struct frame_info *) NULL
>>                        && !(get_frame_type (fi->next) == SIGTRAMP_FRAME)
>>                        && !(get_frame_type (fi->next) == DUMMY_FRAME)));
>> 
>> (Originally it was worse, as there was also fi->signal_handler_caller 
>> and PC_IN_CALL_DUMMY (fi->pc, fi->frame, fi->frame).)
>> 
>> The attached patch adds the function:
>> 
>> 	find_frame_sal(fi, &sal);
>> 
>> that encapsulates all of the above.
> 
> 
> Should this method be documented in gdbint.texinfo perhaps?

Um, to this level of detail, I don't think so.

The thing I think is missing is a `Frames' chapter which describes how 
things hang together.  Guess why I've not got back to you with that 
frames doco patch I posted ....

Andrew