[RFA] block_innermost_frame tweak

[RFA] block_innermost_frame tweak

[Joel Brobecker]

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

I would like to make the following change:

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

        * blockframe.c (block_innermost_frame): Fix a boundary condition
        bug that was causing GDB to sometimes fail to find the frame
        executing inside the given block.

I verified this change on Linux, and this introduced no regression.

We found out about this problem when we tried in a very particular Ada
program to print the value of a variable. Here are the details:

The Ada program (sorry, I tried the equivalent C program, but the
problem did not reproduce there):
<<
           procedure Try is
              procedure Inside is
              begin
                null;
              end Inside;
           begin
              declare
                Local : Integer := 18;
              begin
                Inside;
              end;
           end Try;
>>

After hitting a  breakpoint inside procedure "Inside", then going one
frame up, GDB should be able to print the value of "Local", but instead
says:

           No frame is currently executing in specified block

This is because selected_frame->pc points to the instruction following
the call to inside (ie this is more a return address than the pc), which
is right at the boundary of the block for which we are trying to find
the innermost frame... Hence the change I am suggesting.

OK to commit?

By the way, I think we have several places where we check whether a
frame is an innermost frame or not, so I think it would be useful to
create a public function in frame.h that would look like this:

     extern int frame_innermost_p (struct frame_info *);

(implemented in blockframe.c). I can submit this addition as a followup
patch if you think this is a good idea.

Thanks,
-- 
Joel

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

Index: blockframe.c
===================================================================
RCS file: /cvs/src/src/gdb/blockframe.c,v
retrieving revision 1.29
diff -u -3 -p -r1.29 blockframe.c
--- blockframe.c	8 Jun 2002 18:30:14 -0000	1.29
+++ blockframe.c	20 Jun 2002 19:54:13 -0000
@@ -970,6 +970,7 @@ block_innermost_frame (struct block *blo
   struct frame_info *frame;
   register CORE_ADDR start;
   register CORE_ADDR end;
+  int innermost;
 
   if (block == NULL)
     return NULL;
@@ -983,7 +984,18 @@ block_innermost_frame (struct block *blo
       frame = get_prev_frame (frame);
       if (frame == NULL)
 	return NULL;
-      if (frame->pc >= start && frame->pc < end)
+      /* To evaluate if a given PC address is within a block range,
+         we normally check if PC is inside [block start .. block end[.
+         However, If FRAME is not the innermost frame, then frame->pc
+         normally points to the instruction *following* the call, which
+         means that the comparison with the block boundaries need to be
+         offset by one instruction. This is done by checking if PC is
+         inside ]block start .. block end] instead.  */
+      innermost = !frame->next
+                    || frame->next->signal_handler_caller
+                    || frame_in_dummy (frame->next);
+      if ((innermost && frame->pc >= start && frame->pc < end)
+          || (!innermost && frame->pc > start && frame->pc <= end))
 	return frame;
     }
 }

Re: [RFA] block_innermost_frame tweak

[Jim Blandy]

This is similar to the code in get_frame_block that decrements the
PC before looking up its block.

