Re: [RFA] block_innermost_frame tweak

[Jim Blandy <jimb@redhat.com>]

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