Re: [RFC] GDB Hurd Fixes

[Thomas Schwinge <thomas at codesourcery dot com>]

Hi!

Sorry for the delay, and thanks for the patches you posted.  Here are
three patches, based on yours, that I intend to apply if there are no
further comments.


On Fri, 20 Sep 2013 11:17:08 -0400, David Michael <fedora.dm0@gmail.com> wrote:
> On Fri, Sep 20, 2013 at 4:47 AM, Pedro Alves <palves@redhat.com> wrote:
> > On 09/20/2013 01:43 AM, David Michael wrote:
> >> (Copying gdb-patches this time.)
> > But, we're missing all the context on the gdb-patches@ side.
> 
> Sorry about that--here's an explanation of the problems in GDB's build
> process with current Hurd:
> 
> First, mig has stopped using the "auto" keyword in its output.[1]
> Without that keyword, gdb/reply_mig_hack.awk fails to match a
> necessary pattern and outputs a bad gdb/process_reply_S.c file.  The
> first change I made adds a new pattern to the script in addition to
> the old one, so it should work with both old and new mig binaries.

Instead of adding a new rule, I suggest to extend the existing one as
follows:

commit d8131897afba28934ced82c507114123027a40f8
Author: Thomas Schwinge <thomas@codesourcery.com>
Date:   Mon Jan 6 15:56:33 2014 +0100

    Hurd: Adapt to changed MIG output.
    
    	gdb/
    	* reply_mig_hack.awk: Don't expect to see the auto keyword.
    
    Based on patch by David Michael <fedora.dm0@gmail.com>.

diff --git gdb/reply_mig_hack.awk gdb/reply_mig_hack.awk
index 97e080f..e137a27 100644
--- gdb/reply_mig_hack.awk
+++ gdb/reply_mig_hack.awk
@@ -78,9 +78,9 @@ parse_phase == 4 {
   print; next;
 }
 
-parse_phase == 5 && /^[ \t]*(auto|static) const mach_msg_type_t/ {
+parse_phase == 5 && /^[ \t]*(auto |static |)const mach_msg_type_t/ {
   # The type check structure for an argument.
-  arg_check_name[num_checks] = $4;
+  arg_check_name[num_checks] = $(NF - 2);
   num_checks++;
   print; next;
 }


> Next, new function definitions were added (then renamed) in
> <hurd/process_reply.defs>.[2]  In the generated
> gdb/process_reply_S.raw, some of the new functions match patterns in
> gdb/reply_mig_hack.awk in a different order than expected, producing
> bad output again.  The second change I made to the script ensures a
> necessary definition is found before writing output.  (It may be
> preferable to add a "parse_phase = 6" instead.)

I had the same thought about adding a new parsing phase, but didn't find
an easy and reliable way for detecting it; for clarity, I suggest to add
a comment, and solve this as follows:

commit ef7a4c2453a8300e93c6cc21e98ac63e0751fadd
Author: Thomas Schwinge <thomas@codesourcery.com>
Date:   Mon Jan 6 16:01:08 2014 +0100

    Hurd: Make MIG output parsing more robust.
    
    	gdb/
    	* reply_mig_hack.awk: In phase 5, keep going if we have not yet
    	collected the type check structures.
    
    Based on patch by David Michael <fedora.dm0@gmail.com>.

diff --git gdb/reply_mig_hack.awk gdb/reply_mig_hack.awk
index e137a27..b117554 100644
--- gdb/reply_mig_hack.awk
+++ gdb/reply_mig_hack.awk
@@ -92,6 +92,12 @@ parse_phase == 5 && /^[ \t]*mig_external kern_return_t/ {
 }
 
 parse_phase == 5 && /^#if[ \t]TypeCheck/ {
+  # Keep going if we have not yet collected the type check structures.
+  if (num_checks == 0)
+    {
+      print; next;
+    }
+
   # The first args type checking statement; we need to insert our chunk of
   # code that bypasses all the type checks if this is an error return, after
   # which we're done until we get to the next function.  Handily, the size


> Also because of [2], linking fails due to missing some new functions
> in gdb/process_reply_S.c.  I just extended the way other unused
> functions from process_reply.defs were handled previously in
> gdb/gnu-nat.c.

commit 8949ba1abb3100d763850b48365793f12bde0374
Author: Thomas Schwinge <thomas@codesourcery.com>
Date:   Mon Jan 6 16:28:00 2014 +0100

    Hurd: New RPC reply stub functions.
    
    	gdb/
    	* gnu-nat.c (ill_rpc): Remove function; replaced with this...
    	(ILL_RPC): ... new macro.
    	(do_mach_notify_no_senders, do_mach_notify_port_deleted)
    	(do_mach_notify_msg_accepted, do_mach_notify_port_destroyed)
    	(do_mach_notify_send_once, S_proc_setmsgport_reply)
    	(S_proc_getmsgport_reply, S_msg_sig_post_reply): Generate stub
    	functions with ILL_RPC macro.
    	(S_proc_pid2task_reply, S_proc_task2pid_reply)
    	(S_proc_task2proc_reply, S_proc_proc2task_reply)
    	(S_proc_pid2proc_reply, S_proc_getprocinfo_reply)
    	(S_proc_getprocargs_reply, S_proc_getprocenv_reply)
    	(S_proc_getloginid_reply, S_proc_getloginpids_reply)
    	(S_proc_getlogin_reply, S_proc_getsid_reply)
    	(S_proc_getsessionpgids_reply, S_proc_getsessionpids_reply)
    	(S_proc_getsidport_reply, S_proc_getpgrp_reply)
    	(S_proc_getpgrppids_reply, S_proc_get_tty_reply)
    	(S_proc_getnports_reply, S_proc_is_important_reply)
    	(S_proc_get_code_reply): New stub functions, generated with
    	ILL_RPC macro.
    
    Based on patch by David Michael <fedora.dm0@gmail.com>.

diff --git gdb/gnu-nat.c gdb/gnu-nat.c
index fa55b10..70d7f4f 100644
--- gdb/gnu-nat.c
+++ gdb/gnu-nat.c
@@ -1795,43 +1795,23 @@ do_mach_notify_dead_name (mach_port_t notify, mach_port_t dead_port)
 }
 
 