It seems to me that we generally want to use the return address - 1
(well, ideally, the address of the `call' instruction, but we don't
have that) to decide which block any frame but the youngest is "in".

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?

Joel Brobecker <brobecker@gnat.com> writes:

> I would like to make the following change:
> 
> 2002-06-20  Joel Brobecker  <brobecker@gnat.com>
> 
>         * blockframe.c (block_innermost_frame): Fix a boundary condition
>         bug that was causing GDB to sometimes fail to find the frame
>         executing inside the given block.
> 
> I verified this change on Linux, and this introduced no regression.
> 
> We found out about this problem when we tried in a very particular Ada
> program to print the value of a variable. Here are the details:
> 
> The Ada program (sorry, I tried the equivalent C program, but the
> problem did not reproduce there):
> <<
>            procedure Try is
>               procedure Inside is
>               begin
>                 null;
>               end Inside;
>            begin
>               declare
>                 Local : Integer := 18;
>               begin
>                 Inside;
>               end;
>            end Try;
> >>
> 
> After hitting a  breakpoint inside procedure "Inside", then going one
> frame up, GDB should be able to print the value of "Local", but instead
> says:
> 
>            No frame is currently executing in specified block
> 
> This is because selected_frame->pc points to the instruction following
> the call to inside (ie this is more a return address than the pc), which
> is right at the boundary of the block for which we are trying to find
> the innermost frame... Hence the change I am suggesting.
> 
> OK to commit?
> 
> By the way, I think we have several places where we check whether a
> frame is an innermost frame or not, so I think it would be useful to
> create a public function in frame.h that would look like this:
> 
>      extern int frame_innermost_p (struct frame_info *);
> 
> (implemented in blockframe.c). I can submit this addition as a followup
> patch if you think this is a good idea.
> 
> Thanks,
> -- 
> Joel
> 
> Index: blockframe.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/blockframe.c,v
> retrieving revision 1.29
> diff -u -3 -p -r1.29 blockframe.c
> --- blockframe.c	8 Jun 2002 18:30:14 -0000	1.29
> +++ blockframe.c	20 Jun 2002 19:54:13 -0000
> @@ -970,6 +970,7 @@ block_innermost_frame (struct block *blo
>    struct frame_info *frame;
>    register CORE_ADDR start;
>    register CORE_ADDR end;
> +  int innermost;
>  
>    if (block == NULL)
>      return NULL;
> @@ -983,7 +984,18 @@ block_innermost_frame (struct block *blo
>        frame = get_prev_frame (frame);
>        if (frame == NULL)
>  	return NULL;
> -      if (frame->pc >= start && frame->pc < end)
> +      /* To evaluate if a given PC address is within a block range,
> +         we normally check if PC is inside [block start .. block end[.
> +         However, If FRAME is not the innermost frame, then frame->pc
> +         normally points to the instruction *following* the call, which
> +         means that the comparison with the block boundaries need to be
> +         offset by one instruction. This is done by checking if PC is
> +         inside ]block start .. block end] instead.  */
> +      innermost = !frame->next
> +                    || frame->next->signal_handler_caller
> +                    || frame_in_dummy (frame->next);
> +      if ((innermost && frame->pc >= start && frame->pc < end)
> +          || (!innermost && frame->pc > start && frame->pc <= end))
>  	return frame;
>      }
>  }

Re: [RFA] block_innermost_frame tweak

[Joel Brobecker]

[-- 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;
      }
  }

Re: [RFA] block_innermost_frame tweak

[Andrew Cagney]

> This is similar to the code in get_frame_block that decrements the
> PC before looking up its block.
> 
> It seems to me that we generally want to use the return address - 1
> (well, ideally, the address of the `call' instruction, but we don't
> have that) to decide which block any frame but the youngest is "in".
> 
> 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?

Is there a better name?  As you note, it isn't a valid PC (it may not 
even point into an instruction!).  Further, it is isn't the address of 
the instruction ``calling'' the ``frame''.  Last time this came up 
address_in_block() was used - frame_address_in_block()?

> +      /* To evaluate if a given PC address is within a block range,
> +         we normally check if PC is inside [block start .. block end[.
> +         However, If FRAME is not the innermost frame, then frame->pc
> +         normally points to the instruction *following* the call, which
> +         means that the comparison with the block boundaries need to be
> +         offset by one instruction. This is done by checking if PC is
> +         inside ]block start .. block end] instead.  */
> +      innermost = !frame->next
> +                    || frame->next->signal_handler_caller
> +                    || frame_in_dummy (frame->next);
> +      if ((innermost && frame->pc >= start && frame->pc < end)
> +          || (!innermost && frame->pc > start && frame->pc <= end))
>  	return frame;

Just BTW frame_in_dummy() isn't an indication of an innermost frame. 
There can easily be other frames (and even dummy frames) more inner most 
than that dummy frame.

Andrew

Re: [RFA] block_innermost_frame tweak

[Joel Brobecker]

> Is there a better name?  As you note, it isn't a valid PC (it may not 
> even point into an instruction!).  Further, it is isn't the address of 
> the instruction ``calling'' the ``frame''.  Last time this came up 
> address_in_block() was used - frame_address_in_block()?

I have no problem renaming it to frame_address_in_block, provided we all
agree on the name.

> Just BTW frame_in_dummy() isn't an indication of an innermost frame. 
> There can easily be other frames (and even dummy frames) more inner most 
> than that dummy frame.

Right. I noticed this when I reworked my changes, but thanks.

-- 
Joel

Re: [RFA] block_innermost_frame tweak

[Jim Blandy]

Andrew Cagney <ac131313@cygnus.com> writes:
> Is there a better name?  As you note, it isn't a valid PC (it may not
> even point into an instruction!).  Further, it is isn't the address of
> the instruction ``calling'' the ``frame''.  Last time this came up
> address_in_block() was used - frame_address_in_block()?

