Re: [RFA] block_innermost_frame tweak

[Joel Brobecker <brobecker@gnat.com>]

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

Wow, that was fast, thanks for the quick review.

> Would it make sense to create a new function --- `frame_calling_pc',
> perhaps --- which encapsulates the logic in get_frame_block, and then
> use that instead of `frame->pc' in get_frame_block and
> block_innermost_frame?

Sure it does. A new patch is attached for:

2002-06-20  Joel Brobecker  <brobecker@gnat.com>

        * frame.h (get_frame_calling_pc): New function.

        * blockframe.c (get_frame_calling_pc): New function extracted
        from get_frame_block().
        (get_frame_block): Use get_frame_calling_pc().
        (block_innermost_frame): Match the frame calling pc against the
        block boundaries rather than the frame pc directly. This prevents
        a failure when a frame pc is actually a return address pointing
        immediately after the end of the given block.

No regression on Linux x86.

-- 
Joel

[-- Attachment #2: frame.h.diff --]
[-- Type: text/plain, Size: 581 bytes --]

Index: frame.h
===================================================================
RCS file: /cvs/src/src/gdb/frame.h,v
retrieving revision 1.20
diff -c -3 -p -r1.20 frame.h
*** frame.h	18 Jun 2002 09:04:24 -0000	1.20
--- frame.h	20 Jun 2002 22:13:39 -0000
*************** extern struct symbol *get_frame_function
*** 250,255 ****
--- 250,257 ----
  
  extern CORE_ADDR get_frame_pc (struct frame_info *);
  
+ extern CORE_ADDR get_frame_calling_pc (struct frame_info *);
+ 
  extern CORE_ADDR get_pc_function_start (CORE_ADDR);
  
  extern struct block *block_for_pc (CORE_ADDR);

[-- Attachment #3: blockframe.c.diff --]
[-- Type: text/plain, Size: 2966 bytes --]

Index: blockframe.c
===================================================================
RCS file: /cvs/src/src/gdb/blockframe.c,v
retrieving revision 1.29
diff -c -3 -p -r1.29 blockframe.c
*** blockframe.c	8 Jun 2002 18:30:14 -0000	1.29
--- blockframe.c	20 Jun 2002 22:19:15 -0000
*************** get_frame_pc (struct frame_info *frame)
*** 528,533 ****
--- 528,553 ----
    return frame->pc;
  }
  
+ /* return the address of the PC for the given FRAME, ie the current PC value
+    if FRAME is the innermost frame, or the address adjusted to point to the
+    call instruction if not.  */
+ 
+ CORE_ADDR
+ get_frame_calling_pc (struct frame_info *frame)
+ {
+   CORE_ADDR pc = frame->pc;
+ 
+   /* If we are not in the innermost frame, and we are not interrupted
+      by a signal, frame->pc points to the instruction following the
+      call. As a consequence, we need to get the address of the previous
+      instruction. Unfortunately, this is not straightforward to do, so
+      we just use the address minus one, which is a good enough
+      approximation.  */
+   if (frame->next != 0 && frame->next->signal_handler_caller == 0)
+     --pc;
+ 
+   return pc;
+ }
  
  #ifdef FRAME_FIND_SAVED_REGS
  /* XXX - deprecated.  This is a compatibility function for targets
*************** get_frame_saved_regs (struct frame_info 
*** 576,592 ****
  struct block *
  get_frame_block (struct frame_info *frame, CORE_ADDR *addr_in_block)
  {
!   CORE_ADDR pc;
! 
!   pc = frame->pc;
!   if (frame->next != 0 && frame->next->signal_handler_caller == 0)
!     /* We are not in the innermost frame and we were not interrupted
!        by a signal.  We need to subtract one to get the correct block,
!        in case the call instruction was the last instruction of the block.
!        If there are any machines on which the saved pc does not point to
!        after the call insn, we probably want to make frame->pc point after
!        the call insn anyway.  */
!     --pc;
  
    if (addr_in_block)
      *addr_in_block = pc;
--- 596,602 ----
  struct block *
  get_frame_block (struct frame_info *frame, CORE_ADDR *addr_in_block)
  {
!   const CORE_ADDR pc = get_frame_calling_pc (frame);
  
    if (addr_in_block)
      *addr_in_block = pc;
*************** block_innermost_frame (struct block *blo
*** 970,975 ****
--- 980,986 ----
    struct frame_info *frame;
    register CORE_ADDR start;
    register CORE_ADDR end;
+   CORE_ADDR calling_pc;
  
    if (block == NULL)
      return NULL;
*************** block_innermost_frame (struct block *blo
*** 983,989 ****
        frame = get_prev_frame (frame);
        if (frame == NULL)
  	return NULL;
!       if (frame->pc >= start && frame->pc < end)
  	return frame;
      }
  }
--- 994,1001 ----
        frame = get_prev_frame (frame);
        if (frame == NULL)
  	return NULL;
!       calling_pc = get_frame_calling_pc (frame);
!       if (calling_pc >= start && calling_pc < end)
  	return frame;
      }
  }