PR13901

PR13901

[Jack Howarth]

Tristan,
   Have you noticed that recent FSF gdb releases including 7.4 have been
non-functional when built targeting i386-apple-darwin? The run-time failures
have changed between 7.2 and 7.4. Currently the failure is exhibited as
errors of the form...

(gdb) break main
Breakpoint 1 at 0xd80: file himenoBMTxpa.c, line 71.
(gdb) r
Starting program: /Users/howarth/a.out 
darwin_set_sstep: unknown flavour: 4
Error calling thread_get_state for GP registers for thread 0x8451lxwarning:
Mach error at "i386-darwin-nat.c:118" in function
"i386_darwin_fetch_inferior_registers": (os/kern) invalid argument (0x4)

http://sourceware.org/bugzilla/show_bug.cgi?id=13901

Is this something that is trivial to fix? It would be nice if both MacPorts
and fink could have a functional i386 build of FSF gdb. In fink, we currently
have gdb restricted to x86_64 fink (although the x86_64-apple-darwin build
can debug i386 binaries fine). MacPorts has left their gdb pacakge at 7.1
(which I guess is that last version that worked for both i386-apple-darwin
and x86_64-apple-darwin. Thanks in advance for any clarifications.
           Jack

Re: PR13901

[Tristan Gingold]

On Mar 30, 2012, at 3:42 PM, Jack Howarth wrote:

> Tristan,
>   Have you noticed that recent FSF gdb releases including 7.4 have been
> non-functional when built targeting i386-apple-darwin? The run-time failures
> have changed between 7.2 and 7.4. Currently the failure is exhibited as
> errors of the form...
> 
> (gdb) break main
> Breakpoint 1 at 0xd80: file himenoBMTxpa.c, line 71.
> (gdb) r
> Starting program: /Users/howarth/a.out 
> darwin_set_sstep: unknown flavour: 4
> Error calling thread_get_state for GP registers for thread 0x8451lxwarning:
> Mach error at "i386-darwin-nat.c:118" in function
> "i386_darwin_fetch_inferior_registers": (os/kern) invalid argument (0x4)
> 
> http://sourceware.org/bugzilla/show_bug.cgi?id=13901
> 
> Is this something that is trivial to fix? It would be nice if both MacPorts
> and fink could have a functional i386 build of FSF gdb. In fink, we currently
> have gdb restricted to x86_64 fink (although the x86_64-apple-darwin build
> can debug i386 binaries fine). MacPorts has left their gdb pacakge at 7.1
> (which I guess is that last version that worked for both i386-apple-darwin
> and x86_64-apple-darwin. Thanks in advance for any clarifications.

I think that should be fixable.  Will investigate.
I haven't built for i386 for a long time...

Tristan.

Re: PR13901

[Tristan Gingold]

On Mar 30, 2012, at 3:42 PM, Jack Howarth wrote:

> Tristan,
>   Have you noticed that recent FSF gdb releases including 7.4 have been
> non-functional when built targeting i386-apple-darwin? The run-time failures
> have changed between 7.2 and 7.4. Currently the failure is exhibited as
> errors of the form...
> 
> (gdb) break main
> Breakpoint 1 at 0xd80: file himenoBMTxpa.c, line 71.
> (gdb) r
> Starting program: /Users/howarth/a.out 
> darwin_set_sstep: unknown flavour: 4
> Error calling thread_get_state for GP registers for thread 0x8451lxwarning:
> Mach error at "i386-darwin-nat.c:118" in function
> "i386_darwin_fetch_inferior_registers": (os/kern) invalid argument (0x4)
> 
> http://sourceware.org/bugzilla/show_bug.cgi?id=13901
> 
> Is this something that is trivial to fix? It would be nice if both MacPorts
> and fink could have a functional i386 build of FSF gdb. In fink, we currently
> have gdb restricted to x86_64 fink (although the x86_64-apple-darwin build
> can debug i386 binaries fine). MacPorts has left their gdb pacakge at 7.1
> (which I guess is that last version that worked for both i386-apple-darwin
> and x86_64-apple-darwin. Thanks in advance for any clarifications.

I now understand the issue:

gdb spawns bash to run the program, but the bash spawned is 64 bits, which is not understood by gdb...

Tristan.

Re: PR13901

[Tristan Gingold]

On Apr 2, 2012, at 12:44 PM, Tristan Gingold wrote:

> 
> On Mar 30, 2012, at 3:42 PM, Jack Howarth wrote:
> 
>> Tristan,
>>  Have you noticed that recent FSF gdb releases including 7.4 have been
>> non-functional when built targeting i386-apple-darwin? The run-time failures
>> have changed between 7.2 and 7.4. Currently the failure is exhibited as
>> errors of the form...
>> 
>> (gdb) break main
>> Breakpoint 1 at 0xd80: file himenoBMTxpa.c, line 71.
>> (gdb) r
>> Starting program: /Users/howarth/a.out 
>> darwin_set_sstep: unknown flavour: 4
>> Error calling thread_get_state for GP registers for thread 0x8451lxwarning:
>> Mach error at "i386-darwin-nat.c:118" in function
>> "i386_darwin_fetch_inferior_registers": (os/kern) invalid argument (0x4)
>> 
>> http://sourceware.org/bugzilla/show_bug.cgi?id=13901
>> 
>> Is this something that is trivial to fix? It would be nice if both MacPorts
>> and fink could have a functional i386 build of FSF gdb. In fink, we currently
>> have gdb restricted to x86_64 fink (although the x86_64-apple-darwin build
>> can debug i386 binaries fine). MacPorts has left their gdb pacakge at 7.1
>> (which I guess is that last version that worked for both i386-apple-darwin
>> and x86_64-apple-darwin. Thanks in advance for any clarifications.
> 
> I now understand the issue:
> 
> gdb spawns bash to run the program, but the bash spawned is 64 bits, which is not understood by gdb...

Fixed by this patch (committed on trunk):

2012-04-02  Tristan Gingold  <gingold@adacore.com>

	PR gdb/13901
	* darwin-nat.c (darwin_execvp): Sey binary preference.

Index: darwin-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/darwin-nat.c,v
retrieving revision 1.31
diff -c -r1.31 darwin-nat.c
*** darwin-nat.c	14 Mar 2012 01:46:59 -0000	1.31
--- darwin-nat.c	2 Apr 2012 11:55:36 -0000
***************
*** 39,44 ****
--- 39,45 ----
  #include "value.h"
  #include "arch-utils.h"
  #include "bfd.h"
+ #include "bfd/mach-o.h"
  
  #include <sys/ptrace.h>
  #include <sys/signal.h>
***************
*** 1538,1543 ****
--- 1539,1560 ----
        return;
      }
  
+   /* Specify the same binary preference to spawn the shell as the
+      exec binary.  This avoids spawning a 64bit shell while debugging
+      a 32bit program, which may confuse gdb.
+      Also, this slightly breaks internal layers as we suppose the binary
+      is Mach-O.  Doesn't harm in practice.  */
+   if (exec_bfd != NULL)
+     {
+       cpu_type_t pref;
+       size_t ocount;
+ 
+       pref = bfd_mach_o_get_data (exec_bfd)->header.cputype;
+       res = posix_spawnattr_setbinpref_np (&attr, 1, &pref, &ocount);
+       if (res != 0 || ocount != 1)
+ 	fprintf_unfiltered (gdb_stderr, "Cannot set posix_spawn binpref\n");
+     }
+ 
    posix_spawnp (NULL, argv[0], NULL, &attr, argv, env);
  }
  

Re: PR13901

[Jack Howarth]

On Mon, Apr 02, 2012 at 01:57:31PM +0200, Tristan Gingold wrote:
> 
> On Apr 2, 2012, at 12:44 PM, Tristan Gingold wrote:
> 
> > 
> > On Mar 30, 2012, at 3:42 PM, Jack Howarth wrote:
> > 
> >> Tristan,
> >>  Have you noticed that recent FSF gdb releases including 7.4 have been
> >> non-functional when built targeting i386-apple-darwin? The run-time failures
> >> have changed between 7.2 and 7.4. Currently the failure is exhibited as
> >> errors of the form...
> >> 
> >> (gdb) break main
> >> Breakpoint 1 at 0xd80: file himenoBMTxpa.c, line 71.
> >> (gdb) r
> >> Starting program: /Users/howarth/a.out 
> >> darwin_set_sstep: unknown flavour: 4
> >> Error calling thread_get_state for GP registers for thread 0x8451lxwarning:
> >> Mach error at "i386-darwin-nat.c:118" in function
> >> "i386_darwin_fetch_inferior_registers": (os/kern) invalid argument (0x4)
> >> 
> >> http://sourceware.org/bugzilla/show_bug.cgi?id=13901
> >> 
> >> Is this something that is trivial to fix? It would be nice if both MacPorts
> >> and fink could have a functional i386 build of FSF gdb. In fink, we currently
> >> have gdb restricted to x86_64 fink (although the x86_64-apple-darwin build
> >> can debug i386 binaries fine). MacPorts has left their gdb pacakge at 7.1
> >> (which I guess is that last version that worked for both i386-apple-darwin
> >> and x86_64-apple-darwin. Thanks in advance for any clarifications.
> > 
> > I now understand the issue:
> > 
> > gdb spawns bash to run the program, but the bash spawned is 64 bits, which is not understood by gdb...
> 
> Fixed by this patch (committed on trunk):
> 
> 2012-04-02  Tristan Gingold  <gingold@adacore.com>
> 
> 	PR gdb/13901
> 	* darwin-nat.c (darwin_execvp): Sey binary preference.
> 
> Index: darwin-nat.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/darwin-nat.c,v
> retrieving revision 1.31
> diff -c -r1.31 darwin-nat.c
> *** darwin-nat.c	14 Mar 2012 01:46:59 -0000	1.31
> --- darwin-nat.c	2 Apr 2012 11:55:36 -0000
> ***************
> *** 39,44 ****
> --- 39,45 ----
>   #include "value.h"
>   #include "arch-utils.h"
>   #include "bfd.h"
> + #include "bfd/mach-o.h"
>   
>   #include <sys/ptrace.h>
>   #include <sys/signal.h>
> ***************
> *** 1538,1543 ****
> --- 1539,1560 ----
>         return;
>       }
>   
> +   /* Specify the same binary preference to spawn the shell as the
> +      exec binary.  This avoids spawning a 64bit shell while debugging
> +      a 32bit program, which may confuse gdb.
> +      Also, this slightly breaks internal layers as we suppose the binary
> +      is Mach-O.  Doesn't harm in practice.  */
> +   if (exec_bfd != NULL)
> +     {
> +       cpu_type_t pref;
> +       size_t ocount;
> + 
> +       pref = bfd_mach_o_get_data (exec_bfd)->header.cputype;
> +       res = posix_spawnattr_setbinpref_np (&attr, 1, &pref, &ocount);
> +       if (res != 0 || ocount != 1)
> + 	fprintf_unfiltered (gdb_stderr, "Cannot set posix_spawn binpref\n");
> +     }
> + 
>     posix_spawnp (NULL, argv[0], NULL, &attr, argv, env);
>   }
>   

Tristan,
   This almost fixes the bug for the i386-apple-darwin10 build. However, while this
eliminates the failures when debugging i386 executables in the i386 darwin build of gdb 7.4,
it still results in the following failure when debugging x86_64 executables in the
same i386 darwin build of gdb...

% clang -arch x86_64 -g himenoBMTxpa.c
% file ./a.out
./a.out: Mach-O 64-bit executable x86_64
% fsf-gdb ./a.out
GNU gdb (GDB) 7.4
Copyright (C) 2012 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "i386-apple-darwin10.8.0".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...

warning: A handler for the OS ABI "Darwin" is not built into this configuration
of GDB.  Attempting to continue with the default i386:x86-64 settings.

Reading symbols from /Users/howarth/a.out...Reading symbols from
/Users/howarth/a.out.dSYM/Contents/Resources/DWARF/a.out...done.
done.
(gdb) break main
Breakpoint 1 at 0xd80: file himenoBMTxpa.c, line 71.
(gdb) r
Starting program: /Users/howarth/a.out 
darwin_set_sstep: unknown flavour: 4
Error calling thread_get_state for GP registers for thread 0x5379lxwarning:
Mach error at "i386-darwin-nat.c:118" in function
"i386_darwin_fetch_inferior_registers": (os/kern) invalid argument (0x4)
(gdb)

Thanks for your efforts in fixing this.
            Jack

Re: PR13901

[Tristan Gingold]

On Apr 2, 2012, at 3:17 PM, Jack Howarth wrote:

> On Mon, Apr 02, 2012 at 01:57:31PM +0200, Tristan Gingold wrote:
>> 
>> On Apr 2, 2012, at 12:44 PM, Tristan Gingold wrote:
>> 
>>> 
>>> On Mar 30, 2012, at 3:42 PM, Jack Howarth wrote:
>>> 
>>>> Tristan,
>>>> Have you noticed that recent FSF gdb releases including 7.4 have been
>>>> non-functional when built targeting i386-apple-darwin? The run-time failures
>>>> have changed between 7.2 and 7.4. Currently the failure is exhibited as
>>>> errors of the form...
>>>> 
>>>> (gdb) break main
>>>> Breakpoint 1 at 0xd80: file himenoBMTxpa.c, line 71.
>>>> (gdb) r
>>>> Starting program: /Users/howarth/a.out 
>>>> darwin_set_sstep: unknown flavour: 4
>>>> Error calling thread_get_state for GP registers for thread 0x8451lxwarning:
>>>> Mach error at "i386-darwin-nat.c:118" in function
>>>> "i386_darwin_fetch_inferior_registers": (os/kern) invalid argument (0x4)
>>>> 
>>>> http://sourceware.org/bugzilla/show_bug.cgi?id=13901
>>>> 
>>>> Is this something that is trivial to fix? It would be nice if both MacPorts
>>>> and fink could have a functional i386 build of FSF gdb. In fink, we currently
>>>> have gdb restricted to x86_64 fink (although the x86_64-apple-darwin build
>>>> can debug i386 binaries fine). MacPorts has left their gdb pacakge at 7.1
>>>> (which I guess is that last version that worked for both i386-apple-darwin
>>>> and x86_64-apple-darwin. Thanks in advance for any clarifications.
>>> 
>>> I now understand the issue:
>>> 
>>> gdb spawns bash to run the program, but the bash spawned is 64 bits, which is not understood by gdb...
>> 
>> Fixed by this patch (committed on trunk):
>> 
>> 2012-04-02  Tristan Gingold  <gingold@adacore.com>
>> 
>> 	PR gdb/13901
>> 	* darwin-nat.c (darwin_execvp): Sey binary preference.
>> 
>> Index: darwin-nat.c
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/darwin-nat.c,v
>> retrieving revision 1.31
>> diff -c -r1.31 darwin-nat.c
>> *** darwin-nat.c	14 Mar 2012 01:46:59 -0000	1.31
>> --- darwin-nat.c	2 Apr 2012 11:55:36 -0000
>> ***************
>> *** 39,44 ****
>> --- 39,45 ----
>>  #include "value.h"
>>  #include "arch-utils.h"
>>  #include "bfd.h"
>> + #include "bfd/mach-o.h"
>> 
>>  #include <sys/ptrace.h>
>>  #include <sys/signal.h>
>> ***************
>> *** 1538,1543 ****
>> --- 1539,1560 ----
>>        return;
>>      }
>> 
>> +   /* Specify the same binary preference to spawn the shell as the
>> +      exec binary.  This avoids spawning a 64bit shell while debugging
>> +      a 32bit program, which may confuse gdb.
>> +      Also, this slightly breaks internal layers as we suppose the binary
>> +      is Mach-O.  Doesn't harm in practice.  */
>> +   if (exec_bfd != NULL)
>> +     {
>> +       cpu_type_t pref;
>> +       size_t ocount;
>> + 
>> +       pref = bfd_mach_o_get_data (exec_bfd)->header.cputype;
>> +       res = posix_spawnattr_setbinpref_np (&attr, 1, &pref, &ocount);
>> +       if (res != 0 || ocount != 1)
>> + 	fprintf_unfiltered (gdb_stderr, "Cannot set posix_spawn binpref\n");
>> +     }
>> + 
>>    posix_spawnp (NULL, argv[0], NULL, &attr, argv, env);
>>  }
>> 
> 
> Tristan,
>   This almost fixes the bug for the i386-apple-darwin10 build. However, while this
> eliminates the failures when debugging i386 executables in the i386 darwin build of gdb 7.4,
> it still results in the following failure when debugging x86_64 executables in the
> same i386 darwin build of gdb...

