[RFA/RFC] Replace call_ptrace and ptrace_wait in inf-ptrace.c

[RFA/RFC] Replace call_ptrace and ptrace_wait in inf-ptrace.c

[Mark Kettenis]

This is another step in the direction of eliminating the need for both
inf-ptrace.c and infptrace.c.  It eliminates the calls to call_ptrace
and ptrace_wait.

Andrew has recently suggested that we'd want debugging support for the
ptrace(2) interface, which could be implemented by using call_ptrace()
unconditionally.  That, however, is a bad idea, since this makes it
impossible for the compiler to properly typecheck the arguments to
ptrace().  Hence the removal of call_ptrace() usage.

In inf_ptrace_kill_inferior, I just replaced ptrace_wait() with
wait().  Calling target_post_wait here seems inappropriate.

If nobody objetcs, I'll check this in later this week.

Mark


Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>

	* inf-ptrace.c (inf_ptrace_kill_inferior): Call ptrace directly
	instead of call_ptrace.  Call wait directly instead of
	ptrace_wait.
	(inf_ptrace_me): Call ptrace directly instead of call_ptrace.
	(inf_ptrace_wait): Inline ptrace_wait call.

Index: inf-ptrace.c
===================================================================
RCS file: /cvs/src/src/gdb/inf-ptrace.c,v
retrieving revision 1.2
diff -u -p -r1.2 inf-ptrace.c
--- inf-ptrace.c 16 Sep 2004 19:54:18 -0000 1.2
+++ inf-ptrace.c 20 Sep 2004 18:58:17 -0000
@@ -55,8 +55,8 @@ inf_ptrace_kill_inferior (void)
 
      The kill call causes problems under hpux10, so it's been removed;
      if this causes problems we'll deal with them as they arise.  */
-  call_ptrace (PT_KILL, pid, (PTRACE_TYPE_ARG3) 0, 0);
-  ptrace_wait (null_ptid, &status);
+  ptrace (PT_KILL, pid, (PTRACE_TYPE_ARG3) 0, 0);
+  wait (&status);
   target_mourn_inferior ();
 }
 
