Re: [PATCH 1/2] avoid infinite loop with bad debuginfo

[Pedro Alves <palves@redhat.com>]

On 11/13/2013 08:51 PM, Tom Tromey wrote:
>        while (VALUE_LVAL (new_val) == lval_register && value_lazy (new_val))
>  	{
> -	  frame = frame_find_by_id (VALUE_FRAME_ID (new_val));
> +	  struct frame_id last_frame_id = VALUE_FRAME_ID (new_val);
> +
> +	  frame = frame_find_by_id (last_frame_id);
>  	  regnum = VALUE_REGNUM (new_val);
>  
>  	  gdb_assert (frame != NULL);
> @@ -3521,6 +3523,11 @@ value_fetch_lazy (struct value *val)
>  						   regnum, type));
>  
>  	  new_val = get_frame_register_value (frame, regnum);
> +	  if (VALUE_LVAL (new_val) == lval_register
> +	      && value_lazy (new_val)
> +	      && frame_id_eq (VALUE_FRAME_ID (new_val), last_frame_id))

I think this should also check the regnum:

	  if (VALUE_LVAL (new_val) == lval_register
	      && value_lazy (new_val)
	      && last_regnum == VALUE_REGNUM (new_val);
	      && frame_id_eq (VALUE_FRAME_ID (new_val), last_frame_id))

Makes sense to me with that change.  But see below.  It seems very odd
to me that we'd get into a situation where we have two frames with the
same id.

> +	    error (_("infinite loop while fetching a register; "
> +		     "probably bad debug info"));

What swallows this error?

As it leads to:

    Backtrace stopped: previous frame identical to this frame (corrupt stack?)

I'd mildly suggest changing the new error to match (corrupt stack?)

	    error (_("infinite loop while fetching a register (corrupt stack?)"));

However,

>     #4  0x0000007fb7f0956c in clone () from /lib64/libc.so.6
>     #5  0x0000007fb7f0956c in clone () from /lib64/libc.so.6
>     Backtrace stopped: previous frame identical to this frame (corrupt stack?)

Doesn't this all then mean that we somehow ended up with two identical
frames with the same id on the frame chain (#4 and #5) ?
That seems very wrong to me.

It seems to be a better fix would be to make
get_prev_frame_1/get_prev_frame_raw discard frame #5 before it
was ever linked in.  Either that, or, if we really need to keep
#5 linked in, we should find a way for frame_id_eq (#4, #5) to
return false.

-- 
Pedro Alves