I don't disagree with your objections (raised in a previous
discussion) that `frame->pc - 1' isn't a proper PC.  It may never have
been the value of the PC register (if indeed the architecture has a
register named `PC'); it doesn't even necessarily point to an
instruction.

That said, I feel that replacing "PC" with just "address" actually
makes matters worse, not better.  It's very helpful to see at a glance
that a particular CORE_ADDR value is a pointer into the instruction
stream.  The exact semantics of the value --- is this the return
address or the address of the call?  do we need to apply
DECR_PC_AFTER_BREAK?  and so on --- is something that one uncovers
when one researches the value more carefully, as with anything else.

But if I'm the only one who has this reaction, then I don't mind the
renaming.

Is there some third terse term that indicates (or could indicate, by
establishing a convention) "pointer into the instruction stream that
isn't necessarily an instruction address or the value of a register"?

Re: [RFA] block_innermost_frame tweak

[Andrew Cagney]

> Andrew Cagney <ac131313@cygnus.com> writes:
> 
>> Is there a better name?  As you note, it isn't a valid PC (it may not
>> even point into an instruction!).  Further, it is isn't the address of
>> the instruction ``calling'' the ``frame''.  Last time this came up
>> address_in_block() was used - frame_address_in_block()?
> 
> 
> I don't disagree with your objections (raised in a previous
> discussion) that `frame->pc - 1' isn't a proper PC.  It may never have
> been the value of the PC register (if indeed the architecture has a
> register named `PC'); it doesn't even necessarily point to an
> instruction.
> 
> That said, I feel that replacing "PC" with just "address" actually
> makes matters worse, not better.  It's very helpful to see at a glance
> that a particular CORE_ADDR value is a pointer into the instruction
> stream.  The exact semantics of the value --- is this the return
> address or the address of the call?  do we need to apply
> DECR_PC_AFTER_BREAK?  and so on --- is something that one uncovers
> when one researches the value more carefully, as with anything else.

Except for a small window in WFI, a ``PC'' refers to the address of the 
instruction that will be executed next.  It is just unfortunate that no 
one has found the time to zap DECR_PC_AFTER_BREAK and hence eliminate 
that small window.  Please don't add to this confusion.

> But if I'm the only one who has this reaction, then I don't mind the
> renaming.
> 
> Is there some third terse term that indicates (or could indicate, by
> establishing a convention) "pointer into the instruction stream that
> isn't necessarily an instruction address or the value of a register"?

The reason for suggesting ``block'' was that it hopefully implies a code 
block. frame_address_within_code_block()?

enjoy,
Andrew

Re: [RFA] block_innermost_frame tweak

[Jim Blandy]

Andrew Cagney <ac131313@cygnus.com> writes:
> Except for a small window in WFI, a ``PC'' refers to the address of
> the instruction that will be executed next.  It is just unfortunate
> that no one has found the time to zap DECR_PC_AFTER_BREAK and hence
> eliminate that small window.  Please don't add to this confusion.

