[RFA] Darwin/x86 port (v4 - part 2/4: darwin-nat.?)

Stan Shebs stan@codesourcery.com
Tue Nov 18 18:48:00 GMT 2008


Tristan Gingold wrote:
>
> darwin-nat.h:
>
>
> DEF_VEC_I(thread_t);
As per my comments on machoread.c.
> struct darwin_inferior
> {
>   int pid;
>   task_t task;
I think there should be at least a one-liner describing each field.  
Even if it's obvious, the one-liner at least tells us that the meaning 
is indeed the obvious one, and not something tricky.
>   /* Previous port for requesr notification on task.  */
"request"
> extern darwin_inferior *darwin_inf;
I guess it would be mean to insist that the Darwin port be multi-process 
ready. :-)
> #if (defined __GNUC__)
> #define __MACH_CHECK_FUNCTION __PRETTY_FUNCTION__
> #else
> #define __MACH_CHECK_FUNCTION ((__const char *) 0)
> #endif
Follow what gdb_assert.h does for __PRETTY_FUNCTION__ .
> static union {
>   mach_msg_header_t hdr;
>   char data[1024];
> } msgin, msgout;
>
Definitely want to say a few words about what we have to do to work with 
Mach. Refer people to a URL or manual for full details, what we're 
interested in here is a) basic hints, and b) special issues for GDB.
>
> #define X86_EFLAGS_T 0x100UL
>
> static void
> darwin_set_sstep (thread_t thread, int enable)
> {
>   x86_thread_state_t regs;
>   unsigned int count = x86_THREAD_STATE_COUNT;
Won't the PowerPC port get indigestion from this part? :-)
>           res = PTRACE (PT_THUPDATE, pid,
>                 (void *)exc_msg.thread_port, nsignal);
Let's just call darwin_ptrace directly. I don't see much value in a 
macro, it's just in this one file.
>  #ifdef HAVE_64_BIT_MACH_EXCEPTIONS
Where does this macro come from? (Yes, I could look it up, but don't 
make me, add a comment. :-) )
>     case EXC_BREAKPOINT:
>       /* Many internal GDB routines expect breakpoints to be reported
>          as TARGET_SIGNAL_TRAP, and will report TARGET_EXC_BREAKPOINT
>          as a spurious signal. */
>       status->value.sig = TARGET_SIGNAL_TRAP;
Hmmm. OK for now, I'm sure we'll have to revisit.
>        return ptid_build (pid, 0, exc_msg.thread_port);
Thread port is the id we want to use for threads? I guess it's 
plausible, we should say that somewhere.
>
> #if 0
> static void
> darwin_list_gdb_ports (const char *msg)
If the code has a use, keep it and make a maintenance command, else 
discard. Our policy these days is to discourage if-0 blocks everywhere.
>
> /* The child must synchronize with gdb: gdb must set the exception port
>    before the child call PTRACE_SIGEXC.  We use a pipe to achieve this.
>    FIXME: is there a lighter way ?  */
Huh, interesting. I don't have any better ideas myself.
>
>   add_setshow_zinteger_cmd ("inferior", class_obscure,
>                 &inferior_debug_flag, _("\
> Set if printing inferior communication debugging statements."), _("\
> Show if printing inferior communication debugging statements."), NULL,
>                 NULL, NULL,
>                 &setdebuglist, &showdebuglist);
Wasn't this in a different file too? Make sure there is only one.
>
>   add_setshow_boolean_cmd ("mach-exceptions", class_support,
>                &enable_mach_exceptions, _("\
> Set if mach exceptions are caught."), _("\
> Show if mach exceptions are caught."), _("\
> When this mode is on, all low level exceptions are reported before 
> being\n\
> reported by the kernel."),
>                &set_enable_mach_exceptions, NULL,
>                &setlist, &showlist);
> }
>
Likewise.

I'd like to see the x86-specific bits pulled out, we should get off on 
the right foot with that sort of thing. Otherwise this file is fine, 
once feedback is incorporated.

Stan



More information about the Gdb-patches mailing list