Re: PR13901

[Tristan Gingold <gingold@adacore.com>]

On Apr 4, 2012, at 4:22 PM, Pedro Alves wrote:

> On 04/03/2012 08:28 AM, John Gilmore wrote:
> 
>>> If we can't skip darwin_set_sstep for all continues that are not single-steps,
>>> we could at least skip those while starting up (when continuing the shell
>>> until we see enough execs).  That'd suggest a new flag like
>>> darwin-nat.h:struct private_inferior->starting_up, set and cleared in
>>> darwin_create_inferior, and then making darwin_resume_thread do ...
>> 
>> When I was maintaining GDB (many years ago), touching *anything* in
>> the state machine that figured out what to do next when the inferior
>> stopped was guaranteed to produce several bugs for every fix.
> 
>> In the changes we made, I tried to reduce that tendency, and make
> 
>> the code more modular and less fragile.  Today, "just" adding a new
>> flag for this may be as simple as you hope.
> 
> 
> Oh, a challenge!  ;-)
> 
> I think this should work, but I'm not set up for testing it...

Welcome in the Darwin team :-)

I was thinking about a slightly better way (IMHO): just call darwin_set_sstep when
the single-step flag is set or was just cleared.  This should improve performance.

There might be subtile difference for signal handling, but I am not even sure that gdb behaviour is well defined in that case.

WDYT ?

> As noted in a comment below, this isn't just about the shell; a
> "set exec-wrapper WRAPPER" wrapper of the wrong bitness should also
> trigger the original problem.

Sure.

Tristan.

> 
> 2012-04-04  Pedro Alves  <palves@redhat.com>
> 
> 	* darwin-nat.c (darwin_resume_thread): Don't set the thread to
> 	single-step if the inferior is still starting up.
> 	(darwin_ptrace_him): Set and clear the new starting_up flag.
> 	* darwin-nat.h (struct private_inferior) <starting_up>: New flag.
> ---
> 
> gdb/darwin-nat.c |   18 ++++++++++++++----
> gdb/darwin-nat.h |    4 ++++
> 2 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
> index 010700c..bdd174b 100644
> --- a/gdb/darwin-nat.c
> +++ b/gdb/darwin-nat.c
> @@ -705,10 +705,18 @@ darwin_resume_thread (struct inferior *inf, darwin_thread_t *thread,
> 	  thread->signaled = 1;
> 	}
> 
> -      /* Set single step.  */
> -      inferior_debug (4, _("darwin_set_sstep (thread=%x, enable=%d)\n"),
> -                      thread->gdb_port, step);
> -      darwin_set_sstep (thread->gdb_port, step);
> +      /* If our target process hasn't been exec'ed yet, when avoid
> +	 accessing anything in the inferior (registers, memory, etc.).
> +	 We might have spawned a 64-bit shell while debugging a 32-bit
> +	 program.  */
> +      gdb_assert (!inf->private->starting_up || !step);
> +      if (!inf->private->starting_up)
> +	{
> +	  /* Set single step.  */
> +	  inferior_debug (4, _("darwin_set_sstep (thread=%x, enable=%d)\n"),
> +			  thread->gdb_port, step);
> +	  darwin_set_sstep (thread->gdb_port, step);
> +	}
>       thread->single_step = step;
> 
>       darwin_send_reply (inf, thread);
> @@ -1505,7 +1513,9 @@ darwin_ptrace_him (int pid)
> 
>   darwin_init_thread_list (inf);
> 
> +  inf->private->starting_up = 1;
>   startup_inferior (START_INFERIOR_TRAPS_EXPECTED);
> +  inf->private->starting_up = 0;
> }
> 
> static void
> diff --git a/gdb/darwin-nat.h b/gdb/darwin-nat.h
> index 6c89299..d9d4e33 100644
> --- a/gdb/darwin-nat.h
> +++ b/gdb/darwin-nat.h
> @@ -126,6 +126,10 @@ struct private_inferior
> 
>   /* Sorted vector of known threads.  */
>   VEC(darwin_thread_t) *threads;
> +
> +  /* True if starting up (going through the shell, or an
> +     exec-wrapper).  */
> +  int starting_up;
> };
> typedef struct private_inferior darwin_inferior;
>