I replied in bugzilla.  You simply need to configure with --enable-64-bit-bfd to enable the support of 64 bit executables.

Tristan.

> 
> % clang -arch x86_64 -g himenoBMTxpa.c
> % file ./a.out
> ./a.out: Mach-O 64-bit executable x86_64
> % fsf-gdb ./a.out
> GNU gdb (GDB) 7.4
> Copyright (C) 2012 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
> and "show warranty" for details.
> This GDB was configured as "i386-apple-darwin10.8.0".
> For bug reporting instructions, please see:
> <http://www.gnu.org/software/gdb/bugs/>...
> 
> warning: A handler for the OS ABI "Darwin" is not built into this configuration
> of GDB.  Attempting to continue with the default i386:x86-64 settings.
> 
> Reading symbols from /Users/howarth/a.out...Reading symbols from
> /Users/howarth/a.out.dSYM/Contents/Resources/DWARF/a.out...done.
> done.
> (gdb) break main
> Breakpoint 1 at 0xd80: file himenoBMTxpa.c, line 71.
> (gdb) r
> Starting program: /Users/howarth/a.out 
> darwin_set_sstep: unknown flavour: 4
> Error calling thread_get_state for GP registers for thread 0x5379lxwarning:
> Mach error at "i386-darwin-nat.c:118" in function
> "i386_darwin_fetch_inferior_registers": (os/kern) invalid argument (0x4)
> (gdb)
> 
> Thanks for your efforts in fixing this.
>            Jack

