[PATCH] s390: Remove PT_IEEE_IP ptrace calls.

[PATCH] s390: Remove PT_IEEE_IP ptrace calls.

[Martin Schwidefsky]

Greetings,
as I revisited the ptrace code in the kernel to see what we have to change
for the new utrace interface and I noticed a bug in regard to PT_IEEE_IP.

The PT_IEEE_IP ptrace peek/poke calls are supposed to return/modify the
address of the last IEEE floating point fault. It can be called for a
traced child process and for the current(!) process. The later is used by
glibc in the fesetenv/fegetenv functions for the ieee_instruction_pointer
field of the fenv_t structure.

This self reference of a process via ptrace is rather alien to the ptrace
interface, the call never quite fit in. The curious thing now is that this
call has been broken for the last 6 years. A ptrace cleanup patch against
the kernel simply removed the support for PT_IEEE_IP, peeking always
returns 0 and poking the value has no effect. Nobody noticed for 6 years,
so I conclude that nobody really needs the call. In that case we can as
well get rid of it. Kernel patch will follow if this patch gets accepted.

blue skies,
  Martin.
---
2008-03-07  Martin Schwidefsky  <schwidefsky@de.ibm.com>

	* sysdeps/s390/fpu/fegetenv.c (fegetenv): Remove PTRACE_PEEKUSER
	ptrace call to get the ieee_instruction_pointer from the kernel.
	* sysdeps/s390/fpu/fesetenv.c (fesetenv): Remove PTRACE_POKEUSER
	ptrace call to set the ieee_instructtion_pointer.
	* sysdeps/unix/sysv/linux/s390/sys/user.h (struct _user_regs_struct):
	Add comment that ieee_instruction_pointer is always 0.


diff -urpN libc/sysdeps/s390/fpu/fegetenv.c libc-s390/sysdeps/s390/fpu/fegetenv.c
--- libc/sysdeps/s390/fpu/fegetenv.c	2003-05-05 20:22:18.000000000 +0200
+++ libc-s390/sysdeps/s390/fpu/fegetenv.c	2008-03-07 13:56:27.000000000 +0100
@@ -1,5 +1,5 @@
 /* Store current floating-point environment.
-   Copyright (C) 2000, 2003 Free Software Foundation, Inc.
+   Copyright (C) 2000, 2008 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Denis Joseph Barrow (djbarrow@de.ibm.com).
 
@@ -28,15 +28,7 @@
 int
 fegetenv (fenv_t *envp)
 {
-  /* The S/390 IEEE fpu doesn't keep track of the ieee instruction pointer.
-     To get around that the kernel will store the address of the last
-     fpu fault to the process structure. This ptrace call reads this value
-     from the kernel space. That means the ieee_instruction_pointer is
-     only correct after a fpu fault. That's the best we can do, there is
-     no way to find out the ieee instruction pointer if there was no fault.  */
   _FPU_GETCW (envp->fpc);
-  envp->ieee_instruction_pointer =
-    (void *) ptrace (PTRACE_PEEKUSER, getpid (), PT_IEEE_IP);
 
   /* Success.  */
   return 0;
diff -urpN libc/sysdeps/s390/fpu/fesetenv.c libc-s390/sysdeps/s390/fpu/fesetenv.c
--- libc/sysdeps/s390/fpu/fesetenv.c	2002-09-10 03:26:38.000000000 +0200
+++ libc-s390/sysdeps/s390/fpu/fesetenv.c	2008-03-07 13:56:47.000000000 +0100
@@ -1,5 +1,5 @@
 /* Install given floating-point environment.
-   Copyright (C) 2000, 2002 Free Software Foundation, Inc.
+   Copyright (C) 2000, 2008 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Denis Joseph Barrow (djbarrow@de.ibm.com).
 
@@ -43,13 +43,7 @@ fesetenv (const fenv_t *envp)
   else
     env = (*envp);
 
-  /* The S/390 IEEE fpu doesn't have a register for the ieee
-     instruction pointer. The operating system is required to keep an
-     instruction pointer on a per process base. We read and write this
-     value with the ptrace interface.  */
   _FPU_SETCW (env.fpc);
