Re: [PATCH 1/2] Port gdbserver to GNU/Hurd

[Yue Lu]

Hi,

This is the my new patch.

On Tue, Sep 3, 2013 at 7:11 PM, Pedro Alves  wrote:
> So my idea would be, instead of adding the new files under gdbserver,
> to remove the spurious differences (formatting, reordering, etc.) that
> were introduced in the gdbserver copies of the files, eliminating the
> unnecessary divergence, and then fold back the resulting differences into
> the original gdb/gnu-nat.c etc. files, guarded by #ifdef GDBSERVER.  Some
> cleanups might have been identified and done in the gdbserver files, and
> it might make sense to do those as preparatory work, in the original files.
> This should result in smaller patches, and will actually avoid
> the need for most of the polishing Thomas mentioned, and as consequence
> review burden -- reviewing the new gnu-low.c etc., for GNU conventions,
> formatting, or even appropriate use of the Hurd's debug APIs etc., is
> just unnecessary that way, by design, and we'll be able to focus on the
> bits that are the real new code -- the glue between the target and gdb, and
> the target and gdbserver.

I have put some soft link in directory gdbserver like *.defs which
point to [gdb]/gdb/*.defs.
2013-09-03  Yue Lu  
gdb
* configure.tgt: Set build_gdbserver=yes for GNU/Hurd hosts.
* configure: Regenerate.
* config/i386/i386gnu.mh: Add #ifdef to determine which rule
to use in gdbserver.
* i386gnu-nat.c: Add macor GDBSERVER to support build gdbserver.
* gnu-nat.h: Add macor GDBSERVER to support build gdbserver.
* gnu-nat.c: Add macor GDBSERVER to support build gdbserver.
(gnu_debug): New function, use for debug printf.
(gnu_wait_1): wait for inferior used in gdbserver.
(gnu_resume_1): resume inferior used in gdbserver.
(gnu_kill): New function, used in gdbserver.
(gnu_mourn): New function, clean up after inferior dies, used
in gdbserver.
(gnu_create_inferior): New function, use for create inferior
in gdbserver.
(gnu_attach): New function, a placeholder.
(gnu_detach): New function, used in gdbserver detach from inferior.
(gnu_thread_alive): New function, call find_thread_ptid().
(gnu_ptid_build): New function, build structure ptid_t from
pid, and tid.
(gnu_get_tid): New function, return lwp from structure ptid_t,
this is different with gdb's behavior.
(gnu_add_process): New function, add a new process to process list.
(gnu_join): New function, a placeholder.
(gnu_resume): New function, a wrap function, just call gnu_resume_1().
(gnu_wait): New function, a wrap function, just call gnu_wait_1().
(gnu_fetch_registers_wrap): New function, a wrap function,
just call gnu_fetch_registers().
(gnu_store_registers_wrap): New function, a wrap function,
just call gnu_store_registers().
(gnu_read_memory): New function, a wrap function, just call
gnu_read_inferior().
(gnu_write_memory): New function, a wrap function, just call
gnu_write_inferior().
(gnu_request_interrupt): New function, a placeholder.
(store_waitstatus): New function, helper function, copy from
[gdb]/gdb/inf-child.c.
(initialize_low): New function, use for initialize gdbserver's
target_ops.
(initialize_low_arch): New function, used by initialize_low().
(_initialize_gnu_nat): New function, used by initialize_low().

diff --git a/gdb/config/i386/i386gnu.mh b/gdb/config/i386/i386gnu.mh
index a3ea122..224cf0f 100644
--- a/gdb/config/i386/i386gnu.mh
+++ b/gdb/config/i386/i386gnu.mh
@@ -1,4 +1,5 @@
 # Host: Intel 386 running the GNU Hurd
+ifndef GDBSERVER
 NATDEPFILES= i386gnu-nat.o gnu-nat.o core-regset.o fork-child.o \
  notify_S.o process_reply_S.o msg_reply_S.o \
  msg_U.o exc_request_U.o exc_request_S.o
@@ -12,6 +13,10 @@ XM_CLIBS = -lshouldbeinlibc
 # Use our own user stubs for the msg rpcs, so we can make them time out, in
 # case the program is fucked, or we guess the wrong signal thread.
 msg-MIGUFLAGS = -D'MSG_IMPORTS=waittime 1000;'
+else
+NATDEPFILES= notify_S.o process_reply_S.o msg_reply_S.o \
+  exc_request_U.o exc_request_S.o
+endif

 # ick
 MIGCOM = $(MIG) -cc cat - /dev/null
diff --git a/gdb/configure.tgt b/gdb/configure.tgt
index b0bee47..3eb2ff7 100644
--- a/gdb/configure.tgt
+++ b/gdb/configure.tgt
@@ -231,6 +231,7 @@ i[34567]86-*-linux*)
 i[34567]86-*-gnu*)
  # Target: Intel 386 running the GNU Hurd
  gdb_target_obs="i386-tdep.o i387-tdep.o i386gnu-tdep.o solib-svr4.o"
+ build_gdbserver=yes
  ;;
 i[34567]86-*-cygwin*)
  # Target: Intel 386 running win32
diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
index 59d2f23..1cdcd1d 100644
--- a/gdb/gnu-nat.c
+++ b/gdb/gnu-nat.c
@@ -1,6 +1,7 @@
 /* Interface GDB to the GNU Hurd.
Copyright (C) 1992-2013 Free Software Foundation, Inc.

+
This file is part of GDB.

Written by Miles Bader 
@@ -20,6 +21,7 @@
You should have received a copy of the GNU General Public License
along with this pro

Re: [PATCH 03/17] Add proc_set_init_task, make runsystem pid 1

[Justus Winter]

Quoting Justus Winter (2013-08-29 12:09:02)
> Quoting Samuel Thibault (2013-08-29 01:48:10)
> > I think we should keep this in the Debian package, the upstream GNU
> > system will a priori still use /hurd/init as reaper.
> 
> Well, it is not my place to question this and maintaining the patches
> is your burden, but I was hoping to remove the process reaping code
> from /hurd/init.

I just spent about five hours debugging why my patched /hurd/proc
crashes early on in the boot process (and yes, I also used subhurds
and tried to hook gdb into it, it didn't produce any useful results,
not quite sure why). Turns out it was only because my /hurd/init
contained the proc_set_init_task patches while the /hurd/proc I built
on top of hurd upstream did not :/ and this was even though I wrote
those patches.

So I'd like to restate these questions:

1. Who is "upstream GNU"?

2. If I shall continue working on the Hurd, how suspectible is
   "upstream GNU" to my patches?

I ask this because I see the difference between "Hurd with Debian
patches" and "upstream Hurd" as a burden not only for the Debian
maintainers but also for any potential developer.

Also, according to [0] Debian/Hurd is the only "working" Hurd
distribution (whatever that means, let's say not in "early stages of
development" and not "defunct" as used on that page). In particular,
the "GNU" distribution is marked as "defunct" and according to [1]
there has not been a release for seven years. So from my point of view
they are *not* using /hurd/init as reaper, so they might as well *not*
use sysvinit (or any other established init system) as reaper.

0: http://www.gnu.org/software/hurd/hurd/running/distrib.html
1: http://www.gnu.org/software/hurd/hurd/running/gnu.html

Justus

[PATCH 2/2] kern: make the parent of a task available via task_info

[Justus Winter]

XXX: this most likely subverts the security of your system. DO NOT
APPLY.

* include/mach/task_info.h (struct task_basic_info): Add parent_task.
* kern/task.c (task_info): Include a port to parent_task.
---
 include/mach/task_info.h |2 ++
 kern/task.c  |3 +++
 2 files changed, 5 insertions(+)

diff --git a/include/mach/task_info.h b/include/mach/task_info.h
index 5607178..dc520a9 100644
--- a/include/mach/task_info.h
+++ b/include/mach/task_info.h
@@ -37,6 +37,7 @@
 
 #include 
 #include 
+#include 
 
 /*
  * Generic information structure to allow for expansion.
@@ -61,6 +62,7 @@ struct task_basic_info {
time_value_tsystem_time;/* total system run time for
   terminated threads */