Re: PR13901

[Pedro Alves]

On 04/02/2012 12:57 PM, Tristan Gingold wrote:

> On Apr 2, 2012, at 12:44 PM, Tristan Gingold wrote:
>> > On Mar 30, 2012, at 3:42 PM, Jack Howarth wrote:

>>> >> (gdb) break main

>>> >> Breakpoint 1 at 0xd80: file himenoBMTxpa.c, line 71.
>>> >> (gdb) r
>>> >> Starting program: /Users/howarth/a.out 
>>> >> darwin_set_sstep: unknown flavour: 4
>>> >> Error calling thread_get_state for GP registers for thread 0x8451lxwarning:
>>> >> Mach error at "i386-darwin-nat.c:118" in function
>>> >> "i386_darwin_fetch_inferior_registers": (os/kern) invalid argument (0x4)


>> > I now understand the issue:
>> > 
>> > gdb spawns bash to run the program, but the bash spawned is 64 bits, which is not understood by gdb...
> Fixed by this patch (committed on trunk):


<disclaimer>I don't know much about darwin/osx</disclaimer>

This assumes there's even a 32-bit version of the user's whatever $SHELL.
Not sure we can claim that's always true?  It also doesn't feel right
to force a different $SHELL version/build of the shell that runs if GDB
is not involved.  Very unlikely to cause problems, but still...