@@ -255,7 +255,8 @@ inf_ptrace_wait (ptid_t ptid, struct tar
 				   attached process. */
       set_sigio_trap ();
 
-      pid = ptrace_wait (inferior_ptid, &status);
+      pid = wait (&status);
+      target_post_wait (pid_to_ptid (pid), status);
 
       save_errno = errno;
 
@@ -433,7 +434,7 @@ static void
 inf_ptrace_me (void)
 {
   /* "Trace me, Dr. Memory!" */
-  call_ptrace (0, 0, (PTRACE_TYPE_ARG3) 0, 0);
+  ptrace (0, 0, (PTRACE_TYPE_ARG3) 0, 0);
 }
 
 /* Stub function which causes the GDB that runs it, to start ptrace-ing

Re: [RFA/RFC] Replace call_ptrace and ptrace_wait in inf-ptrace.c

[Andrew Cagney]

> This is another step in the direction of eliminating the need for both
> inf-ptrace.c and infptrace.c.  It eliminates the calls to call_ptrace
> and ptrace_wait.
> 
> Andrew has recently suggested that we'd want debugging support for the
> ptrace(2) interface, which could be implemented by using call_ptrace()
> unconditionally.

Having, again, spent some time debugging GNU/Linux threads, I'm pretty 
much certain of this.

 > That, however, is a bad idea, since this makes it
> impossible for the compiler to properly typecheck the arguments to
> ptrace().

How so?

I've noticed that ptrace can sometimes be declared with a variable 
number of arguments, but that just suggests there should be a 
gdb_ptrace4() and gdb_ptrace5() with explicitly 4 and 5 arguments.

Andrew

Re: [RFA/RFC] Replace call_ptrace and ptrace_wait in inf-ptrace.c

[Daniel Jacobowitz]

On Mon, Sep 20, 2004 at 03:34:41PM -0400, Andrew Cagney wrote:
> >This is another step in the direction of eliminating the need for both
> >inf-ptrace.c and infptrace.c.  It eliminates the calls to call_ptrace
> >and ptrace_wait.
> >
> >Andrew has recently suggested that we'd want debugging support for the
> >ptrace(2) interface, which could be implemented by using call_ptrace()
> >unconditionally.
> 
> Having, again, spent some time debugging GNU/Linux threads, I'm pretty 
> much certain of this.

I completely agree.

-- 
Daniel Jacobowitz

Re: [RFA/RFC] Replace call_ptrace and ptrace_wait in inf-ptrace.c

[Mark Kettenis]

   Date: Mon, 20 Sep 2004 15:34:41 -0400
   From: Andrew Cagney <cagney@gnu.org>

   > This is another step in the direction of eliminating the need for both
   > inf-ptrace.c and infptrace.c.  It eliminates the calls to call_ptrace
   > and ptrace_wait.
   > 
   > Andrew has recently suggested that we'd want debugging support for the
   > ptrace(2) interface, which could be implemented by using call_ptrace()
   > unconditionally.

   Having, again, spent some time debugging GNU/Linux threads, I'm pretty 
   much certain of this.

   > That, however, is a bad idea, since this makes it
   > impossible for the compiler to properly typecheck the arguments to
   > ptrace().

   How so?

The variety in ptrace(2) prototypes is pretty big.  Arguments can be
integer or pointer types of various sizes (32-bit, 64-bit).  We simply
cannot get that right for all supported operating systems.  So we have
to guess.  Being conservative, we use a long integer type, say
CORE_ADDR, for the n-th argument of call_ptrace().  Suppose that on an
LP64 platform we pass, by mistake, a pointer as the n-th argument of
ptrace, but that argument should really be an int.  Because of the
intermediate call_ptrace() the compiler doesn't warn us about it.  The
result is probably a mysterious bug.

If we'd used a macro instead, the compiler would have warned us.

   I've noticed that ptrace can sometimes be declared with a variable 
   number of arguments, but that just suggests there should be a 
   gdb_ptrace4() and gdb_ptrace5() with explicitly 4 and 5 arguments.

Linux does variable number of arguments, although the underlying
system call isn't.  I believe the 5-arg SunOS-compatible
PTRACE_READDATA on SPARC Linux simply doesn't work.

We shouldn't need an explicit 5-arg ptrace.  The fifth argument is
always zero in GDB.

Mark

Re: [RFA/RFC] Replace call_ptrace and ptrace_wait in inf-ptrace.c

[Andrew Cagney]

>    > That, however, is a bad idea, since this makes it
>    > impossible for the compiler to properly typecheck the arguments to
>    > ptrace().
> 
>    How so?
> 
> The variety in ptrace(2) prototypes is pretty big.  Arguments can be
> integer or pointer types of various sizes (32-bit, 64-bit).  We simply
> cannot get that right for all supported operating systems.  So we have
> to guess.  Being conservative, we use a long integer type, say
> CORE_ADDR, for the n-th argument of call_ptrace().  Suppose that on an
> LP64 platform we pass, by mistake, a pointer as the n-th argument of
> ptrace, but that argument should really be an int.  Because of the
> intermediate call_ptrace() the compiler doesn't warn us about it.  The
> result is probably a mysterious bug.

Either way we'll end up with casts:
	CORE_ADDR -> (void *)
	(void *) -> long
etc, why not have methods that at least avoid the casts (or do it 
locally to GDB's ptrace code?).

> If we'd used a macro instead, the compiler would have warned us.
> 
>    I've noticed that ptrace can sometimes be declared with a variable 
>    number of arguments, but that just suggests there should be a 
>    gdb_ptrace4() and gdb_ptrace5() with explicitly 4 and 5 arguments.
> 
> Linux does variable number of arguments, although the underlying
> system call isn't.  I believe the 5-arg SunOS-compatible
> PTRACE_READDATA on SPARC Linux simply doesn't work.
> 
> We shouldn't need an explicit 5-arg ptrace.  The fifth argument is
> always zero in GDB.

good news
Andrew

Re: [RFA/RFC] Replace call_ptrace and ptrace_wait in inf-ptrace.c

[Mark Kettenis]

   Date: Tue, 21 Sep 2004 09:07:28 -0400
   From: Andrew Cagney <cagney@gnu.org>

I committed the patch:

2004-09-24  Mark Kettenis  <kettenis@gnu.org>

	* inf-ptrace.c (inf_ptrace_kill_inferior): Call ptrace directly
	instead of call_ptrace.  Call wait directly instead of
	ptrace_wait.
	(inf_ptrace_me): Call ptrace directly instead of call_ptrace.
	(inf_ptrace_wait): Inline ptrace_wait call.

But this doesn't mean we shouldn't continue this discussion. It
actually makes things easier for us whatever the outcome because now
we can forget about replacing call_ptrace, and can just focus on
ptrace().  Please read on.

   > The variety in ptrace(2) prototypes is pretty big.  Arguments can be
   > integer or pointer types of various sizes (32-bit, 64-bit).  We simply
   > cannot get that right for all supported operating systems.  So we have
   > to guess.  Being conservative, we use a long integer type, say
   > CORE_ADDR, for the n-th argument of call_ptrace().  Suppose that on an
   > LP64 platform we pass, by mistake, a pointer as the n-th argument of
   > ptrace, but that argument should really be an int.  Because of the
   > intermediate call_ptrace() the compiler doesn't warn us about it.  The
   > result is probably a mysterious bug.

   Either way we'll end up with casts:
	   CORE_ADDR -> (void *)
	   (void *) -> long
   etc, why not have methods that at least avoid the casts (or do it 
   locally to GDB's ptrace code?).

The problem here is that the third argument of ptrace is used for two
purposes:

1. For specifying memory addresses of objects in the debugger's
   address space (PT_GETREGS, PT_SETREGS, PT_IO).

2. For specifying memory addresses in the inferior.

This gets especially complicated with 32x64 "native" cross-debugging
and is further complicated by the fact that some systems use an
integer type and other systems use a pointer type for this third
arguments.  I've tried to come up with a function signature for
gdb_ptrace() that would work for all targets, and I failed.

Mark

Re: [RFA/RFC] Replace call_ptrace and ptrace_wait in inf-ptrace.c

[Andrew Cagney]

>    Either way we'll end up with casts:
> 	   CORE_ADDR -> (void *)
> 	   (void *) -> long
>    etc, why not have methods that at least avoid the casts (or do it 
>    locally to GDB's ptrace code?).
> 
> The problem here is that the third argument of ptrace is used for two
> purposes:

> 1. For specifying memory addresses of objects in the debugger's
>    address space (PT_GETREGS, PT_SETREGS, PT_IO).
>
> 2. For specifying memory addresses in the inferior.
> 
> This gets especially complicated with 32x64 "native" cross-debugging
> and is further complicated by the fact that some systems use an
> integer type and other systems use a pointer type for this third
> arguments.

Right, no matter what a single function is, casing is required.

> I've tried to come up with a function signature for
> gdb_ptrace() that would work for all targets, and I failed.

With two ptrace functions, one with void* (for host buffer) and one with 
CORE_ADDR (for target address), this problem would be solved.

Andrew