Re: [RFA] fix popping of dummy frame if inferior gets signal with unwindonsignal

["Ulrich Weigand" <uweigand@de.ibm.com>]

Doug Evans wrote:

> Hi.  This fixes a bug where a signal in an inferior function call with
> unwindonsignal set doesn't remove the dummy frame from dummy_frame_stack.
> A testcase is included to exercise the bug.

I agree that it would be better if call_function_by_hand were to call
dummy_frame_pop in this case.  However, why exactly is this a bug?

In general, the list of dummy frames is expected to occasionally contains
stale entries (e.g. if the inferior jumped out of an inferior call via
longjmp).  Your test case only uses "maint pring dummy-frames" to verify
the dummy frame list -- apart from this, was there any actual problem
you experienced due to this issue?

>  	      /* We must get back to the frame we were before the
> -		 dummy call. */
> -	      frame_pop (get_current_frame ());
> +		 dummy call.
> +		 NOTE: This discards the regcache recorded in the dummy frame
> +		 but that's ok, the registers will get restored from the
> +		 regcache in inf_status (which gets called by a cleanup).  */
> +	      dummy_frame_pop (dummy_id);
> +	      reinit_frame_cache ();

Why the reinit_frame_cache here?

Michael Synder wrote:

> I don't understand why calling dummy_frame_pop directly works,
> but calling frame_pop doesn't work.  Doesn't frame_pop call
> dummy_frame_pop?  If not, what prevents it?

frame_pop will call dummy_frame_pop if you pop the dummy frame
directly.  However, in this case, frame_pop (get_current_frame ())
only pops the current frame during the inferior call sequence, 
and the dummy frame is somewhere higher on the stack.

I guess you could do a
  frame_pop (find_frame_by_id (dummy_id))
instead, but as Doug points out this is not really needed because
of the outstanding inf_status cleanup.

On the other hand, I'm wondering if we really need the inf_status
cleanup in the first place.  It supposed to protect against errors
during the initial setup of the registers, see the comment in infrun.c:

  /* These are here because if call_function_by_hand has written some
     registers and then decides to call error(), we better not have changed
     any registers.  */
  struct regcache *registers;

I'm not sure what the point is of having two copies of the regcache
preserved across the full extent of the inferior call ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com