Why does GDB need to touch the shell's registers at all in the first place?

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:

-     /* 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);
+     /* Avoid touching the $SHELL process, and go straight to resuming it.  */
+     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);

WDYT?

-- 
Pedro Alves

Re: PR13901

[Tristan Gingold]

On Apr 2, 2012, at 3:56 PM, Pedro Alves wrote:

> On 04/02/2012 12:57 PM, Tristan Gingold wrote:
> 
>> On Apr 2, 2012, at 12:44 PM, Tristan Gingold wrote:
>>>> On Mar 30, 2012, at 3:42 PM, Jack Howarth wrote:
> 
>>>>>> (gdb) break main
> 
>>>>>> Breakpoint 1 at 0xd80: file himenoBMTxpa.c, line 71.
>>>>>> (gdb) r
>>>>>> Starting program: /Users/howarth/a.out 
>>>>>> darwin_set_sstep: unknown flavour: 4
>>>>>> Error calling thread_get_state for GP registers for thread 0x8451lxwarning:
>>>>>> Mach error at "i386-darwin-nat.c:118" in function
>>>>>> "i386_darwin_fetch_inferior_registers": (os/kern) invalid argument (0x4)
> 
> 
>>>> I now understand the issue:
>>>> 
>>>> gdb spawns bash to run the program, but the bash spawned is 64 bits, which is not understood by gdb...
>> Fixed by this patch (committed on trunk):
> 
> 
> <disclaimer>I don't know much about darwin/osx</disclaimer>
> 
> This assumes there's even a 32-bit version of the user's whatever $SHELL.
> Not sure we can claim that's always true?  It also doesn't feel right
> to force a different $SHELL version/build of the shell that runs if GDB
> is not involved.  Very unlikely to cause problems, but still...