-static error_t
-ill_rpc (char *fun)
-{
-  warning (_("illegal rpc: %s"), fun);
-  return 0;
-}
-
-error_t
-do_mach_notify_no_senders (mach_port_t notify, mach_port_mscount_t count)
-{
-  return ill_rpc ("do_mach_notify_no_senders");
-}
-
-error_t
-do_mach_notify_port_deleted (mach_port_t notify, mach_port_t name)
-{
-  return ill_rpc ("do_mach_notify_port_deleted");
-}
-
-error_t
-do_mach_notify_msg_accepted (mach_port_t notify, mach_port_t name)
-{
-  return ill_rpc ("do_mach_notify_msg_accepted");
-}
-
-error_t
-do_mach_notify_port_destroyed (mach_port_t notify, mach_port_t name)
-{
-  return ill_rpc ("do_mach_notify_port_destroyed");
-}
-
-error_t
-do_mach_notify_send_once (mach_port_t notify)
-{
-  return ill_rpc ("do_mach_notify_send_once");
-}
+#define ILL_RPC(fun, ...) \
+  kern_return_t fun (__VA_ARGS__) \
+  { \
+    warning (_("illegal rpc: %s"), #fun); \
+    return 0; \
+  }
 
+ILL_RPC (do_mach_notify_no_senders,
+	 mach_port_t notify, mach_port_mscount_t count)
+ILL_RPC (do_mach_notify_port_deleted,
+	 mach_port_t notify, mach_port_t name)
+ILL_RPC (do_mach_notify_msg_accepted,
+	 mach_port_t notify, mach_port_t name)
+ILL_RPC (do_mach_notify_port_destroyed,
+	 mach_port_t notify, mach_port_t name)
+ILL_RPC (do_mach_notify_send_once,
+	 mach_port_t notify)
 
 /* Process_reply server routines.  We only use process_wait_reply.  */
 
@@ -1888,19 +1868,66 @@ S_proc_wait_reply (mach_port_t reply, error_t err,
   return 0;
 }
 
-error_t
-S_proc_setmsgport_reply (mach_port_t reply, error_t err,
-			 mach_port_t old_msg_port)
-{
-  return ill_rpc ("S_proc_setmsgport_reply");
-}
-
-error_t
-S_proc_getmsgport_reply (mach_port_t reply, error_t err, mach_port_t msg_port)
-{
-  return ill_rpc ("S_proc_getmsgport_reply");
-}
-
+ILL_RPC (S_proc_setmsgport_reply,
+	 mach_port_t reply_port, kern_return_t return_code,
+	 mach_port_t oldmsgport)
+ILL_RPC (S_proc_getmsgport_reply,
+	 mach_port_t reply_port, kern_return_t return_code,
+	 mach_port_t msgports)
+ILL_RPC (S_proc_pid2task_reply,
+	 mach_port_t reply_port, kern_return_t return_code, mach_port_t task)
+ILL_RPC (S_proc_task2pid_reply,
+	 mach_port_t reply_port, kern_return_t return_code, pid_t pid)
+ILL_RPC (S_proc_task2proc_reply,
+	 mach_port_t reply_port, kern_return_t return_code, mach_port_t proc)
+ILL_RPC (S_proc_proc2task_reply,
+	 mach_port_t reply_port, kern_return_t return_code, mach_port_t task)
+ILL_RPC (S_proc_pid2proc_reply,
+	 mach_port_t reply_port, kern_return_t return_code, mach_port_t proc)
+ILL_RPC (S_proc_getprocinfo_reply,
+	 mach_port_t reply_port, kern_return_t return_code,
+	 int flags, procinfo_t procinfo, mach_msg_type_number_t procinfoCnt,
+	 data_t threadwaits, mach_msg_type_number_t threadwaitsCnt)
+ILL_RPC (S_proc_getprocargs_reply,
+	 mach_port_t reply_port, kern_return_t return_code,
+	 data_t procargs, mach_msg_type_number_t procargsCnt)
+ILL_RPC (S_proc_getprocenv_reply,
+	 mach_port_t reply_port, kern_return_t return_code,
+	 data_t procenv, mach_msg_type_number_t procenvCnt)
+ILL_RPC (S_proc_getloginid_reply,
+	 mach_port_t reply_port, kern_return_t return_code, pid_t login_id)
+ILL_RPC (S_proc_getloginpids_reply,
+	 mach_port_t reply_port, kern_return_t return_code,
+	 pidarray_t pids, mach_msg_type_number_t pidsCnt)
+ILL_RPC (S_proc_getlogin_reply,
+	 mach_port_t reply_port, kern_return_t return_code, string_t logname)
+ILL_RPC (S_proc_getsid_reply,
+	 mach_port_t reply_port, kern_return_t return_code, pid_t sid)
+ILL_RPC (S_proc_getsessionpgids_reply,
+	 mach_port_t reply_port, kern_return_t return_code,
+	 pidarray_t pgidset, mach_msg_type_number_t pgidsetCnt)
+ILL_RPC (S_proc_getsessionpids_reply,
+	 mach_port_t reply_port, kern_return_t return_code,
+	 pidarray_t pidset, mach_msg_type_number_t pidsetCnt)
+ILL_RPC (S_proc_getsidport_reply,
+	 mach_port_t reply_port, kern_return_t return_code,
+	 mach_port_t sessport)
+ILL_RPC (S_proc_getpgrp_reply,
+	 mach_port_t reply_port, kern_return_t return_code, pid_t pgrp)
+ILL_RPC (S_proc_getpgrppids_reply,
+	 mach_port_t reply_port, kern_return_t return_code,
+	 pidarray_t pidset, mach_msg_type_number_t pidsetCnt)
+ILL_RPC (S_proc_get_tty_reply,
+	 mach_port_t reply_port, kern_return_t return_code, mach_port_t tty)
+ILL_RPC (S_proc_getnports_reply,
+	 mach_port_t reply_port, kern_return_t return_code,
+	 mach_msg_type_number_t nports)
+ILL_RPC (S_proc_is_important_reply,
+	 mach_port_t reply_port, kern_return_t return_code,
+	 boolean_t essential)
+ILL_RPC (S_proc_get_code_reply,
+	 mach_port_t reply_port, kern_return_t return_code,
+	 vm_address_t start_code, vm_address_t end_code)
 
 /* Msg_reply server routines.  We only use msg_sig_post_untraced_reply.  */
 
@@ -1934,12 +1961,8 @@ S_msg_sig_post_untraced_reply (mach_port_t reply, error_t err)
   return 0;
 }
 
-error_t
-S_msg_sig_post_reply (mach_port_t reply, error_t err)
-{
-  return ill_rpc ("S_msg_sig_post_reply");
-}
-
+ILL_RPC (S_msg_sig_post_reply,
+	 mach_port_t reply, error_t err)
 
 /* Returns the number of messages queued for the receive right PORT.  */
 static mach_port_msgcount_t


> [1] http://git.savannah.gnu.org/cgit/hurd/mig.git/commit/?id=b53836447df7230cd5665a7ccabd2a6e1a6607e5
> [2] http://git.savannah.gnu.org/cgit/hurd/hurd.git/commit/?id=e19cc6184fb99394845d56e6e915fea9805e5c28


GrÃÃe,
 Thomas

Attachment: pgp8fR6m3fnre.pgp
Description: PGP signature