time_value_tcreation_time;  /* creation time stamp */
+   mach_port_t parent_task;/* parent of the task */
 };
 
 typedef struct task_basic_info task_basic_info_data_t;
diff --git a/kern/task.c b/kern/task.c
index 0c0be99..4d98132 100644
--- a/kern/task.c
+++ b/kern/task.c
@@ -750,6 +750,9 @@ kern_return_t task_info(
basic_info->system_time.microseconds
= task->total_system_time.microseconds;
basic_info->creation_time = task->creation_time;
+   task_reference(task->parent_task);
+   basic_info->parent_task
+   = convert_task_to_port(task->parent_task);
task_unlock(task);
 
if (*task_info_count > TASK_BASIC_INFO_COUNT)
-- 
1.7.10.4

[PATCH 1/2] kern: track the parent of a task

[Justus Winter]

Align Machs model of a task more closely with that of a Unix process
by tracking the parent task.

XXX: Not sure if it's worth keeping a real reference to the parent
since that way we prevent the parent task from being collected if it
dies before the child does.

* kern/task.h (struct task): Add field parent_task.
* kern/task.c (task_create): Keep a reference to the parent task.
(task_deallocate): Dereference the parent task.
---
 kern/task.c |3 +++
 kern/task.h |1 +
 2 files changed, 4 insertions(+)

diff --git a/kern/task.c b/kern/task.c
index 114dd31..0c0be99 100644
--- a/kern/task.c
+++ b/kern/task.c
@@ -105,6 +105,8 @@ kern_return_t task_create(
simple_lock_init(&new_task->lock);
queue_init(&new_task->thread_list);
new_task->suspend_count = 0;
+   task_reference(parent_task);
+   new_task->parent_task = parent_task;
new_task->active = TRUE;
new_task->user_stop_count = 0;
new_task->thread_count = 0;
@@ -206,6 +208,7 @@ void task_deallocate(
pset_deallocate(pset);
vm_map_deallocate(task->map);
is_release(task->itk_space);
+   task_deallocate(task->parent_task);
kmem_cache_free(&task_cache, (vm_offset_t) task);
 }
 
diff --git a/kern/task.h b/kern/task.h
index 9bfea57..c8d9f3d 100644
--- a/kern/task.h
+++ b/kern/task.h
@@ -58,6 +58,7 @@ struct task {
vm_map_tmap;/* Address space description */
queue_chain_t   pset_tasks; /* list of tasks assigned to pset */
int suspend_count;  /* Internal scheduling only */
+   task_t  parent_task;/* Parent of the task */
 
/* Thread information */
queue_head_tthread_list;/* list of threads */
-- 
1.7.10.4

Hacking gnumach to track parental relationship of tasks

[Justus Winter]

Hi,

I made two rather small and (as I thought) straight forward changes to
gnumach to keep track of a tasks father task and to make this
information available. Now I know handing out task ports right and
left is not a wise thing to do, but I wanted to do it this way as a
first step.

Unfortunately it does not work as I expected it to work. Here is a
little test program to get the information:

~~~ snip ~~~
#define _GNU_SOURCE
#include 
#include 
#include 
#include 
#include 
#include 

struct my_task_basic_info {
integer_t   suspend_count;  /* suspend count for task */
integer_t   base_priority;  /* base scheduling priority */
vm_size_t   virtual_size;   /* number of virtual pages */
vm_size_t   resident_size;  /* number of resident pages */
time_value_tuser_time;  /* total user run time for
   terminated threads */
time_value_tsystem_time;/* total system run time for
   terminated threads */
time_value_tcreation_time;  /* creation time stamp */
mach_port_t parent_task;/* parent of the task */
};

int
main ()
{
  error_t err;

  task_t t;
  err = proc_pid2task (getproc (), getpid (), &t);
  if (err)
error (1, err, "proc_pid2task");

  task_t p;
  err = proc_pid2task (getproc (), getppid (), &p);
  if (err)
error (1, err, "proc_pid2task");

  struct my_task_basic_info info = {};
  mach_msg_type_number_t info_count;
  err = task_info (t, TASK_BASIC_INFO, &info, &info_count);
  if (err)
error (1, err, "task_info");

  printf ("my task port is %i(pid %i) created by task %i; my parent task is 
%i(pid %i)\n",
  t, getpid (), info.parent_task, p, getppid ());

  return p == info.parent_task? EXIT_SUCCESS: EXIT_FAILURE;
}
~~~ snap ~~~

On an unpatched gnumach kernel it prints

  my task port is 1(pid 18) created by task 0; my parent task is 31(pid 6)

on a patched kernel it says something like

  my task port is 1(pid 13) created by task -527895648; my parent task is 
31(pid 1)

which (ignoring the signedness) looks a lot more like a pointer than
like a port. Richard suggested to hardcode a value to test whether the
value is actually handed across, and that works (i.e. a 3 came across
just fine).

Any thoughts?

To dig deeper into this I compiled in the kdb, but unfortunately it
does not translate from symbols to addresses and vice versa, that
makes debugging using it rather difficult. Is this a known problem or
am I holding it wrong?

I also tried to attach gdb to qemus gdb stub, and it has the same
issue, but at least I can set breakpoints using symbolic names.

The binary was compiled using -ggdb and was not stripped.

A little help?

Thanks,
Justus

Re: Hacking gnumach to track parental relationship of tasks

[Ludovic Courtès]

Justus Winter <4win...@informatik.uni-hamburg.de> skribis:

> I made two rather small and (as I thought) straight forward changes to
> gnumach to keep track of a tasks father task and to make this
> information available.

Isn’t that what ‘proc_getpids’ is for?

It feels wrong to retrofit POSIX concepts in Mach.

Ludo’.

[PATCH 5/7] hurd: add notification callbacks for the process management

[Justus Winter]

XXX

* hurd/process_notify.defs: New file.
---
 hurd/process_notify.defs |   21 +
 1 file changed, 21 insertions(+)
 create mode 100644 hurd/process_notify.defs

diff --git a/hurd/process_notify.defs b/hurd/process_notify.defs
new file mode 100644
index 000..7972ef7
--- /dev/null
+++ b/hurd/process_notify.defs
@@ -0,0 +1,21 @@
+/* XXX */
+
+subsystem process_notify 24500;
+
+#include 
+
+#ifdef PROCESS_NOTIFY_IMPORTS
+PROCESS_NOTIFY_IMPORTS
+#endif
+
+/* For maximum robustness, the server must not wait for the client to
+   receive the notification message.  This is achieved by setting a
+   send timeout (XXX which is implicitely 0 with MACH_MSG_TIMEOUT_NONE).  */
+MsgOption MACH_SEND_TIMEOUT;
+
+/* XXX: Notify that a process has been created or died.  */
+simpleroutine notify_process_changed (
+   notify_port: mach_port_t;
+   pid: pid_t;
+   ppid: pid_t;
+   dead: boolean_t);
-- 
1.7.10.4

Re: [PATCH 1/2] Port gdbserver to GNU/Hurd

[Pedro Alves]

On 09/05/2013 11:53 AM, Yue Lu wrote:
> Hi,
> 
> This is the my new patch.

Thanks.  Follows a few comments, by no means an in depth review.
We'll probably need to iterate a few times.  I'm counting on
Thomas and others to help as well!

I'm actually very excited to see gdb and gdbserver sharing
a single target backend, even if we still have many wrinkles
in the interfaces to iron out!  It does looks like this way
results in lots of less work, and makes me believe we can
port other targets to gdbserver already without unsurmountable
effort.

It'd be very useful if you send and maintain along
with the patch the rationale for all design divergences between
the ports you found necessary.  E.g., it seems that gdbserver's
task/proc bookkeeping is different from gdb's.  Why?

> I have put some soft link in directory gdbserver like *.defs which
> point to [gdb]/gdb/*.defs.

We'll need to adjust the Makefile etc. instead to use the original
files instead of that.  No symlinks please.

> 2013-09-03  Yue Lu  
> gdb
> * configure.tgt: Set build_gdbserver=yes for GNU/Hurd hosts.
> * configure: Regenerate.
> * config/i386/i386gnu.mh: Add #ifdef to determine which rule
> to use in gdbserver.
> * i386gnu-nat.c: Add macor GDBSERVER to support build gdbserver.
> * gnu-nat.h: Add macor GDBSERVER to support build gdbserver.
> * gnu-nat.c: Add macor GDBSERVER to support build gdbserver.
> (gnu_debug): New function, use for debug printf.
> (gnu_wait_1): wait for inferior used in gdbserver.
> (gnu_resume_1): resume inferior used in gdbserver.
> (gnu_kill): New function, used in gdbserver.
> (gnu_mourn): New function, clean up after inferior dies, used
> in gdbserver.
> (gnu_create_inferior): New function, use for create inferior
> in gdbserver.
> (gnu_attach): New function, a placeholder.
> (gnu_detach): New function, used in gdbserver detach from inferior.
> (gnu_thread_alive): New function, call find_thread_ptid().
> (gnu_ptid_build): New function, build structure ptid_t from
> pid, and tid.
> (gnu_get_tid): New function, return lwp from structure ptid_t,
> this is different with gdb's behavior.
> (gnu_add_process): New function, add a new process to process list.
> (gnu_join): New function, a placeholder.
> (gnu_resume): New function, a wrap function, just call gnu_resume_1().
> (gnu_wait): New function, a wrap function, just call gnu_wait_1().
> (gnu_fetch_registers_wrap): New function, a wrap function,
> just call gnu_fetch_registers().
> (gnu_store_registers_wrap): New function, a wrap function,
> just call gnu_store_registers().
> (gnu_read_memory): New function, a wrap function, just call
> gnu_read_inferior().
> (gnu_write_memory): New function, a wrap function, just call
> gnu_write_inferior().
> (gnu_request_interrupt): New function, a placeholder.
> (store_waitstatus): New function, helper function, copy from
> [gdb]/gdb/inf-child.c.
> (initialize_low): New function, use for initialize gdbserver's
> target_ops.
> (initialize_low_arch): New function, used by initialize_low().
> (_initialize_gnu_nat): New function, used by initialize_low().
> 
> diff --git a/gdb/config/i386/i386gnu.mh b/gdb/config/i386/i386gnu.mh
> index a3ea122..224cf0f 100644
> --- a/gdb/config/i386/i386gnu.mh
> +++ b/gdb/config/i386/i386gnu.mh
> @@ -1,4 +1,5 @@
>  # Host: Intel 386 running the GNU Hurd
> +ifndef GDBSERVER
>  NATDEPFILES= i386gnu-nat.o gnu-nat.o core-regset.o fork-child.o \
>   notify_S.o process_reply_S.o msg_reply_S.o \
>   msg_U.o exc_request_U.o exc_request_S.o
> @@ -12,6 +13,10 @@ XM_CLIBS = -lshouldbeinlibc
>  # Use our own user stubs for the msg rpcs, so we can make them time out, in
>  # case the program is fucked, or we guess the wrong signal thread.
>  msg-MIGUFLAGS = -D'MSG_IMPORTS=waittime 1000;'
> +else
> +NATDEPFILES= notify_S.o process_reply_S.o msg_reply_S.o \
> +  exc_request_U.o exc_request_S.o
> +endif
> 
>  # ick
>  MIGCOM = $(MIG) -cc cat - /dev/null
> diff --git a/gdb/configure.tgt b/gdb/configure.tgt
> index b0bee47..3eb2ff7 100644
> --- a/gdb/configure.tgt
> +++ b/gdb/configure.tgt
> @@ -231,6 +231,7 @@ i[34567]86-*-linux*)
>  i[34567]86-*-gnu*)
>   # Target: Intel 386 running the GNU Hurd
>   gdb_target_obs="i386-tdep.o i387-tdep.o i386gnu-tdep.o solib-svr4.o"
> + build_gdbserver=yes
>   ;;
>  i[34567]86-*-cygwin*)
>   # Target: Intel 386 running win32
> diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
> index 59d2f23..1cdcd1d 100644
> --- a/gdb/gnu-nat.c
> +++ b/gdb/gnu-nat.c
> @@ -1,6 +1,7 @@
>  /* Interface GDB to the GNU Hurd.
> Copyright (C) 1992-2013 Free Software Foundation, Inc.
> 
> +
> This file is part of GDB.

Please go through the patch and remove spurious whitespace
changes line these.

> 
> Written by Miles Bader 
> @@ -20,6 +21,7 @@
> 

[PATCH 7/7] hurd: add proc_notify_new_processes

[Justus Winter]

XXX

* hurd/process.defs: Add proc_notify_new_processes.
---
 hurd/process.defs |5 +
 1 file changed, 5 insertions(+)

diff --git a/hurd/process.defs b/hurd/process.defs
index b7e1775..37add48 100644
--- a/hurd/process.defs
+++ b/hurd/process.defs
@@ -383,3 +383,8 @@ routine proc_mark_important (
 routine proc_is_important (
process: process_t;
out essential: boolean_t);
+
+/* XXX: Request notification of new processes on NOTIFY.  */
+routine proc_request_process_change_notification (
+   process: process_t;
+   notify: mach_port_send_t);
-- 
1.7.10.4

[PATCH 2/7] proc: Remove unused declaration of zombie_list

[Justus Winter]

* proc/proc.h (zombie_list): Remove declaration.
---
 proc/proc.h |2 --
 1 file changed, 2 deletions(-)

diff --git a/proc/proc.h b/proc/proc.h
index ed47ccc..4369abf 100644
--- a/proc/proc.h
+++ b/proc/proc.h
@@ -130,8 +130,6 @@ struct exc
   natural_t thread_state[0];
 };
 
-struct zombie *zombie_list;
-
 mach_port_t authserver;
 struct proc *self_proc;/* process 0 (us) */
 struct proc *startup_proc; /* process 1 (init) */
-- 
1.7.10.4

[PATCH 3/7] libihash: add HURD_IHASH_ITERATE_ITEMS macro

[Justus Winter]

Add a macro HURD_IHASH_ITERATE_ITEMS that iterates over all elements
in the hash table making both the key and the value available.

* libihash/ihash.h (HURD_IHASH_ITERATE_ITEMS): New macro.
---
 libihash/ihash.h |   17 +
 1 file changed, 17 insertions(+)

diff --git a/libihash/ihash.h b/libihash/ihash.h
index a4e76dc..3ca5ec3 100644
--- a/libihash/ihash.h
+++ b/libihash/ihash.h
@@ -218,6 +218,23 @@ hurd_ihash_value_t hurd_ihash_find (hurd_ihash_t ht, 
hurd_ihash_key_t key);
 (((_hurd_ihash_item_t) _hurd_ihash_valuep) + 1))   \
 if (val != _HURD_IHASH_EMPTY && val != _HURD_IHASH_DELETED)
 
+/* Iterate over all elements in the hash table making both the key and
+   the value available.  You use this macro with a block, for example
+   like this:
+
+ HURD_IHASH_ITERATE_ITEMS (ht, item)
+   foo (item->key, item->value);
+
+   The block will be run for every element in the hash table HT.  The
+   key and value of the current element is available as ITEM->key and
+   ITEM->value.  */
+#define HURD_IHASH_ITERATE_ITEMS(ht, item)  \
+  for (_hurd_ihash_item_t item = (ht)->size? &(ht)->items[0]: 0;   \
+   (ht)->size && item - &(ht)->items[0] < (ht)->size;   \
+   item++)  \
+if (item->value != _HURD_IHASH_EMPTY && \
+item->value != _HURD_IHASH_DELETED)
+
 /* Remove the entry with the key KEY from the hash table HT.  If such
an entry was found and removed, 1 is returned, otherwise 0.  */
 int hurd_ihash_remove (hurd_ihash_t ht, hurd_ihash_key_t key);
-- 
1.7.10.4

[PATCH 1/7] proc: remove unused file exc-reply.defs

[Justus Winter]

* proc/exc-reply.defs: Delete file.
---
 proc/exc-reply.defs |   36 
 1 file changed, 36 deletions(-)
 delete mode 100644 proc/exc-reply.defs

diff --git a/proc/exc-reply.defs b/proc/exc-reply.defs
deleted file mode 100644
index 8a04723..000
--- a/proc/exc-reply.defs
+++ /dev/null
@@ -1,36 +0,0 @@
-/* This special version of exc.defs exists because the proc server
-   has to be able to do both halves of the exc interface separately. 
-   Its interface is identical to . */
-
-/* 
- * Mach Operating System
- * Copyright (c) 1991,1990,1989,1988,1987 Carnegie Mellon University
- * All Rights Reserved.
- * 
- * Permission to use, copy, modify and distribute this software and its
- * documentation is hereby granted, provided that both the copyright
- * notice and this permission notice appear in all copies of the
- * software, derivative works or modified versions, and any portions
- * thereof, and that both notices appear in supporting documentation.
- * 
- * CARNEGIE MELLON ALLOWS FREE USE OF THIS SOFTWARE IN ITS "AS IS"
- * CONDITION.  CARNEGIE MELLON DISCLAIMS ANY LIABILITY OF ANY KIND FOR
- * ANY DAMAGES WHATSOEVER RESULTING FROM THE USE OF THIS SOFTWARE.
- * 
- * Carnegie Mellon requests users of this software to return to
- * 
- *  Software Distribution Coordinator  or  software.distribut...@cs.cmu.edu
- *  School of Computer Science
- *  Carnegie Mellon University
- *  Pittsburgh PA 15213-3890
- * 
- * any improvements or extensions that they make and grant Carnegie Mellon
- * the rights to redistribute these changes.
- */
-
-subsystem exc 2400;
-
-#include 
-
-simpleroutine exception_raise (exception_port: mach_port_t;
-   
\ No newline at end of file
-- 
1.7.10.4

Re: [PATCH 1/2] Port gdbserver to GNU/Hurd

[Thomas Schwinge]

Hi!

Just a very quick one; short on time.

On Thu, 05 Sep 2013 20:29:43 +0100, Pedro Alves  wrote:
> On 09/05/2013 11:53 AM, Yue Lu wrote:
> > This is the my new patch.

You've received quite some positive feedback, good!  :-)

> Thanks.  Follows a few comments, by no means an in depth review.

Thanks for those reviews -- helps a lot.

> We'll probably need to iterate a few times.  I'm counting on
> Thomas and others to help as well!

Yes, but unfortunately not immediatelly.  Anyway, the more the code
converges towards using the present gnu-nat.c code on the one hand (which
I'll try to help with) as well as the present gdbserver code on the other
hand (which I don't have experience with, but Pedro has already pointed
out several items), the easier it will be to review.  And, we've made
quite a step forward with this revision of the patch, as far as I can
tell!

> It'd be very useful if you send and maintain along
> with the patch the rationale for all design divergences between
> the ports you found necessary.  E.g., it seems that gdbserver's
> task/proc bookkeeping is different from gdb's.  Why?

Yes, it'd be useful to have some source code comments next to (some of)
the »#ifdef GDBSERVER« (as well as elsewhere, of course).

> > --- a/gdb/config/i386/i386gnu.mh
> > +++ b/gdb/config/i386/i386gnu.mh
> > @@ -1,4 +1,5 @@
> >  # Host: Intel 386 running the GNU Hurd
> > +ifndef GDBSERVER
> >  NATDEPFILES= i386gnu-nat.o gnu-nat.o core-regset.o fork-child.o \
> >   notify_S.o process_reply_S.o msg_reply_S.o \
> >   msg_U.o exc_request_U.o exc_request_S.o
> > @@ -12,6 +13,10 @@ XM_CLIBS = -lshouldbeinlibc
> >  # Use our own user stubs for the msg rpcs, so we can make them time out, in
> >  # case the program is fucked, or we guess the wrong signal thread.
> >  msg-MIGUFLAGS = -D'MSG_IMPORTS=waittime 1000;'
> > +else
> > +NATDEPFILES= notify_S.o process_reply_S.o msg_reply_S.o \
> > +  exc_request_U.o exc_request_S.o
> > +endif

Pedro, is the idea that this file should also be shared with gdbserver?
Well, probably yes (for the moment), and the idea really is to move the
shared bits into common/, as with the other shared code.  So, I guess
this is fine for the moment.

> > --- a/gdb/gnu-nat.c
> > +++ b/gdb/gnu-nat.c
> > @@ -1,6 +1,7 @@
> >  /* Interface GDB to the GNU Hurd.
> > Copyright (C) 1992-2013 Free Software Foundation, Inc.
> > 
> > +
> > This file is part of GDB.
> 
> Please go through the patch and remove spurious whitespace
> changes line these.

Yes, this is generally good advise -- I (try to...) re-read every patch
I'm about to send by email or directly commit, and avoid any such
"spurious" changes.  GDB (as other GNU projects, too) tend to set a high
standard on code quality, which is good, and this also includes such
discipline when doing changes to the code.

> > +#ifdef GDBSERVER
> > +/* this should move into gnu-i386-low.c ?*/
> 
> I guess that means i386gnu-nat.c now, but it sounds like
> it, yes.
> 
> > +/* Defined in auto-generated file i386.c.  */
> > +extern void init_registers_i386 (void);
> > +extern const struct target_desc *tdesc_i386;

Yes, something with a i386 tag in it can't be in the generic gnu-nat.c
file.  (Not that there'd be any other GNU Hurd ports available, but you
get the idea.)

> (correct me if
> I'm wrong here), the Hurd's threads are kernel threads

Correct.

> so it'd
> be better to just make the GDB side use the lwp field too.
> It's really a simple and mechanic change.  Nothing in GDB core
> actually cares which field is used.  So in this case, it'd be
> better if you send a preparatory patch

Based on the current upstream master branch.

> that does that change
> (and nothing else)

Confirm that you haven't caused any regressions by running the GDB
testsuite (natively) without and then with your change (don't forget to
apply the testsuite patch I gave you earlier, to avoid the testsuite
hanging (known Hurd issue)), and diff the *.sum files to see that there
are no (major) differences (post them with your patch).

> and then rebase the port on top of that
> patch.

Please tell if you need help with how to use Git to rebase your current
patch onto the upstream master branch.  Also, did you figure out your
earlier question about how to merge several Git commits into one?

> >  /* Attach to process PID, then initialize for debugging it
> > and wait for the trace-trap that results from attaching.  */
> > +#ifndef GDBSERVER
> >  static void
> >  gnu_attach (struct target_ops *ops, char *args, int from_tty)
> >  {
> > @@ -2207,8 +2347,14 @@ gnu_attach (struct target_ops *ops, char *args,
> > int from_tty)
> >renumber_threads (0); /* Give our threads reasonable names.  */
> >  #endif
> >  }
> > +#else
> > +static int
> > +gnu_attach (unsigned long pid)
> > +{
> > +  return -1; //not support now
> 
> Doesn't look like this should be hard to get going.

With the native GDB port, there is a known issue when attaching to a
running processes (specifical

Re: [PATCH 1/2] Port gdbserver to GNU/Hurd

[Joel Brobecker]

> I'm actually very excited to see gdb and gdbserver sharing
> a single target backend, even if we still have many wrinkles
> in the interfaces to iron out!

Mee too!

-- 
Joel

process change notification prototype

[Justus Winter]

Hi :)

this is an early patch series implementing process notifications in
the proc server.

[PATCH 1/7] proc: remove unused file exc-reply.defs
[PATCH 2/7] proc: Remove unused declaration of zombie_list

Cleanups that can be merged independently of the rest.

[PATCH 3/7] libihash: add HURD_IHASH_ITERATE_ITEMS macro

I'd also like to propose this as an alternative to the
HURD_IHASH_ITERATE macro that only exposes the value, not the
key. This one gives one both the key and the value. The implementation
is also a lot less scary.

[PATCH 4/7] libnotify: add a general notification library

This is my shot at a hurdish general purpose notification
library. Please comment.

This could also be used in /hurd/init for the shutdown
notifications, and Richard suggested that we could have another class
of shutdown notifications available for non-privileged users that may
not stall the shutdown process (otoh I'm not so sure anymore that the
current mechanism can stall it, but I think this happened to me
once).

The function hurd_notify_do_notify_wait is not tested, the idea is to
asynchronously notify all receivers and then collect the responses.

[PATCH 5/7] hurd: add notification callbacks for the process
[PATCH 6/7] proc: implement proc_notify_new_processes
[PATCH 7/7] hurd: add proc_notify_new_processes

With these in place my cgroupfs can track all processes that play by
the book, i.e. that do use proc_child to mark a new process as their
child.

Cheers,
Justus

[PATCH 6/7] proc: implement proc_notify_new_processes

[Justus Winter]

XXX

XXX
---
 proc/Makefile |5 +++--
 proc/main.c   |1 +
 proc/mgt.c|   37 +
 proc/proc.h   |6 ++
 4 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/proc/Makefile b/proc/Makefile
index 2eed13c..4f3be81 100644
--- a/proc/Makefile
+++ b/proc/Makefile
@@ -29,9 +29,10 @@ MIGSFLAGS="-DPROCESS_INTRAN=pstruct_t reqport_find 
(process_t)" \
"-DPROCESS_IMPORTS=import \"proc.h\";"
 
 MIGSTUBS = processServer.o notifyServer.o \
-   ourmsgUser.o proc_excUser.o proc_excServer.o
+   ourmsgUser.o proc_excUser.o proc_excServer.o \
+   process_notifyUser.o
 OBJS = $(SRCS:.c=.o) $(MIGSTUBS)
-HURDLIBS = ihash ports shouldbeinlibc
+HURDLIBS = ihash ports shouldbeinlibc notify
 OTHERLIBS = -lpthread
 
 include ../Makeconf
diff --git a/proc/main.c b/proc/main.c
index 73abbc0..481c813 100644
--- a/proc/main.c
+++ b/proc/main.c
@@ -80,6 +80,7 @@ main (int argc, char **argv, char **envp)
   ports_create_port (generic_port_class, proc_bucket,
 sizeof (struct port_info), &genport);
   generic_port = ports_get_right (genport);
+  hurd_notify_create (¬ify_on_new_processes, do_notify_process_changed);
 
   /* Create the initial proc object for init (PID 1).  */
   startup_proc = create_startup_proc ();
diff --git a/proc/mgt.c b/proc/mgt.c
index 6a10e91..f1e924a 100644
--- a/proc/mgt.c
+++ b/proc/mgt.c
@@ -210,6 +210,11 @@ S_proc_child (struct proc *parentp,
childp->p_parent->p_pid, childp->p_pgrp->pg_pgid,
!childp->p_pgrp->pg_orphcnt);
   childp->p_parentset = 1;
+
+  /* XXX do the notification here in the mean time.  */
+  /* Notify any process change listeners.  */
+  hurd_notify_do_notify (notify_on_new_processes, (void *) childp);
+
   return 0;
 }
 
@@ -688,6 +693,10 @@ new_proc (task_t task)
   p = allocate_proc (task);
   if (p)
 complete_proc (p, genpid ());
+
+  /* XXX: once the kernel provides the parent task, do the
+ notification here */
+
   return p;
 }
 
@@ -765,6 +774,9 @@ process_has_exited (struct proc *p)
 
   /* Cancel any outstanding RPCs done on behalf of the dying process.  */
   ports_interrupt_rpcs (p);
+
+  /* Notify any process change listeners.  */
+  hurd_notify_do_notify (notify_on_new_processes, (void *) p);
 }
 
 void
@@ -903,3 +915,28 @@ S_proc_is_important (struct proc *callerp,
 
   return 0;
 }
+
+/* XXX */
+error_t
+S_proc_request_process_change_notification (struct proc *callerp,
+mach_port_t notify)
+{
+  if (!callerp)
+return EOPNOTSUPP;
+
+  return hurd_notify_add (notify_on_new_processes, notify, (void *) 1);
+}
+
+error_t
+do_notify_process_changed (mach_port_t port,
+   mach_port_t reply_port,
+   void *data,
+   void *common_data)
+{
+  struct proc *p = common_data;
+  return notify_process_changed (port,
+ p->p_pid,
+ p->p_parent->p_pid,
+ p->p_dead);
+}
+
diff --git a/proc/proc.h b/proc/proc.h
index 4369abf..dc46ed7 100644
--- a/proc/proc.h
+++ b/proc/proc.h
@@ -26,6 +26,7 @@ the Free Software Foundation, 675 Mass Ave, Cambridge, MA 
02139, USA.  */
 #include 
 #include 
 #include 
+#include 
 #include 
 
 struct proc
@@ -138,6 +139,7 @@ struct port_bucket *proc_bucket;
 struct port_class *proc_class;
 struct port_class *generic_port_class;
 struct port_class *exc_class;
+struct hurd_notify *notify_on_new_processes;
 
 mach_port_t master_host_port;
 mach_port_t master_device_port;
@@ -211,5 +213,9 @@ void initialize_version_info (void);
 
 void send_signal (mach_port_t, int, mach_port_t);
 
+error_t do_notify_process_changed (mach_port_t port,
+   mach_port_t reply_port,
+   void *data,
+   void *common_data);
 
 #endif
-- 
1.7.10.4

[PATCH 4/7] libnotify: add a general notification library

[Justus Winter]

XXX
---
 libnotify/Makefile |   28 ++
 libnotify/notify.c |  152 
 libnotify/notify.h |   50 +
 3 files changed, 230 insertions(+)
 create mode 100644 libnotify/Makefile
 create mode 100644 libnotify/notify.c
 create mode 100644 libnotify/notify.h

diff --git a/libnotify/Makefile b/libnotify/Makefile
new file mode 100644
index 000..64d55b5
--- /dev/null
+++ b/libnotify/Makefile
@@ -0,0 +1,28 @@
+# Copyright (C) 2013 Free Software Foundation, Inc.
+#
+# This file is part of the GNU Hurd.
+#
+# The GNU Hurd is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2, or (at
+# your option) any later version.
+#
+# The GNU Hurd is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with the GNU Hurd.  If not, see .
+
+dir := libnotify
+makemode := library
+
+libname := libnotify
+SRCS = notify.c
+installhdrs = notify.h
+
+OBJS = $(SRCS:.c=.o)
+HURDLIBS = ihash
+
+include ../Makeconf
diff --git a/libnotify/notify.c b/libnotify/notify.c
new file mode 100644
index 000..9419e3c
--- /dev/null
+++ b/libnotify/notify.c
@@ -0,0 +1,152 @@
+/* XXX */
+
+#include 
+#include 
+#include 
+
+/* XXX */
+void
+hurd_notify_init (hurd_notify_t n, hurd_notify_func_t notify_func)
+{
+  hurd_ihash_init (&n->receivers, HURD_IHASH_NO_LOCP);
+  n->notify_func = notify_func;
+}
+
+/* XXX */
+void
+hurd_notify_destroy (hurd_notify_t n)
+{
+  hurd_ihash_destroy (&n->receivers);
+}
+
+/* XXX */
+error_t
+hurd_notify_create (hurd_notify_t *n, hurd_notify_func_t notify_func)
+{
+  *n = malloc (sizeof (struct hurd_notify));
+  if (*n == NULL)
+return ENOMEM;
+
+  hurd_notify_init (*n, notify_func);
+
+  return 0;
+}
+
+/* XXX */
+void
+hurd_notify_free (hurd_notify_t n)
+{
+  hurd_notify_destroy (n);
+  free (n);
+}
+
+/* XXX */
+error_t
+hurd_notify_add (hurd_notify_t n, mach_port_t notify, void *data)
+{
+  return hurd_ihash_add (&n->receivers, notify, data);
+}
+
+/* XXX */
+int
+hurd_notify_remove (hurd_notify_t n, mach_port_t notify)
+{
+  return hurd_ihash_remove (&n->receivers, notify);
+}
+
+
+/* XXX */
+error_t
+hurd_notify_do_notify (hurd_notify_t n, void *common_data)
+{
+  error_t err;
+  HURD_IHASH_ITERATE_ITEMS (&n->receivers, item)
+{
+  err = n->notify_func (item->key,
+MACH_PORT_NULL,
+item->value,
+common_data);
+  if (err == MACH_SEND_INVALID_DEST)
+hurd_notify_remove (n, item->key);
+}
+
+  return 0;
+}
+
+/* XXX */
+error_t
+hurd_notify_do_notify_wait (hurd_notify_t n,
+void *common_data,
+natural_t timeout)
+{
+  error_t err;
+  mach_port_t port_set;
+  err = mach_port_allocate (mach_task_self (),
+MACH_PORT_RIGHT_PORT_SET,
+&port_set);
+  if (err)
+return err;
+
+  int count = 0;
+  HURD_IHASH_ITERATE_ITEMS (&n->receivers, item)
+{
+  mach_port_t reply_port = mach_reply_port ();
+  if (reply_port == MACH_PORT_NULL)
+return ENOMEM; /* XXX */
+
+  err = mach_port_move_member (mach_task_self (),
+   reply_port,
+   port_set);
+  if (err)
+return err; /* XXX */
+
+  err = n->notify_func (item->key,
+reply_port,
+item->value,
+common_data);
+  if (err == MACH_SEND_INVALID_DEST)
+{
+  hurd_notify_remove (n, item->key);
+  continue;
+}
+
+  count += 1;
+}
+
+  time_t start_time = time (NULL);
+  time_t remaining_time;
+  while (count > 0 &&
+ (remaining_time = timeout - (time (NULL) - start_time) * 1000) > 0)
+{
+  struct Reply {
+mach_msg_header_t Head;
+mach_msg_type_t RetCodeType;
+kern_return_t RetCode;
+  } reply;
+
+  /* Receive a reply message from any port in our port set with
+ timeout being set to the remaining time.  */
+  err = mach_msg (&reply.Head,
+  MACH_RCV_MSG|MACH_MSG_OPTION_NONE|MACH_RCV_TIMEOUT,
+  0,
+  sizeof (struct Reply),
+  port_set,
+  remaining_time,
+  MACH_PORT_NULL);
+  if (err)
+return err; /* XXX */
+
+  /* Remove that port from the set.  */
+  err = mach_port_move_member (mach_task_self (),
+   reply.Head.msgh_remote_port,
+