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

[Pedro Alves <palves at redhat dot com>]

On 11/18/2013 06:23 PM, Tom Tromey wrote:
>>> +	  if (VALUE_LVAL (new_val) == lval_register
>>> +	      && value_lazy (new_val)
>>> +	      && frame_id_eq (VALUE_FRAME_ID (new_val), last_frame_id))
> 
> Pedro> I think this should also check the regnum:
> 
> Barf.  I have a memory of actually writing that.  False memory I guess.
> Sigh.
> 
>>> #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?)
> 
> Pedro> Doesn't this all then mean that we somehow ended up with two identical
> Pedro> frames with the same id on the frame chain (#4 and #5) ?
> Pedro> That seems very wrong to me.
> 
> Pedro> It seems to be a better fix would be to make
> Pedro> get_prev_frame_1/get_prev_frame_raw discard frame #5 before it
> Pedro> was ever linked in.  Either that, or, if we really need to keep
> Pedro> #5 linked in, we should find a way for frame_id_eq (#4, #5) to
> Pedro> return false.
> 
> I will look into it, but my recollection is that last time we got into
> this area, it was somehow undesirable to undo whatever changes were done
> by existing frame sniffers.

Hmm, I don't think that's the same issue.   I'm just talking about
something like moving the frame_id_eq check to the end of
get_prev_frame_1, after calling get_prev_frame_raw.  Something
like:

  prev_frame = get_prev_frame_raw (this_frame);

  /* Check that this and the prev frame are not identical.  If they
     are, there is most likely a stack cycle.  */
  if (prev_frame != NULL
      && frame_id_eq (get_frame_id (prev_frame),
		      get_frame_id (this_frame)))
    {
      if (frame_debug)
	{
	  fprintf_unfiltered (gdb_stdlog, "-> ");
	  fprint_frame (gdb_stdlog, NULL);
	  fprintf_unfiltered (gdb_stdlog, " // this frame has same ID }\n");
	}
      this_frame->stop_reason = UNWIND_SAME_ID;
      /* Unlink.  */
      this_frame->prev = NULL;
      return NULL;
    }

  return prev_frame;
}

-- 
Pedro Alves