What about all the prologue analyzers?

    /* Return PC of first real instruction.  */

    int
    i386_skip_prologue (int pc)
    {
      unsigned char op;
      int i;

    static CORE_ADDR
    mn10300_analyze_prologue (struct frame_info *fi, CORE_ADDR pc)
    {

What about the line table entries?

    /* Each item represents a line-->pc (or the reverse) mapping.  This is
       somewhat more wasteful of space than one might wish, but since only
       the files which are actually debugged are read in to core, we don't
       waste much space.  */

    struct linetable_entry
      {
        int line;
        CORE_ADDR pc;
      };

I'm all for choosing conventions and sticking to them, but in everyday
speech (well, everyday speech for debugger people), a `pc' is just any
kind of pointer to an instruction.  And it just looks to me like
that's the way GDB uses it, too.

Why is this distinction so urgent to maintain?  I think I've shown
that it has some cost.

> > But if I'm the only one who has this reaction, then I don't mind the
> > renaming.
> > Is there some third terse term that indicates (or could indicate, by
> > establishing a convention) "pointer into the instruction stream that
> > isn't necessarily an instruction address or the value of a register"?
> 
> The reason for suggesting ``block'' was that it hopefully implies a
> code block. frame_address_within_code_block()?

This is getting worse.

Re: [RFA] block_innermost_frame tweak

[Andrew Cagney]

> 
> What about all the prologue analyzers?
> 
>     /* Return PC of first real instruction.  */
> 
>     int
>     i386_skip_prologue (int pc)
>     {
>       unsigned char op;
>       int i;
> 
>     static CORE_ADDR
>     mn10300_analyze_prologue (struct frame_info *fi, CORE_ADDR pc)
>     {
> 
> What about the line table entries?
> 
>     /* Each item represents a line-->pc (or the reverse) mapping.  This is
>        somewhat more wasteful of space than one might wish, but since only
>        the files which are actually debugged are read in to core, we don't
>        waste much space.  */
> 
>     struct linetable_entry
>       {
>         int line;
>         CORE_ADDR pc;
>       };
> 
> I'm all for choosing conventions and sticking to them, but in everyday
> speech (well, everyday speech for debugger people), a `pc' is just any
> kind of pointer to an instruction.  And it just looks to me like
> that's the way GDB uses it, too.

Yes, and in each of the above, the PC designates the address of an 
instruction that, hopefully, the target would/could resume execution from.

> But if I'm the only one who has this reaction, then I don't mind the
>> > renaming.
>> > Is there some third terse term that indicates (or could indicate, by
>> > establishing a convention) "pointer into the instruction stream that
>> > isn't necessarily an instruction address or the value of a register"?
> 
>> 
>> The reason for suggesting ``block'' was that it hopefully implies a
>> code block. frame_address_within_code_block()?
> 
> 
> This is getting worse.

Oops, I guess we're back to frame_address_in_block() then?

Andrew

Re: [RFA] block_innermost_frame tweak

[Jim Blandy]

Andrew Cagney <ac131313@cygnus.com> writes:
> Oops, I guess we're back to frame_address_in_block() then?

Sure thing.

Re: [RFA] block_innermost_frame tweak

[Joel Brobecker]

> Andrew Cagney <ac131313@cygnus.com> writes:
> > Oops, I guess we're back to frame_address_in_block() then?
> 
> Sure thing.

Sorry for letting this drag for so long. So is the last patch I
submitted fine if I replace get_frame_calling_pc by
frame_address_in_block?

I am including the previous changelog just as a "refresher".

Thanks,
-- 
Joel

Re: [RFA] block_innermost_frame tweak

[Jim Blandy]

Joel Brobecker <brobecker@gnat.com> writes:
> > Andrew Cagney <ac131313@cygnus.com> writes:
> > > Oops, I guess we're back to frame_address_in_block() then?
> > 
> > Sure thing.
> 
> Sorry for letting this drag for so long. So is the last patch I
> submitted fine if I replace get_frame_calling_pc by
> frame_address_in_block?

The latter is the name we all agreed on, right?  I think it's okay,
then.

> I am including the previous changelog just as a "refresher".

No you aren't.  :)

Re: [RFA] block_innermost_frame tweak

[Joel Brobecker]

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

> The latter is the name we all agreed on, right?  I think it's okay,
> then.

That's what I understood, but not being a native english speaker, I
sometime have doubts about what I understand.

I committed the attached change (I'll try to remember to add the
attachement this time :). Basically, I just did a search and replace
to use the name we agreed on.

2002-07-02  Joel Brobecker  <brobecker@gnat.com>

        * frame.h (frame_address_in_block): New function.

        * blockframe.c (frame_address_in_block): New function extracted
        from get_frame_block().
        (get_frame_block): Use frame_address_in_block().
        (block_innermost_frame): Use frame_address_in_block() to match
        the frame pc address 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. 

Thanks for the feedback,
-- 
Joel

[-- Attachment #2: frame.diff --]
[-- Type: text/plain, Size: 3553 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	2 Jul 2002 18:21:32 -0000
*************** extern struct symbol *get_frame_function
*** 250,255 ****
--- 250,257 ----
  
  extern CORE_ADDR get_frame_pc (struct frame_info *);
  
+ extern CORE_ADDR frame_address_in_block (struct frame_info *);
+ 
  extern CORE_ADDR get_pc_function_start (CORE_ADDR);
  
  extern struct block *block_for_pc (CORE_ADDR);
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	2 Jul 2002 18:21:32 -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
+ frame_address_in_block (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 = frame_address_in_block (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 = frame_address_in_block (frame);
!       if (calling_pc >= start && calling_pc < end)
  	return frame;
      }
  }