Indeed, this is very unlikely, but I understand the concern.

> Why does GDB need to touch the shell's registers at all in the first place?

I haven't checked why.

> 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:
> 
> -     /* 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);
> +     /* Avoid touching the $SHELL process, and go straight to resuming it.  */
> +     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);
> 
> WDYT?

Yes, it might be cleaner.

Honestly, I'd prefer to get rid of the shell step and directly execute the user program - or at least have an option to do that.  I think I also understand the cons of this approach.

Tristan.

Re: PR13901

[Pedro Alves]

On 04/02/2012 03:06 PM, Tristan Gingold wrote:

> 
> On Apr 2, 2012, at 3:56 PM, Pedro Alves wrote:


>> Why does GDB need to touch the shell's registers at all in the first place?
> 
> I haven't checked why.


Well, I claim that it shouldn't.  :-)  The whole existence of
fork-child.c:startup_inferior was justified on making GDB not touch the
shell.  We used to have the startup phase go through the whole
wait_for_inferior shebang, which was problematic as it touched the shell.

> 
>> 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:
>>
>> -     /* 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);
>> +     /* Avoid touching the $SHELL process, and go straight to resuming it.  */
>> +     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);
>>
>> WDYT?
> 
> Yes, it might be cleaner.
> 
> Honestly, I'd prefer to get rid of the shell step and directly execute the user program - or at least have an option to do that.  I think I also understand the cons of this approach.