-  ptrace (PTRACE_POKEUSER, getpid (), PT_IEEE_IP,
-	  env.ieee_instruction_pointer);
 
   /* Success.  */
   return 0;
diff -urpN libc/sysdeps/unix/sysv/linux/s390/sys/user.h libc-s390/sysdeps/unix/sysv/linux/s390/sys/user.h
--- libc/sysdeps/unix/sysv/linux/s390/sys/user.h	2006-01-11 10:03:47.000000000 +0100
+++ libc-s390/sysdeps/unix/sysv/linux/s390/sys/user.h	2008-03-07 16:54:20.000000000 +0100
@@ -1,4 +1,4 @@
-/* Copyright (C) 2000,2005 Free Software Foundation, Inc.
+/* Copyright (C) 2000,2008 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -56,7 +56,7 @@ struct _user_regs_struct
   unsigned long orig_gpr2;		/* Original gpr2.  */
   struct _user_fpregs_struct fp_regs;	/* Floating point registers.  */
   struct _user_per_struct per_info;	/* Hardware tracing registers.  */
-  unsigned long ieee_instruction_pointer; 
+  unsigned long ieee_instruction_pointer;	/* Always 0.  */
 };
 
 struct user {

Re: [PATCH] s390: Remove PT_IEEE_IP ptrace calls.

[Roland McGrath]

If you aren't worried about any compatibility issue for s390, we surely aren't.
I assume you've checked with both users. ;-)


Thanks,
Roland

Re: [PATCH] s390: Remove PT_IEEE_IP ptrace calls.

[Martin Schwidefsky]

On Fri, 2008-03-07 at 10:58 -0800, Roland McGrath wrote:
> If you aren't worried about any compatibility issue for s390, we surely aren't.
> I assume you've checked with both users. ;-)

Compatibility issues for a field that does not contain the value it
should contain? It can't get more broken than it is today, all the major
distribution versions have a kernel that null the field. So I am not
worried about compatibility. In fact I am quite happy that we can remove
this nasty ptrace hack in fesetenv/fegetenv. It even showed up on some
benchmarks.

-- 
blue skies,
  Martin.

"Reality continues to ruin my life." - Calvin.

Re: [PATCH] s390: Remove PT_IEEE_IP ptrace calls.

[Martin Schwidefsky]

On Mon, 2008-03-10 at 09:04 +0100, Martin Schwidefsky wrote:
> On Fri, 2008-03-07 at 10:58 -0800, Roland McGrath wrote:
> > If you aren't worried about any compatibility issue for s390, we surely aren't.
> > I assume you've checked with both users. ;-)
> 
> Compatibility issues for a field that does not contain the value it
> should contain? It can't get more broken than it is today, all the major
> distribution versions have a kernel that null the field. So I am not
> worried about compatibility. In fact I am quite happy that we can remove
> this nasty ptrace hack in fesetenv/fegetenv. It even showed up on some
> benchmarks.

Made up you mind? I would like to see this patch in glibc accepted first
before we go ahead and remove the kernel code for it as well.

-- 
blue skies,
  Martin.

"Reality continues to ruin my life." - Calvin.

Re: [PATCH] s390: Remove PT_IEEE_IP ptrace calls.

[Roland McGrath]

> Made up you mind? I would like to see this patch in glibc accepted first
> before we go ahead and remove the kernel code for it as well.

What's the question?  Your change was committed a month ago and is in the
2.8 release.

Re: [PATCH] s390: Remove PT_IEEE_IP ptrace calls.

[Martin Schwidefsky]

Hi Roland,

On Tue, 2008-04-29 at 14:32 -0700, Roland McGrath wrote:
> > Made up you mind? I would like to see this patch in glibc accepted first
> > before we go ahead and remove the kernel code for it as well.
> 
> What's the question?  Your change was committed a month ago and is in the
> 2.8 release.

Perfect. I didn't see the usual "applied" mail on libc-hacker so I
assumed that it has not been applied. Just checked and indeed that
change is in 2.8. Thanks.

-- 
blue skies,
  Martin.

"Reality continues to ruin my life." - Calvin.