Re: PR13901

Tristan Gingold
2012-04-04T14:47:19Z

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