I'd be glad to see STARTUP_WITH_SHELL turned into a run-time option.  I think there's
a PR open for that even.  However, we need the shell at least for argument globbing,
as in, e.g., debugging `ls *', so I don't think we could make it off by default,
which practically renders it an orthogonal feature.

-- 
Pedro Alves

Re: PR13901

[Tristan Gingold]

On Apr 2, 2012, at 4:16 PM, Pedro Alves wrote:

> On 04/02/2012 03:06 PM, Tristan Gingold wrote:
> 
>> 
>> On Apr 2, 2012, at 3:56 PM, Pedro Alves wrote:
> 
> 
>>> Why does GDB need to touch the shell's registers at all in the first place?
>> 
>> I haven't checked why.
> 
> 
> Well, I claim that it shouldn't.  :-)  The whole existence of
> fork-child.c:startup_inferior was justified on making GDB not touch the
> shell.  We used to have the startup phase go through the whole
> wait_for_inferior shebang, which was problematic as it touched the shell.
> 
>> 
>>> 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:
>>> 
>>> -     /* 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);
>>> +     /* Avoid touching the $SHELL process, and go straight to resuming it.  */
>>> +     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);
>>> 
>>> WDYT?
>> 
>> Yes, it might be cleaner.
>> 
>> Honestly, I'd prefer to get rid of the shell step and directly execute the user program - or at least have an option to do that.  I think I also understand the cons of this approach.
> 
> 
> I'd be glad to see STARTUP_WITH_SHELL turned into a run-time option.

Good to know that.

>  I think there's
> a PR open for that even.  However, we need the shell at least for argument globbing,
> as in, e.g., debugging `ls *', so I don't think we could make it off by default,
> which practically renders it an orthogonal feature.

Sure.

Tristan.

Re: PR13901

[Andreas Schwab]

Pedro Alves <palves@redhat.com> writes:

> I'd be glad to see STARTUP_WITH_SHELL turned into a run-time option.  I think there's
> a PR open for that even.  However, we need the shell at least for argument globbing,
> as in, e.g., debugging `ls *', so I don't think we could make it off by default,
> which practically renders it an orthogonal feature.

It's actually redirections that are problematic case here (argument
expansions could be implemented completely independent from starting the
inferior).

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: PR13901

[Tristan Gingold]

On Apr 2, 2012, at 4:58 PM, Andreas Schwab wrote:

> Pedro Alves <palves@redhat.com> writes:
> 
>> I'd be glad to see STARTUP_WITH_SHELL turned into a run-time option.  I think there's
>> a PR open for that even.  However, we need the shell at least for argument globbing,
>> as in, e.g., debugging `ls *', so I don't think we could make it off by default,
>> which practically renders it an orthogonal feature.
> 
> It's actually redirections that are problematic case here (argument
> expansions could be implemented completely independent from starting the
> inferior).

There are a few targets (such as djgpp) that already implement redirections.

Tristan.

Re: PR13901

[Pedro Alves]

On 04/02/2012 03:58 PM, Andreas Schwab wrote:

> Pedro Alves <palves@redhat.com> writes:
> 
>> I'd be glad to see STARTUP_WITH_SHELL turned into a run-time option.  I think there's
>> a PR open for that even.  However, we need the shell at least for argument globbing,
>> as in, e.g., debugging `ls *', so I don't think we could make it off by default,
>> which practically renders it an orthogonal feature.
> 
> It's actually redirections that are problematic case here (argument
> expansions could be implemented completely independent from starting the
> inferior).


We can't make GDB fully compatible in all cases with what it can do today
if we skip the shell completely.  E.g., with "set args $(foo)", "$(foo)" is
expanded by the shell, and what that means is up to the user's shell.
That may mean "invoke the 'foo' program", as in "$(ls)" with sh.
So I'd even inclined to have !STARTUP_WITH_SHELL end up with no
argument expansion (as today).

So, if we made STARTUP_WITH_SHELL a runtime option, and even if we
defaulted it to off, we'd still need to make target code account for
when the user toggled the setting on.  So I still claim that that
would be an orthogonal feature, and that GDB should avoid touching
the SHELL process -- it should just PTRACE_CONT it, and nothing else.

-- 
Pedro Alves

Re: PR13901

[Andreas Schwab]

Pedro Alves <palves@redhat.com> writes:

> We can't make GDB fully compatible in all cases with what it can do today
> if we skip the shell completely.  E.g., with "set args $(foo)", "$(foo)" is
> expanded by the shell, and what that means is up to the user's shell.

You can handle that by calling the shell only for expanding the
arguments.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: PR13901

[Pedro Alves]

On 04/02/2012 05:22 PM, Andreas Schwab wrote:

> Pedro Alves <palves@redhat.com> writes:
> 
>> We can't make GDB fully compatible in all cases with what it can do today
>> if we skip the shell completely.  E.g., with "set args $(foo)", "$(foo)" is
>> expanded by the shell, and what that means is up to the user's shell.
> 
> You can handle that by calling the shell only for expanding the
> arguments.


True, but with that and redirection syntax being also shell
specific, I'm not sure what would be the point compared to what
we've always done.

-- 
Pedro Alves

Re: PR13901

[Andreas Schwab]

Pedro Alves <palves@redhat.com> writes:

> On 04/02/2012 05:22 PM, Andreas Schwab wrote:
>
>> Pedro Alves <palves@redhat.com> writes:
>> 
>>> We can't make GDB fully compatible in all cases with what it can do today
>>> if we skip the shell completely.  E.g., with "set args $(foo)", "$(foo)" is
>>> expanded by the shell, and what that means is up to the user's shell.
>> 
>> You can handle that by calling the shell only for expanding the
>> arguments.
>
>
> True, but with that and redirection syntax being also shell
> specific, I'm not sure what would be the point compared to what
> we've always done.

You can do the expansion in a step that is completely separate from
starting the inferior.  But you cannot handle redirections this way.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: PR13901

[John Gilmore]

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

	John Gilmore

Re: PR13901

[Pedro Alves]

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

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.

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;

Re: PR13901

[Tristan Gingold]

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

Re: PR13901

[Pedro Alves]

On 04/04/2012 03:47 PM, Tristan Gingold wrote:

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

Hmm, good idea.  Kernel's are known to forget the trace flag around signals and I'm under
the impression that darwin is one of those that can't step to signal handlers.  But
indeed if we always single-step when the flag is set, and only "optimize" on
step->!step, it does sounds like it should work.

-- 
Pedro Alves