[PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Stefan Liebler]

[-- Attachment #1: Type: text/plain, Size: 1999 bytes --]

Hi,

this patch removes PTRACE_GETREGS, PTRACE_SETREGS, PTRACE_GETFPREGS
and PTRACE_SETFPREGS as these requests does not exist on s390 kernel.

But the kernel has support for PTRACE_SINGLEBLOCK,
PTRACE_SECCOMP_GET_FILTER, PTRACE_PEEKUSR_AREA, PTRACE_POKEUSR_AREA,
PTRACE_GET_LAST_BREAK, PTRACE_ENABLE_TE, PTRACE_DISABLE_TE and
PTRACE_TE_ABORT_RAND.  Thus those are defined now.

The current kernel s390 specific ptrace.h file also defines
PTRACE_PEEKTEXT_AREA, PTRACE_PEEKDATA_AREA, PTRACE_POKETEXT_AREA,
PTRACE_POKEDATA_AREA, PTRACE_PEEK_SYSTEM_CALL, PTRACE_POKE_SYSTEM_CALL
and PTRACE_PROT, but those requests are not supported.
Thus those defines are skipped in glibc ptrace.h.

There were old includes of ptrace.h in sysdeps/s390/fpu/fesetenv.c.
The ptrace feature isn't used there anymore, thus I removed the includes.

Before this patch, <glibc>/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
uses ptrace-request 12 for PTRACE_GETREGS,
but <kernel>/include/uapi/linux/ptrace.h uses 12 for PTRACE_SINGLEBLOCK.

The s390 kernel has never had support for PTRACE_GETREGS!
Thus glibc ptrace.h is adjusted to match kernel ptrace.h.

The new s390 specific test ensures, that PTRACE_SINGLEBLOCK defined
in glibc works as expected.  If the kernel would interpret it as
PTRACE_GETREGS, then the testcase will not make any progress
and will time out.

Bye.
Stefan

ChangeLog:

	[BZ #21539]
	* sysdeps/unix/sysv/linux/s390/sys/ptrace.h
	(PTRACE_GETREGS, PTRACE_SETREGS, PTRACE_GETFPREGS,
	PTRACE_SETFPREGS): Remove enum constant.
	(PT_GETREGS, PT_SETREGS, PT_GETFPREGS, T_SETFPREGS):
	Remove defines.
	(PTRACE_SINGLEBLOCK): New enum constant.
	(PT_STEPBLOCK): New define.
	(PTRACE_PEEKUSR_AREA, PTRACE_POKEUSR_AREA,
	PTRACE_GET_LAST_BREAK, PTRACE_ENABLE_TE, PTRACE_DISABLE_TE,
	PTRACE_TE_ABORT_RAND): New enum constant and define.
	* sysdeps/s390/fpu/fesetenv.c: Remove ptrace.h includes.
	* sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c:
	New file.
	* sysdeps/unix/sysv/linux/s390/Makefile: Add test.

[-- Attachment #2: 20170606_s390_ptraceh.patch --]
[-- Type: text/x-patch, Size: 8670 bytes --]

commit c24a9b5367e91cf36a5a309eced8d0e13bc951e4
Author: Stefan Liebler <stli@linux.vnet.ibm.com>
Date:   Fri Jun 2 15:31:34 2017 +0200

    S390: Sync ptrace.h with kernel. [BZ #21539]
    
    This patch removes PTRACE_GETREGS, PTRACE_SETREGS, PTRACE_GETFPREGS
    and PTRACE_SETFPREGS as these requests does not exist on s390 kernel.
    
    But the kernel has support for PTRACE_SINGLEBLOCK,
    PTRACE_SECCOMP_GET_FILTER, PTRACE_PEEKUSR_AREA, PTRACE_POKEUSR_AREA,
    PTRACE_GET_LAST_BREAK, PTRACE_ENABLE_TE, PTRACE_DISABLE_TE and
    PTRACE_TE_ABORT_RAND.  Thus those are defined now.
    
    The current kernel s390 specific ptrace.h file also defines
    PTRACE_PEEKTEXT_AREA, PTRACE_PEEKDATA_AREA, PTRACE_POKETEXT_AREA,
    PTRACE_POKEDATA_AREA, PTRACE_PEEK_SYSTEM_CALL, PTRACE_POKE_SYSTEM_CALL
    and PTRACE_PROT, but those requests are not supported.
    Thus those defines are skipped in glibc ptrace.h.
    
    There were old includes of ptrace.h in sysdeps/s390/fpu/fesetenv.c.
    The ptrace feature isn't used there anymore, thus I removed the includes.
    
    Before this patch, <glibc>/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
    uses ptrace-request 12 for PTRACE_GETREGS,
    but <kernel>/include/uapi/linux/ptrace.h uses 12 for PTRACE_SINGLEBLOCK.
    
    The s390 kernel has never had support for PTRACE_GETREGS!
    Thus glibc ptrace.h is adjusted to match kernel ptrace.h.
    
    The new s390 specific test ensures, that PTRACE_SINGLEBLOCK defined
    in glibc works as expected.  If the kernel would interpret it as
    PTRACE_GETREGS, then the testcase will not make any progress
    and will time out.
    
    ChangeLog:
    
    	[BZ #21539]
    	* sysdeps/unix/sysv/linux/s390/sys/ptrace.h
    	(PTRACE_GETREGS, PTRACE_SETREGS, PTRACE_GETFPREGS,
    	PTRACE_SETFPREGS): Remove enum constant.
    	(PT_GETREGS, PT_SETREGS, PT_GETFPREGS, T_SETFPREGS):
    	Remove defines.
    	(PTRACE_SINGLEBLOCK): New enum constant.
    	(PT_STEPBLOCK): New define.
    	(PTRACE_PEEKUSR_AREA, PTRACE_POKEUSR_AREA,
    	PTRACE_GET_LAST_BREAK, PTRACE_ENABLE_TE, PTRACE_DISABLE_TE,
    	PTRACE_TE_ABORT_RAND): New enum constant and define.
    	* sysdeps/s390/fpu/fesetenv.c: Remove ptrace.h includes.
    	* sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c:
    	New file.
    	* sysdeps/unix/sysv/linux/s390/Makefile: Add test.

diff --git a/sysdeps/s390/fpu/fesetenv.c b/sysdeps/s390/fpu/fesetenv.c
index 4c9bcf0..0f64a3f 100644
--- a/sysdeps/s390/fpu/fesetenv.c
+++ b/sysdeps/s390/fpu/fesetenv.c
@@ -20,8 +20,6 @@
 #include <fenv_libc.h>
 #include <fpu_control.h>
 #include <stddef.h>
-#include <asm/ptrace.h>
-#include <sys/ptrace.h>
 #include <unistd.h>
 
 int
diff --git a/sysdeps/unix/sysv/linux/s390/Makefile b/sysdeps/unix/sysv/linux/s390/Makefile
index 3867c33..f30a6bb 100644
--- a/sysdeps/unix/sysv/linux/s390/Makefile
+++ b/sysdeps/unix/sysv/linux/s390/Makefile
@@ -29,3 +29,7 @@ CFLAGS-elision-trylock.c = $(elision-CFLAGS)
 CFLAGS-elision-unlock.c = $(elision-CFLAGS)
 endif
 endif
+
+ifeq ($(subdir),misc)
+tests += tst-ptrace-singleblock
+endif
diff --git a/sysdeps/unix/sysv/linux/s390/sys/ptrace.h b/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
index 7caf101..88079fc 100644
--- a/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
+++ b/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
@@ -89,25 +89,9 @@ enum __ptrace_request
   PTRACE_SINGLESTEP = 9,
 #define PT_STEP PTRACE_SINGLESTEP
 
-  /* Get all general purpose registers used by a processes.
-     This is not supported on all machines.  */
-   PTRACE_GETREGS = 12,
-#define PT_GETREGS PTRACE_GETREGS
-
-  /* Set all general purpose registers used by a processes.
-     This is not supported on all machines.  */
-   PTRACE_SETREGS = 13,
-#define PT_SETREGS PTRACE_SETREGS
-
-  /* Get all floating point registers used by a processes.
-     This is not supported on all machines.  */
-   PTRACE_GETFPREGS = 14,
-#define PT_GETFPREGS PTRACE_GETFPREGS
-
-  /* Set all floating point registers used by a processes.
-     This is not supported on all machines.  */
-   PTRACE_SETFPREGS = 15,
-#define PT_SETFPREGS PTRACE_SETFPREGS
+  /* Execute process until next taken branch.  */
+  PTRACE_SINGLEBLOCK = 12,
+#define PT_STEPBLOCK PTRACE_SINGLEBLOCK
 
   /* Attach to a process that is already running. */
   PTRACE_ATTACH = 16,
@@ -167,8 +151,26 @@ enum __ptrace_request
   PTRACE_SETSIGMASK = 0x420b,
 #define PTRACE_SETSIGMASK PTRACE_SETSIGMASK
 
-  PTRACE_SECCOMP_GET_FILTER = 0x420c
+  PTRACE_SECCOMP_GET_FILTER = 0x420c,
 #define PTRACE_SECCOMP_GET_FILTER PTRACE_SECCOMP_GET_FILTER
+
+  PTRACE_PEEKUSR_AREA = 0x5000,
+#define PTRACE_PEEKUSR_AREA PTRACE_PEEKUSR_AREA
+
+  PTRACE_POKEUSR_AREA = 0x5001,
+#define PTRACE_POKEUSR_AREA PTRACE_POKEUSR_AREA
+
+  PTRACE_GET_LAST_BREAK = 0x5006,
+#define PTRACE_GET_LAST_BREAK PTRACE_GET_LAST_BREAK
+
+  PTRACE_ENABLE_TE = 0x5009,
+#define PTRACE_ENABLE_TE PTRACE_ENABLE_TE
+
+  PTRACE_DISABLE_TE = 0x5010,
+#define PTRACE_DISABLE_TE PTRACE_DISABLE_TE
+
+  PTRACE_TE_ABORT_RAND = 0x5011
+#define PTRACE_TE_ABORT_RAND PTRACE_TE_ABORT_RAND
 };
 
 
diff --git a/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c b/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c
new file mode 100644
index 0000000..b384562
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c
@@ -0,0 +1,110 @@
+/* Testing s390x PTRACE_SINGLEBLOCK ptrace request.
+   Copyright (C) 2017 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
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library 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
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <sys/wait.h>
+#include <sys/types.h>
+#include <support/xunistd.h>
+
+/* Ensure that we use the PTRACE_SINGLEBLOCK definition from glibc ptrace.h
+   in tracer_func.  We need the kernel ptrace.h for structs ptrace_area
+   and gregset_t.  */
+#include <sys/ptrace.h>
+static const enum __ptrace_request req_singleblock = PTRACE_SINGLEBLOCK;
+#include <asm/ptrace.h>
+
+static void
+tracee_func (int pid)
+{
+  /* Dump the mapping information for manual inspection of the printed
+     tracee addresses.  */
+  char str[80];
+  sprintf (str, "cat /proc/%d/maps", pid);
+  puts (str);
+  system (str);
+  fflush (stdout);
+
+  ptrace (PTRACE_TRACEME);
+  /* Stop tracee.  Afterwards the tracer_func can operate.  */
+  kill (pid, SIGSTOP);
+
+  puts ("The PTRACE_SINGLEBLOCK of the tracer will stop after: "
+	"brasl %r14,<puts@plt>!");
+}
+
+static void
+tracer_func (int pid)
+{
+  unsigned long last_break;
+  ptrace_area parea;
+  gregset_t regs;
+  int status;
+
+  while (1)
+    {
+      /* Wait for the tracee to be stopped or exited.  */
+      wait (&status);
+      if (WIFEXITED (status))
+	break;
+
+      /* Get information about tracee: gprs, last breaking address.  */
+      parea.len = sizeof (regs);
+      parea.process_addr = (unsigned long) &regs;
+      parea.kernel_addr = 0;
+      ptrace (PTRACE_PEEKUSR_AREA, pid, &parea);
+      ptrace (PTRACE_GET_LAST_BREAK, pid, NULL, &last_break);
+
+      printf ("child IA: %p last_break: %p\n",
+	      (void *) regs[1], (void *) last_break);
+
+      /* Execute tracee until next taken branch.
+
+	 Note:
+	 Before the commit which introduced this testcase,
+	 <glibc>/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
+	 uses ptrace-request 12 for PTRACE_GETREGS,
+	 but <kernel>/include/uapi/linux/ptrace.h
+	 uses 12 for PTRACE_SINGLEBLOCK.
+
+	 The s390 kernel has no support for PTRACE_GETREGS!
+	 Thus glibc ptrace.h is adjusted to match kernel ptrace.h.
+
+	 This test ensures, that PTRACE_SINGLEBLOCK defined in glibc
+	 works as expected.  If the kernel would interpret it as
+	 PTRACE_GETREGS, then the tracee will not make any progress
+	 and this testcase will time out.  */
+      ptrace (req_singleblock, pid, NULL, NULL);
+    }
+}
+
+static int
+do_test (void)
+{
+  int pid;
+  pid = xfork ();
+  if (pid)
+    tracer_func (pid);
+  else
+    tracee_func (getpid ());
+
+  return EXIT_SUCCESS;
+}
+
+#include <support/test-driver.c>

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Andreas Schwab]

On Jun 06 2017, Stefan Liebler <stli@linux.vnet.ibm.com> wrote:

> Hi,
>
> this patch removes PTRACE_GETREGS, PTRACE_SETREGS, PTRACE_GETFPREGS
> and PTRACE_SETFPREGS as these requests does not exist on s390 kernel.

Since it is an API change it should probably get a NEWS entry.

> +static void
> +tracer_func (int pid)
> +{
> +  unsigned long last_break;
> +  ptrace_area parea;
> +  gregset_t regs;
> +  int status;
> +
> +  while (1)
> +    {
> +      /* Wait for the tracee to be stopped or exited.  */
> +      wait (&status);

Doesn't that need to use WUNTRACED?

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Dmitry V. Levin]

[-- Attachment #1: Type: text/plain, Size: 620 bytes --]

On Tue, Jun 06, 2017 at 12:44:04PM +0200, Andreas Schwab wrote:
> On Jun 06 2017, Stefan Liebler <stli@linux.vnet.ibm.com> wrote:
[...]
> > +static void
> > +tracer_func (int pid)
> > +{
> > +  unsigned long last_break;
> > +  ptrace_area parea;
> > +  gregset_t regs;
> > +  int status;
> > +
> > +  while (1)
> > +    {
> > +      /* Wait for the tracee to be stopped or exited.  */
> > +      wait (&status);
> 
> Doesn't that need to use WUNTRACED?

No, it doesn't: as the tracee has called PTRACE_TRACEME before raising
SIGSTOP, there is going to be a ptrace signal delivery stop.


-- 
ldv

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Stefan Liebler]

On 06/06/2017 12:44 PM, Andreas Schwab wrote:
> On Jun 06 2017, Stefan Liebler <stli@linux.vnet.ibm.com> wrote:
> 
>> Hi,
>>
>> this patch removes PTRACE_GETREGS, PTRACE_SETREGS, PTRACE_GETFPREGS
>> and PTRACE_SETFPREGS as these requests does not exist on s390 kernel.
> 
> Since it is an API change it should probably get a NEWS entry.
> 
I've added this NEWS entry:
* The s390 specific ptrace requests are adjusted to the kernel ones.
   Request 12 is now used for PTRACE_SINGLEBLOCK instead of
   PTRACE_GETREGS.  The requests PTRACE_GETREGS, PTRACE_SETREGS,
   PTRACE_GETFPREGS and PTRACE_SETFPREGS were removed as those are not
   supported by the s390 kernel.  The requests PTRACE_SINGLEBLOCK,
   PTRACE_SECCOMP_GET_FILTER, PTRACE_PEEKUSR_AREA, PTRACE_POKEUSR_AREA,
   PTRACE_GET_LAST_BREAK, PTRACE_ENABLE_TE, PTRACE_DISABLE_TE and
   PTRACE_TE_ABORT_RAND were added as those are supported by the s390
   kernel.

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Stefan Liebler]

On 06/06/2017 01:56 PM, Stefan Liebler wrote:
> On 06/06/2017 12:44 PM, Andreas Schwab wrote:
>> On Jun 06 2017, Stefan Liebler <stli@linux.vnet.ibm.com> wrote:
>>
>>> Hi,
>>>
>>> this patch removes PTRACE_GETREGS, PTRACE_SETREGS, PTRACE_GETFPREGS
>>> and PTRACE_SETFPREGS as these requests does not exist on s390 kernel.
>>
>> Since it is an API change it should probably get a NEWS entry.
>>
> I've added this NEWS entry:
> * The s390 specific ptrace requests are adjusted to the kernel ones.
>    Request 12 is now used for PTRACE_SINGLEBLOCK instead of
>    PTRACE_GETREGS.  The requests PTRACE_GETREGS, PTRACE_SETREGS,
>    PTRACE_GETFPREGS and PTRACE_SETFPREGS were removed as those are not
>    supported by the s390 kernel.  The requests PTRACE_SINGLEBLOCK,
>    PTRACE_SECCOMP_GET_FILTER, PTRACE_PEEKUSR_AREA, PTRACE_POKEUSR_AREA,
>    PTRACE_GET_LAST_BREAK, PTRACE_ENABLE_TE, PTRACE_DISABLE_TE and
>    PTRACE_TE_ABORT_RAND were added as those are supported by the s390
>    kernel.
> 

Any objection?
Otherwise I'll commit the patch tomorrow.

Bye
Stefan

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Dmitry V. Levin]

[-- Attachment #1: Type: text/plain, Size: 6328 bytes --]

On Tue, Jun 06, 2017 at 12:17:33PM +0200, Stefan Liebler wrote:
[...]
> diff --git a/sysdeps/unix/sysv/linux/s390/sys/ptrace.h b/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
> index 7caf101..88079fc 100644
> --- a/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
> +++ b/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
> @@ -89,25 +89,9 @@ enum __ptrace_request
>    PTRACE_SINGLESTEP = 9,
>  #define PT_STEP PTRACE_SINGLESTEP
>  
> -  /* Get all general purpose registers used by a processes.
> -     This is not supported on all machines.  */
> -   PTRACE_GETREGS = 12,
> -#define PT_GETREGS PTRACE_GETREGS
> -
> -  /* Set all general purpose registers used by a processes.
> -     This is not supported on all machines.  */
> -   PTRACE_SETREGS = 13,
> -#define PT_SETREGS PTRACE_SETREGS
> -
> -  /* Get all floating point registers used by a processes.
> -     This is not supported on all machines.  */
> -   PTRACE_GETFPREGS = 14,
> -#define PT_GETFPREGS PTRACE_GETFPREGS
> -
> -  /* Set all floating point registers used by a processes.
> -     This is not supported on all machines.  */
> -   PTRACE_SETFPREGS = 15,
> -#define PT_SETFPREGS PTRACE_SETFPREGS
> +  /* Execute process until next taken branch.  */
> +  PTRACE_SINGLEBLOCK = 12,
> +#define PT_STEPBLOCK PTRACE_SINGLEBLOCK
>  
>    /* Attach to a process that is already running. */
>    PTRACE_ATTACH = 16,
> @@ -167,8 +151,26 @@ enum __ptrace_request
>    PTRACE_SETSIGMASK = 0x420b,
>  #define PTRACE_SETSIGMASK PTRACE_SETSIGMASK
>  
> -  PTRACE_SECCOMP_GET_FILTER = 0x420c
> +  PTRACE_SECCOMP_GET_FILTER = 0x420c,
>  #define PTRACE_SECCOMP_GET_FILTER PTRACE_SECCOMP_GET_FILTER
> +
> +  PTRACE_PEEKUSR_AREA = 0x5000,
> +#define PTRACE_PEEKUSR_AREA PTRACE_PEEKUSR_AREA
> +
> +  PTRACE_POKEUSR_AREA = 0x5001,
> +#define PTRACE_POKEUSR_AREA PTRACE_POKEUSR_AREA
> +
> +  PTRACE_GET_LAST_BREAK = 0x5006,
> +#define PTRACE_GET_LAST_BREAK PTRACE_GET_LAST_BREAK
> +
> +  PTRACE_ENABLE_TE = 0x5009,
> +#define PTRACE_ENABLE_TE PTRACE_ENABLE_TE
> +
> +  PTRACE_DISABLE_TE = 0x5010,
> +#define PTRACE_DISABLE_TE PTRACE_DISABLE_TE
> +
> +  PTRACE_TE_ABORT_RAND = 0x5011
> +#define PTRACE_TE_ABORT_RAND PTRACE_TE_ABORT_RAND
>  };

The sys/ptrace.h part of the change looks fine.

> diff --git a/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c b/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c
> new file mode 100644
> index 0000000..b384562
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c
> @@ -0,0 +1,110 @@
> +/* Testing s390x PTRACE_SINGLEBLOCK ptrace request.
> +   Copyright (C) 2017 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
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library 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
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <sys/wait.h>
> +#include <sys/types.h>
> +#include <support/xunistd.h>
> +
> +/* Ensure that we use the PTRACE_SINGLEBLOCK definition from glibc ptrace.h
> +   in tracer_func.  We need the kernel ptrace.h for structs ptrace_area
> +   and gregset_t.  */
> +#include <sys/ptrace.h>
> +static const enum __ptrace_request req_singleblock = PTRACE_SINGLEBLOCK;
> +#include <asm/ptrace.h>
> +
> +static void
> +tracee_func (int pid)
> +{
> +  /* Dump the mapping information for manual inspection of the printed
> +     tracee addresses.  */
> +  char str[80];
> +  sprintf (str, "cat /proc/%d/maps", pid);
> +  puts (str);
> +  system (str);
> +  fflush (stdout);
> +
> +  ptrace (PTRACE_TRACEME);
> +  /* Stop tracee.  Afterwards the tracer_func can operate.  */
> +  kill (pid, SIGSTOP);
> +
> +  puts ("The PTRACE_SINGLEBLOCK of the tracer will stop after: "
> +	"brasl %r14,<puts@plt>!");
> +}
> +
> +static void
> +tracer_func (int pid)
> +{
> +  unsigned long last_break;
> +  ptrace_area parea;
> +  gregset_t regs;
> +  int status;
> +
> +  while (1)
> +    {
> +      /* Wait for the tracee to be stopped or exited.  */
> +      wait (&status);
> +      if (WIFEXITED (status))
> +	break;
> +
> +      /* Get information about tracee: gprs, last breaking address.  */
> +      parea.len = sizeof (regs);
> +      parea.process_addr = (unsigned long) &regs;
> +      parea.kernel_addr = 0;
> +      ptrace (PTRACE_PEEKUSR_AREA, pid, &parea);

Note that you can verify whether PTRACE_PEEKUSR_AREA has returned
the expected result by comparing registers with those returned
by PTRACE_GETREGSET.  The latter is implemented on s390 since
linux 2.6.27 so its use in glibc is safe.

> +      ptrace (PTRACE_GET_LAST_BREAK, pid, NULL, &last_break);

As these ptrace calls are expected to succeed,
you might want to check their return code.

> +
> +      printf ("child IA: %p last_break: %p\n",
> +	      (void *) regs[1], (void *) last_break);
> +
> +      /* Execute tracee until next taken branch.
> +
> +	 Note:
> +	 Before the commit which introduced this testcase,
> +	 <glibc>/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
> +	 uses ptrace-request 12 for PTRACE_GETREGS,
> +	 but <kernel>/include/uapi/linux/ptrace.h
> +	 uses 12 for PTRACE_SINGLEBLOCK.
> +
> +	 The s390 kernel has no support for PTRACE_GETREGS!
> +	 Thus glibc ptrace.h is adjusted to match kernel ptrace.h.
> +
> +	 This test ensures, that PTRACE_SINGLEBLOCK defined in glibc
> +	 works as expected.  If the kernel would interpret it as
> +	 PTRACE_GETREGS, then the tracee will not make any progress
> +	 and this testcase will time out.  */
> +      ptrace (req_singleblock, pid, NULL, NULL);

Likewise.


-- 
ldv

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Stefan Liebler]

[-- Attachment #1: Type: text/plain, Size: 2130 bytes --]

On 06/13/2017 10:05 PM, Dmitry V. Levin wrote:
> On Tue, Jun 06, 2017 at 12:17:33PM +0200, Stefan Liebler wrote:
> [...]
>> diff --git a/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c b/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c
>> new file mode 100644
 >> [...]
>> +      /* Get information about tracee: gprs, last breaking address.  */
>> +      parea.len = sizeof (regs);
>> +      parea.process_addr = (unsigned long) &regs;
>> +      parea.kernel_addr = 0;
>> +      ptrace (PTRACE_PEEKUSR_AREA, pid, &parea);
> 
> Note that you can verify whether PTRACE_PEEKUSR_AREA has returned
> the expected result by comparing registers with those returned
> by PTRACE_GETREGSET.  The latter is implemented on s390 since
> linux 2.6.27 so its use in glibc is safe.
> 
Okay. Now the gprs are obtained by PTRACE_PEEKUSR_AREA and 
PTRACE_GETREGSET. Afterwards I use memcmp to check whether the values 
are the same.
>> +      ptrace (PTRACE_GET_LAST_BREAK, pid, NULL, &last_break);
> 
> As these ptrace calls are expected to succeed,
> you might want to check their return code.
> 
Done with several usages of TEST_VERIFY_EXIT.

>> +
>> +      printf ("child IA: %p last_break: %p\n",
>> +	      (void *) regs[1], (void *) last_break);
>> +
>> +      /* Execute tracee until next taken branch.
>> +
>> +	 Note:
>> +	 Before the commit which introduced this testcase,
>> +	 <glibc>/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
>> +	 uses ptrace-request 12 for PTRACE_GETREGS,
>> +	 but <kernel>/include/uapi/linux/ptrace.h
>> +	 uses 12 for PTRACE_SINGLEBLOCK.
>> +
>> +	 The s390 kernel has no support for PTRACE_GETREGS!
>> +	 Thus glibc ptrace.h is adjusted to match kernel ptrace.h.
>> +
>> +	 This test ensures, that PTRACE_SINGLEBLOCK defined in glibc
>> +	 works as expected.  If the kernel would interpret it as
>> +	 PTRACE_GETREGS, then the tracee will not make any progress
>> +	 and this testcase will time out.  */
>> +      ptrace (req_singleblock, pid, NULL, NULL);
> 
> Likewise.
> 
> 


I've attached the patch with the mentioned changes and the NEWS entry 
requested by Andreas.

Is this okay?

Bye
Stefan

[-- Attachment #2: 20170619_s390_ptraceh.patch --]
[-- Type: text/x-patch, Size: 10287 bytes --]

commit 99d6dcd30803d992b2e82c1edb8db797f4ace846
Author: Stefan Liebler <stli@linux.vnet.ibm.com>
Date:   Mon Jun 19 15:02:02 2017 +0200

    S390: Sync ptrace.h with kernel. [BZ #21539]
    
    This patch removes PTRACE_GETREGS, PTRACE_SETREGS, PTRACE_GETFPREGS
    and PTRACE_SETFPREGS as these requests does not exist on s390 kernel.
    
    But the kernel has support for PTRACE_SINGLEBLOCK,
    PTRACE_SECCOMP_GET_FILTER, PTRACE_PEEKUSR_AREA, PTRACE_POKEUSR_AREA,
    PTRACE_GET_LAST_BREAK, PTRACE_ENABLE_TE, PTRACE_DISABLE_TE and
    PTRACE_TE_ABORT_RAND.  Thus those are defined now.
    
    The current kernel s390 specific ptrace.h file also defines
    PTRACE_PEEKTEXT_AREA, PTRACE_PEEKDATA_AREA, PTRACE_POKETEXT_AREA,
    PTRACE_POKEDATA_AREA, PTRACE_PEEK_SYSTEM_CALL, PTRACE_POKE_SYSTEM_CALL
    and PTRACE_PROT, but those requests are not supported.
    Thus those defines are skipped in glibc ptrace.h.
    
    There were old includes of ptrace.h in sysdeps/s390/fpu/fesetenv.c.
    The ptrace feature isn't used there anymore, thus I removed the includes.
    
    Before this patch, <glibc>/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
    uses ptrace-request 12 for PTRACE_GETREGS,
    but <kernel>/include/uapi/linux/ptrace.h uses 12 for PTRACE_SINGLEBLOCK.
    
    The s390 kernel has never had support for PTRACE_GETREGS!
    Thus glibc ptrace.h is adjusted to match kernel ptrace.h.
    
    The new s390 specific test ensures, that PTRACE_SINGLEBLOCK defined
    in glibc works as expected.  If the kernel would interpret it as
    PTRACE_GETREGS, then the testcase will not make any progress
    and will time out.
    
    ChangeLog:
    
    	[BZ #21539]
    	* NEWS: Mention s390 ptrace request changes.
    	* sysdeps/unix/sysv/linux/s390/sys/ptrace.h
    	(PTRACE_GETREGS, PTRACE_SETREGS, PTRACE_GETFPREGS,
    	PTRACE_SETFPREGS): Remove enum constant.
    	(PT_GETREGS, PT_SETREGS, PT_GETFPREGS, T_SETFPREGS):
    	Remove defines.
    	(PTRACE_SINGLEBLOCK): New enum constant.
    	(PT_STEPBLOCK): New define.
    	(PTRACE_PEEKUSR_AREA, PTRACE_POKEUSR_AREA,
    	PTRACE_GET_LAST_BREAK, PTRACE_ENABLE_TE, PTRACE_DISABLE_TE,
    	PTRACE_TE_ABORT_RAND): New enum constant and define.
    	* sysdeps/s390/fpu/fesetenv.c: Remove ptrace.h includes.
    	* sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c:
    	New file.
    	* sysdeps/unix/sysv/linux/s390/Makefile: Add test.

diff --git a/NEWS b/NEWS
index 804c1b9..9ee2218 100644
--- a/NEWS
+++ b/NEWS
@@ -98,6 +98,15 @@ Version 2.26
 * The tunables feature is now enabled by default.  This allows users to tweak
   behavior of the GNU C Library using the GLIBC_TUNABLES environment variable.
 
+* The s390 specific ptrace requests are adjusted to the kernel ones.  Request 12
+  is now used for PTRACE_SINGLEBLOCK instead of PTRACE_GETREGS.  The requests
+  PTRACE_GETREGS, PTRACE_SETREGS, PTRACE_GETFPREGS and PTRACE_SETFPREGS were
+  removed as those are not supported by the s390 kernel.  The requests
+  PTRACE_SINGLEBLOCK, PTRACE_SECCOMP_GET_FILTER, PTRACE_PEEKUSR_AREA,
+  PTRACE_POKEUSR_AREA, PTRACE_GET_LAST_BREAK, PTRACE_ENABLE_TE,
+  PTRACE_DISABLE_TE and PTRACE_TE_ABORT_RAND were added as those are supported
+  by the s390 kernel.
+
 Security related changes:
 
 * The DNS stub resolver limits the advertised UDP buffer size to 1200 bytes,
diff --git a/sysdeps/s390/fpu/fesetenv.c b/sysdeps/s390/fpu/fesetenv.c
index 4c9bcf0..0f64a3f 100644
--- a/sysdeps/s390/fpu/fesetenv.c
+++ b/sysdeps/s390/fpu/fesetenv.c
@@ -20,8 +20,6 @@
 #include <fenv_libc.h>
 #include <fpu_control.h>
 #include <stddef.h>
-#include <asm/ptrace.h>
-#include <sys/ptrace.h>
 #include <unistd.h>
 
 int
diff --git a/sysdeps/unix/sysv/linux/s390/Makefile b/sysdeps/unix/sysv/linux/s390/Makefile
index 3867c33..f30a6bb 100644
--- a/sysdeps/unix/sysv/linux/s390/Makefile
+++ b/sysdeps/unix/sysv/linux/s390/Makefile
@@ -29,3 +29,7 @@ CFLAGS-elision-trylock.c = $(elision-CFLAGS)
 CFLAGS-elision-unlock.c = $(elision-CFLAGS)
 endif
 endif
+
+ifeq ($(subdir),misc)
+tests += tst-ptrace-singleblock
+endif
diff --git a/sysdeps/unix/sysv/linux/s390/sys/ptrace.h b/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
index 7caf101..88079fc 100644
--- a/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
+++ b/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
@@ -89,25 +89,9 @@ enum __ptrace_request
   PTRACE_SINGLESTEP = 9,
 #define PT_STEP PTRACE_SINGLESTEP
 
-  /* Get all general purpose registers used by a processes.
-     This is not supported on all machines.  */
-   PTRACE_GETREGS = 12,
-#define PT_GETREGS PTRACE_GETREGS
-
-  /* Set all general purpose registers used by a processes.
-     This is not supported on all machines.  */
-   PTRACE_SETREGS = 13,
-#define PT_SETREGS PTRACE_SETREGS
-
-  /* Get all floating point registers used by a processes.
-     This is not supported on all machines.  */
-   PTRACE_GETFPREGS = 14,
-#define PT_GETFPREGS PTRACE_GETFPREGS
-
-  /* Set all floating point registers used by a processes.
-     This is not supported on all machines.  */
-   PTRACE_SETFPREGS = 15,
-#define PT_SETFPREGS PTRACE_SETFPREGS
+  /* Execute process until next taken branch.  */
+  PTRACE_SINGLEBLOCK = 12,
+#define PT_STEPBLOCK PTRACE_SINGLEBLOCK
 
   /* Attach to a process that is already running. */
   PTRACE_ATTACH = 16,
@@ -167,8 +151,26 @@ enum __ptrace_request
   PTRACE_SETSIGMASK = 0x420b,
 #define PTRACE_SETSIGMASK PTRACE_SETSIGMASK
 
-  PTRACE_SECCOMP_GET_FILTER = 0x420c
+  PTRACE_SECCOMP_GET_FILTER = 0x420c,
 #define PTRACE_SECCOMP_GET_FILTER PTRACE_SECCOMP_GET_FILTER
+
+  PTRACE_PEEKUSR_AREA = 0x5000,
+#define PTRACE_PEEKUSR_AREA PTRACE_PEEKUSR_AREA
+
+  PTRACE_POKEUSR_AREA = 0x5001,
+#define PTRACE_POKEUSR_AREA PTRACE_POKEUSR_AREA
+
+  PTRACE_GET_LAST_BREAK = 0x5006,
+#define PTRACE_GET_LAST_BREAK PTRACE_GET_LAST_BREAK
+
+  PTRACE_ENABLE_TE = 0x5009,
+#define PTRACE_ENABLE_TE PTRACE_ENABLE_TE
+
+  PTRACE_DISABLE_TE = 0x5010,
+#define PTRACE_DISABLE_TE PTRACE_DISABLE_TE
+
+  PTRACE_TE_ABORT_RAND = 0x5011
+#define PTRACE_TE_ABORT_RAND PTRACE_TE_ABORT_RAND
 };
 
 
diff --git a/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c b/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c
new file mode 100644
index 0000000..95a2f55
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c
@@ -0,0 +1,128 @@
+/* Testing s390x PTRACE_SINGLEBLOCK ptrace request.
+   Copyright (C) 2017 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
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library 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
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/wait.h>
+#include <sys/types.h>
+#include <sys/uio.h>
+#include <elf.h>
+#include <support/xunistd.h>
+#include <support/check.h>
+
+/* Ensure that we use the PTRACE_SINGLEBLOCK definition from glibc ptrace.h
+   in tracer_func.  We need the kernel ptrace.h for structs ptrace_area
+   and gregset_t.  */
+#include <sys/ptrace.h>
+static const enum __ptrace_request req_singleblock = PTRACE_SINGLEBLOCK;
+#include <asm/ptrace.h>
+
+static void
+tracee_func (int pid)
+{
+  /* Dump the mapping information for manual inspection of the printed
+     tracee addresses.  */
+  char str[80];
+  sprintf (str, "cat /proc/%d/maps", pid);
+  puts (str);
+  system (str);
+  fflush (stdout);
+
+  TEST_VERIFY_EXIT (ptrace (PTRACE_TRACEME) == 0);
+  /* Stop tracee.  Afterwards the tracer_func can operate.  */
+  kill (pid, SIGSTOP);
+
+  puts ("The PTRACE_SINGLEBLOCK of the tracer will stop after: "
+	"brasl %r14,<puts@plt>!");
+}
+
+static void
+tracer_func (int pid)
+{
+  unsigned long last_break;
+  ptrace_area parea;
+  gregset_t regs;
+  struct iovec parea2;
+  gregset_t regs2;
+
+  int status;
+
+  while (1)
+    {
+      /* Wait for the tracee to be stopped or exited.  */
+      wait (&status);
+      if (WIFEXITED (status))
+	break;
+
+      /* Get information about tracee: gprs, last breaking address.  */
+      parea.len = sizeof (regs);
+      parea.process_addr = (unsigned long) &regs;
+      parea.kernel_addr = 0;
+      TEST_VERIFY_EXIT (ptrace (PTRACE_PEEKUSR_AREA, pid, &parea) == 0);
+      TEST_VERIFY_EXIT (ptrace (PTRACE_GET_LAST_BREAK, pid, NULL, &last_break)
+			== 0);
+
+      parea2.iov_len = sizeof (regs2);
+      parea2.iov_base = &regs2;
+      TEST_VERIFY_EXIT (ptrace (PTRACE_GETREGSET, pid, NT_PRSTATUS, &parea2)
+			== 0);
+      TEST_VERIFY_EXIT (parea2.iov_len == sizeof (regs2));
+
+      /* Test if gprs obtained by PTRACE_PEEKUSR_AREA and PTRACE_GETREGESET
+	 have the same values.  */
+      TEST_VERIFY_EXIT (memcmp (&regs, &regs2, sizeof (regs)) == 0);
+
+      printf ("child IA: %p last_break: %p\n",
+	      (void *) regs[1], (void *) last_break);
+
+      /* Execute tracee until next taken branch.
+
+	 Note:
+	 Before the commit which introduced this testcase,
+	 <glibc>/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
+	 uses ptrace-request 12 for PTRACE_GETREGS,
+	 but <kernel>/include/uapi/linux/ptrace.h
+	 uses 12 for PTRACE_SINGLEBLOCK.
+
+	 The s390 kernel has no support for PTRACE_GETREGS!
+	 Thus glibc ptrace.h is adjusted to match kernel ptrace.h.
+
+	 This test ensures, that PTRACE_SINGLEBLOCK defined in glibc
+	 works as expected.  If the kernel would interpret it as
+	 PTRACE_GETREGS, then the tracee will not make any progress
+	 and this testcase will time out.  */
+      TEST_VERIFY_EXIT (ptrace (req_singleblock, pid, NULL, NULL) == 0);
+    }
+}
+
+static int
+do_test (void)
+{
+  int pid;
+  pid = xfork ();
+  if (pid)
+    tracer_func (pid);
+  else
+    tracee_func (getpid ());
+
+  return EXIT_SUCCESS;
+}
+
+#include <support/test-driver.c>

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Dmitry V. Levin]

[-- Attachment #1: Type: text/plain, Size: 2378 bytes --]

On Mon, Jun 19, 2017 at 03:10:57PM +0200, Stefan Liebler wrote:
> On 06/13/2017 10:05 PM, Dmitry V. Levin wrote:
> > On Tue, Jun 06, 2017 at 12:17:33PM +0200, Stefan Liebler wrote:
> > [...]
> >> diff --git a/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c b/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c
> >> new file mode 100644
>  >> [...]
> >> +      /* Get information about tracee: gprs, last breaking address.  */
> >> +      parea.len = sizeof (regs);
> >> +      parea.process_addr = (unsigned long) &regs;
> >> +      parea.kernel_addr = 0;
> >> +      ptrace (PTRACE_PEEKUSR_AREA, pid, &parea);
> > 
> > Note that you can verify whether PTRACE_PEEKUSR_AREA has returned
> > the expected result by comparing registers with those returned
> > by PTRACE_GETREGSET.  The latter is implemented on s390 since
> > linux 2.6.27 so its use in glibc is safe.
> > 
> Okay. Now the gprs are obtained by PTRACE_PEEKUSR_AREA and 
> PTRACE_GETREGSET. Afterwards I use memcmp to check whether the values 
> are the same.
> >> +      ptrace (PTRACE_GET_LAST_BREAK, pid, NULL, &last_break);
> > 
> > As these ptrace calls are expected to succeed,
> > you might want to check their return code.
> > 
> Done with several usages of TEST_VERIFY_EXIT.
> 
> >> +
> >> +      printf ("child IA: %p last_break: %p\n",
> >> +	      (void *) regs[1], (void *) last_break);
> >> +
> >> +      /* Execute tracee until next taken branch.
> >> +
> >> +	 Note:
> >> +	 Before the commit which introduced this testcase,
> >> +	 <glibc>/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
> >> +	 uses ptrace-request 12 for PTRACE_GETREGS,
> >> +	 but <kernel>/include/uapi/linux/ptrace.h
> >> +	 uses 12 for PTRACE_SINGLEBLOCK.
> >> +
> >> +	 The s390 kernel has no support for PTRACE_GETREGS!
> >> +	 Thus glibc ptrace.h is adjusted to match kernel ptrace.h.
> >> +
> >> +	 This test ensures, that PTRACE_SINGLEBLOCK defined in glibc
> >> +	 works as expected.  If the kernel would interpret it as
> >> +	 PTRACE_GETREGS, then the tracee will not make any progress
> >> +	 and this testcase will time out.  */
> >> +      ptrace (req_singleblock, pid, NULL, NULL);
> > 
> > Likewise.
> 
> I've attached the patch with the mentioned changes and the NEWS entry 
> requested by Andreas.
> 
> Is this okay?

Looks fine, thanks.


-- 
ldv

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Stefan Liebler]

On 06/19/2017 03:26 PM, Dmitry V. Levin wrote:
> On Mon, Jun 19, 2017 at 03:10:57PM +0200, Stefan Liebler wrote:
>> On 06/13/2017 10:05 PM, Dmitry V. Levin wrote:
>>> On Tue, Jun 06, 2017 at 12:17:33PM +0200, Stefan Liebler wrote:
>>> [...]
>>>> diff --git a/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c b/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c
>>>> new file mode 100644
>>   >> [...]
>>>> +      /* Get information about tracee: gprs, last breaking address.  */
>>>> +      parea.len = sizeof (regs);
>>>> +      parea.process_addr = (unsigned long) &regs;
>>>> +      parea.kernel_addr = 0;
>>>> +      ptrace (PTRACE_PEEKUSR_AREA, pid, &parea);
>>>
>>> Note that you can verify whether PTRACE_PEEKUSR_AREA has returned
>>> the expected result by comparing registers with those returned
>>> by PTRACE_GETREGSET.  The latter is implemented on s390 since
>>> linux 2.6.27 so its use in glibc is safe.
>>>
>> Okay. Now the gprs are obtained by PTRACE_PEEKUSR_AREA and
>> PTRACE_GETREGSET. Afterwards I use memcmp to check whether the values
>> are the same.
>>>> +      ptrace (PTRACE_GET_LAST_BREAK, pid, NULL, &last_break);
>>>
>>> As these ptrace calls are expected to succeed,
>>> you might want to check their return code.
>>>
>> Done with several usages of TEST_VERIFY_EXIT.
>>
>>>> +
>>>> +      printf ("child IA: %p last_break: %p\n",
>>>> +	      (void *) regs[1], (void *) last_break);
>>>> +
>>>> +      /* Execute tracee until next taken branch.
>>>> +
>>>> +	 Note:
>>>> +	 Before the commit which introduced this testcase,
>>>> +	 <glibc>/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
>>>> +	 uses ptrace-request 12 for PTRACE_GETREGS,
>>>> +	 but <kernel>/include/uapi/linux/ptrace.h
>>>> +	 uses 12 for PTRACE_SINGLEBLOCK.
>>>> +
>>>> +	 The s390 kernel has no support for PTRACE_GETREGS!
>>>> +	 Thus glibc ptrace.h is adjusted to match kernel ptrace.h.
>>>> +
>>>> +	 This test ensures, that PTRACE_SINGLEBLOCK defined in glibc
>>>> +	 works as expected.  If the kernel would interpret it as
>>>> +	 PTRACE_GETREGS, then the tracee will not make any progress
>>>> +	 and this testcase will time out.  */
>>>> +      ptrace (req_singleblock, pid, NULL, NULL);
>>>
>>> Likewise.
>>
>> I've attached the patch with the mentioned changes and the NEWS entry
>> requested by Andreas.
>>
>> Is this okay?
> 
> Looks fine, thanks.
> 
> 
Committed.

Thanks.
Stefan

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Florian Weimer]

On 06/19/2017 03:10 PM, Stefan Liebler wrote:
>     	* sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c:
>     	New file.

The test fails for me like this on s390x:

[root@ibm-z-25 build]# bash testrun.sh misc/tst-ptrace-singleblock
cat /proc/23548/maps
80000000-80004000 r-xp 00000000 fd:00 1883202
/root/build/misc/tst-ptrace-singleblock
80004000-80005000 r--p 00003000 fd:00 1883202
/root/build/misc/tst-ptrace-singleblock
80005000-80006000 rw-p 00004000 fd:00 1883202
/root/build/misc/tst-ptrace-singleblock
2aa00794000-2aa007ba000 r-xp 00000000 fd:00 34794498
/root/build/elf/ld.so
2aa007ba000-2aa007bb000 r--p 00025000 fd:00 34794498
/root/build/elf/ld.so
2aa007bb000-2aa007bd000 rw-p 00026000 fd:00 34794498
/root/build/elf/ld.so
3fffd1e9000-3fffd1ea000 rw-s 00000000 00:04 146001
/dev/zero (deleted)
3fffd1ea000-3fffd1ec000 rw-p 00000000 00:00 0
3fffd1ec000-3fffd38d000 r-xp 00000000 fd:00 34794501
/root/build/libc.so
3fffd38d000-3fffd391000 r--p 001a0000 fd:00 34794501
/root/build/libc.so
3fffd391000-3fffd393000 rw-p 001a4000 fd:00 34794501
/root/build/libc.so
3fffd393000-3fffd39a000 rw-p 00000000 00:00 0
3ffffe62000-3fffff83000 rw-p 00000000 00:00 0
[stack]
child IA: 0x3fffd22ac62 last_break: 0x2aa0079e218
error: ../sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c:111: not
true: ptrace (req_singleblock, pid, NULL, NULL) == 0
The PTRACE_SINGLEBLOCK of the tracer will stop after: brasl %r14,<puts@plt>!
error: 1 test failures
[root@ibm-z-25 build]#

Relevant package versions:

kernel-3.10.0-689.el7.s390x
kernel-headers-3.10.0-689.el7.s390x
devtoolset-6-gcc-6.3.1-3.1.el7.s390x

Please let me know if you need additional information to debug this.

Thanks,
Florian

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Stefan Liebler]

[-- Attachment #1: Type: text/plain, Size: 3611 bytes --]

On 06/30/2017 12:09 PM, Florian Weimer wrote:
> On 06/19/2017 03:10 PM, Stefan Liebler wrote:
>>      	* sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c:
>>      	New file.
> 
> The test fails for me like this on s390x:
> 
> [root@ibm-z-25 build]# bash testrun.sh misc/tst-ptrace-singleblock
> cat /proc/23548/maps
> 80000000-80004000 r-xp 00000000 fd:00 1883202
> /root/build/misc/tst-ptrace-singleblock
> 80004000-80005000 r--p 00003000 fd:00 1883202
> /root/build/misc/tst-ptrace-singleblock
> 80005000-80006000 rw-p 00004000 fd:00 1883202
> /root/build/misc/tst-ptrace-singleblock
> 2aa00794000-2aa007ba000 r-xp 00000000 fd:00 34794498
> /root/build/elf/ld.so
> 2aa007ba000-2aa007bb000 r--p 00025000 fd:00 34794498
> /root/build/elf/ld.so
> 2aa007bb000-2aa007bd000 rw-p 00026000 fd:00 34794498
> /root/build/elf/ld.so
> 3fffd1e9000-3fffd1ea000 rw-s 00000000 00:04 146001
> /dev/zero (deleted)
> 3fffd1ea000-3fffd1ec000 rw-p 00000000 00:00 0
> 3fffd1ec000-3fffd38d000 r-xp 00000000 fd:00 34794501
> /root/build/libc.so
> 3fffd38d000-3fffd391000 r--p 001a0000 fd:00 34794501
> /root/build/libc.so
> 3fffd391000-3fffd393000 rw-p 001a4000 fd:00 34794501
> /root/build/libc.so
> 3fffd393000-3fffd39a000 rw-p 00000000 00:00 0
> 3ffffe62000-3fffff83000 rw-p 00000000 00:00 0
> [stack]
> child IA: 0x3fffd22ac62 last_break: 0x2aa0079e218
> error: ../sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c:111: not
> true: ptrace (req_singleblock, pid, NULL, NULL) == 0
> The PTRACE_SINGLEBLOCK of the tracer will stop after: brasl %r14,<puts@plt>!
> error: 1 test failures
> [root@ibm-z-25 build]#
> 
> Relevant package versions:
> 
> kernel-3.10.0-689.el7.s390x
> kernel-headers-3.10.0-689.el7.s390x
> devtoolset-6-gcc-6.3.1-3.1.el7.s390x
> 
> Please let me know if you need additional information to debug this.
> 
> Thanks,
> Florian
> 

Uuups. I also see this behaviour on a RHEL 7.3 machine.
We've checked that request 12 wasn't used in s390 kernel for other 
meanings as PTRACE_SINGLEBLOCK. But we've missed the fact, that it has 
been introduced with kernel 3.15. On older kernels -1 is returned with 
errno EIO.

Please have a look at the attached patch. I've adjusted the test:

Now ptrace request 12 is first done with data argument pointing to
a buffer:
-If request 12 is interpreted as PTRACE_GETREGS, it will store the regs
to buffer without an error. Here the test expects that the buffer is
untouched and an error is returned.

-If request 12 is interpreted as PTRACE_SINGLEBLOCK, it will fail
as data argument is no valid signal.

-If request 12 is not implemented, it will also fail.


Afterwards the request 12 is done with zero data argument:
-If the kernel has support for PTRACE_SINGLEBLOCK (then the kernel
header asm/ptrace.h defines this macro), the ptrace call is not allowed
to fail and has to continue the tracee until next taken branch.

-If the kernel has no support for PTRACE_SINGLEBLOCK, the ptrace call
has to fail with EIO. Then I continue the tracee with PTRACE_CONT.

-If the request 12 is interpreted as PTRACE_GETREGS, it will fail too.
It fails with EFAULT on intel / power as data argument is NULL.
According to the man-page: "Unfortunately, under Linux, different
variations of this fault will return EIO or EFAULT more or less
arbitrarily".
But if request 12 is interpreted as PTRACE_GETREGS, the first ptrace
call will touch the buffer which is detected by this test.

Any thoughts?
If this change is okay, I'll commit it.

Thanks.
Stefan


ChangeLog:

	* sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c:
	Support running on kernels without PTRACE_SINGLEBLOCK.

[-- Attachment #2: 20170704_s390_ptrace_h.patch --]
[-- Type: text/x-patch, Size: 4708 bytes --]

commit 5e668ce6055d6c690dec773b41c11a801265cd21
Author: Stefan Liebler <stli@linux.vnet.ibm.com>
Date:   Tue Jul 4 10:20:05 2017 +0200

    S390: Fix tst-ptrace-singleblock if kernel does not support PTRACE_SINGLEBLOCK.
    
    The request PTRACE_SINGLEBLOCK was introduced  in Linux 3.15.  Thus the ptrace call
    will fail on older kernels.
    Thus the test is now testing PTRACE_SINGLEBLOCK with data argument pointing to a
    buffer on stack which is assumed to fail.  If the request would be interpreted as
    PTRACE_GETREGS, then the ptrace call will not fail and the regs are written to buf.
    
    If we run with a kernel with support for PTRACE_SINGLEBLOCK a ptrace call with
    data=NULL, is not allowed to fail.  If we run with a kernel without support for
    PTRACE_SINGLEBLOCK a ptrace call with data=NULL, has to fail with EIO.
    
    ChangeLog:
    
    	* sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c:
    	Support running on kernels without PTRACE_SINGLEBLOCK.

diff --git a/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c b/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c
index 95a2f55..65858cb 100644
--- a/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c
+++ b/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c
@@ -26,6 +26,8 @@
 #include <elf.h>
 #include <support/xunistd.h>
 #include <support/check.h>
+#include <string.h>
+#include <errno.h>
 
 /* Ensure that we use the PTRACE_SINGLEBLOCK definition from glibc ptrace.h
    in tracer_func.  We need the kernel ptrace.h for structs ptrace_area
@@ -63,6 +65,10 @@ tracer_func (int pid)
   gregset_t regs2;
 
   int status;
+  int ret;
+#define MAX_CHARS_IN_BUF 4096
+  char buf[MAX_CHARS_IN_BUF + 1];
+  size_t buf_count;
 
   while (1)
     {
@@ -104,11 +110,67 @@ tracer_func (int pid)
 	 The s390 kernel has no support for PTRACE_GETREGS!
 	 Thus glibc ptrace.h is adjusted to match kernel ptrace.h.
 
+	 The glibc sys/ptrace.h header contains the identifier
+	 PTRACE_SINGLEBLOCK in enum __ptrace_request.  In contrast, the kernel
+	 asm/ptrace.h header defines PTRACE_SINGLEBLOCK.
+
 	 This test ensures, that PTRACE_SINGLEBLOCK defined in glibc
 	 works as expected.  If the kernel would interpret it as
 	 PTRACE_GETREGS, then the tracee will not make any progress
-	 and this testcase will time out.  */
-      TEST_VERIFY_EXIT (ptrace (req_singleblock, pid, NULL, NULL) == 0);
+	 and this testcase will time out or the ptrace call will fail with
+	 different errors.  */
+
+      /* Ptrace request 12 is first done with data argument pointing to
+	 a buffer:
+	 -If request 12 is interpreted as PTRACE_GETREGS, it will store the regs
+	 to buffer without an error.
+
+	 -If request 12 is interpreted as PTRACE_SINGLEBLOCK, it will fail
+	 as data argument is used as signal-number and the address of
+	 buf is no valid signal.
+
+	 -If request 12 is not implemented, it will also fail.
+
+	 Here the test expects that the buffer is untouched and an error is
+	 returned.  */
+      memset (buf, 'a', MAX_CHARS_IN_BUF);
+      ret = ptrace (req_singleblock, pid, NULL, buf);
+      buf [MAX_CHARS_IN_BUF] = '\0';
+      buf_count = strspn (buf, "a");
+      TEST_VERIFY_EXIT (buf_count == MAX_CHARS_IN_BUF);
+      TEST_VERIFY_EXIT (ret == -1);
+
+      /* Ptrace request 12 is done with zero data argument:
+	 -If the kernel has support for PTRACE_SINGLEBLOCK (then the kernel
+	 header asm/ptrace.h defines this macro), the ptrace call is not allowed
+	 to fail and has to continue the tracee until next taken branch.
+
+	 -If the kernel (<3.15) has no support for PTRACE_SINGLEBLOCK, the
+	 ptrace call has to fail with EIO. Then I continue the tracee with
+	 PTRACE_CONT.
+
+	 -If the request 12 is interpreted as PTRACE_GETREGS, it will fail too.
+	 It fails with EFAULT on intel / power as data argument is NULL.
+	 According to the man-page: "Unfortunately, under Linux, different
+	 variations of this fault will return EIO or EFAULT more or less
+	 arbitrarily".
+	 But if request 12 is interpreted as PTRACE_GETREGS, the first ptrace
+	 call will touch the buffer which is detected by this test.  */
+      errno = 0;
+      ret = ptrace (req_singleblock, pid, NULL, NULL);
+#ifdef PTRACE_SINGLEBLOCK
+      /* The kernel has support for PTRACE_SINGLEBLOCK ptrace request. */
+      TEST_VERIFY_EXIT (errno == 0);
+      TEST_VERIFY_EXIT (ret == 0);
+#else
+      /* The kernel (< 3.15) has no support for PTRACE_SINGLEBLOCK ptrace
+	 request. */
+      TEST_VERIFY_EXIT (errno == EIO);
+      TEST_VERIFY_EXIT (ret == -1);
+
+      /* Just continue tracee until it exits normally.  */
+      TEST_VERIFY_EXIT (ptrace (PTRACE_CONT, pid, NULL, NULL) == 0);
+#endif
     }
 }
 

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Florian Weimer]

On 07/04/2017 10:22 AM, Stefan Liebler wrote:
> +      /* Ptrace request 12 is done with zero data argument:
> +	 -If the kernel has support for PTRACE_SINGLEBLOCK (then the kernel
> +	 header asm/ptrace.h defines this macro), the ptrace call is not allowed
> +	 to fail and has to continue the tracee until next taken branch.

I think this is still bogus.  We can compile with newer kernel headers
than the host kernel, and this will cause the test to fail.

Thanks,
Florian

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Stefan Liebler]

On 07/04/2017 11:41 AM, Florian Weimer wrote:
> On 07/04/2017 10:22 AM, Stefan Liebler wrote:
>> +      /* Ptrace request 12 is done with zero data argument:
>> +	 -If the kernel has support for PTRACE_SINGLEBLOCK (then the kernel
>> +	 header asm/ptrace.h defines this macro), the ptrace call is not allowed
>> +	 to fail and has to continue the tracee until next taken branch.
> 
> I think this is still bogus.  We can compile with newer kernel headers
> than the host kernel, and this will cause the test to fail.
> 
> Thanks,
> Florian
> 

Okay.
So I can check the return value of the second ptrace (req_singleblock, 
pid, NULL, NULL) call at runtime to determine the kernel-support:

       errno = 0;
       ret = ptrace (req_singleblock, pid, NULL, NULL);
       if (ret == 0)
	{
	  /* The kernel has support for PTRACE_SINGLEBLOCK ptrace request. */
	  TEST_VERIFY_EXIT (errno == 0);
	}
       else
	{
	  /* The kernel (< 3.15) has no support for PTRACE_SINGLEBLOCK ptrace
	     request. */
	  TEST_VERIFY_EXIT (errno == EIO);
	  TEST_VERIFY_EXIT (ret == -1);

	  /* Just continue tracee until it exits normally.  */
	  TEST_VERIFY_EXIT (ptrace (PTRACE_CONT, pid, NULL, NULL) == 0);
	}



Then the test runs on kernels with / without support for 
PTRACE_SINGLEBLOCK. The first ptrace call ensures that request 12 is not 
interpreted as PTRACE_GETREGS.

Bye.
Stefan

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Stefan Liebler]

[-- Attachment #1: Type: text/plain, Size: 1654 bytes --]

On 07/04/2017 05:37 PM, Stefan Liebler wrote:
> On 07/04/2017 11:41 AM, Florian Weimer wrote:
>> On 07/04/2017 10:22 AM, Stefan Liebler wrote:
>>> +      /* Ptrace request 12 is done with zero data argument:
>>> +     -If the kernel has support for PTRACE_SINGLEBLOCK (then the kernel
>>> +     header asm/ptrace.h defines this macro), the ptrace call is not 
>>> allowed
>>> +     to fail and has to continue the tracee until next taken branch.
>>
>> I think this is still bogus.  We can compile with newer kernel headers
>> than the host kernel, and this will cause the test to fail.
>>
>> Thanks,
>> Florian
>>
> 
> Okay.
> So I can check the return value of the second ptrace (req_singleblock, 
> pid, NULL, NULL) call at runtime to determine the kernel-support:
> 
>        errno = 0;
>        ret = ptrace (req_singleblock, pid, NULL, NULL);
>        if (ret == 0)
>      {
>        /* The kernel has support for PTRACE_SINGLEBLOCK ptrace request. */
>        TEST_VERIFY_EXIT (errno == 0);
>      }
>        else
>      {
>        /* The kernel (< 3.15) has no support for PTRACE_SINGLEBLOCK ptrace
>           request. */
>        TEST_VERIFY_EXIT (errno == EIO);
>        TEST_VERIFY_EXIT (ret == -1);
> 
>        /* Just continue tracee until it exits normally.  */
>        TEST_VERIFY_EXIT (ptrace (PTRACE_CONT, pid, NULL, NULL) == 0);
>      }
> 
> 
> 
> Then the test runs on kernels with / without support for 
> PTRACE_SINGLEBLOCK. The first ptrace call ensures that request 12 is not 
> interpreted as PTRACE_GETREGS.
> 
> Bye.
> Stefan
> 

Here is the complete patch with the change mentioned above.
Is this approach okay?

Bye.
Stefan

[-- Attachment #2: 20170707_s390_ptrace_h.patch --]
[-- Type: text/x-patch, Size: 4731 bytes --]

commit 5319bad7263aa1dc69da37bd4873876189ee5e97
Author: Stefan Liebler <stli@linux.vnet.ibm.com>
Date:   Fri Jul 7 10:29:56 2017 +0200

    S390: Fix tst-ptrace-singleblock if kernel does not support PTRACE_SINGLEBLOCK.
    
    The request PTRACE_SINGLEBLOCK was introduced  in Linux 3.15.  Thus the ptrace call
    will fail on older kernels.
    Thus the test is now testing PTRACE_SINGLEBLOCK with data argument pointing to a
    buffer on stack which is assumed to fail.  If the request would be interpreted as
    PTRACE_GETREGS, then the ptrace call will not fail and the regs are written to buf.
    
    If we run with a kernel with support for PTRACE_SINGLEBLOCK a ptrace call with
    data=NULL, returns zero with no error.  If we run with a kernel without support for
    PTRACE_SINGLEBLOCK a ptrace call with data=NULL reports an error.
    In the latter case, the test is just continuing with PTRACE_CONT.
    
    ChangeLog:
    
    	* sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c:
    	Support running on kernels without PTRACE_SINGLEBLOCK.

diff --git a/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c b/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c
index 95a2f55..6e4ba00 100644
--- a/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c
+++ b/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c
@@ -26,6 +26,8 @@
 #include <elf.h>
 #include <support/xunistd.h>
 #include <support/check.h>
+#include <string.h>
+#include <errno.h>
 
 /* Ensure that we use the PTRACE_SINGLEBLOCK definition from glibc ptrace.h
    in tracer_func.  We need the kernel ptrace.h for structs ptrace_area
@@ -63,6 +65,10 @@ tracer_func (int pid)
   gregset_t regs2;
 
   int status;
+  int ret;
+#define MAX_CHARS_IN_BUF 4096
+  char buf[MAX_CHARS_IN_BUF + 1];
+  size_t buf_count;
 
   while (1)
     {
@@ -104,11 +110,69 @@ tracer_func (int pid)
 	 The s390 kernel has no support for PTRACE_GETREGS!
 	 Thus glibc ptrace.h is adjusted to match kernel ptrace.h.
 
+	 The glibc sys/ptrace.h header contains the identifier
+	 PTRACE_SINGLEBLOCK in enum __ptrace_request.  In contrast, the kernel
+	 asm/ptrace.h header defines PTRACE_SINGLEBLOCK.
+
 	 This test ensures, that PTRACE_SINGLEBLOCK defined in glibc
 	 works as expected.  If the kernel would interpret it as
 	 PTRACE_GETREGS, then the tracee will not make any progress
-	 and this testcase will time out.  */
-      TEST_VERIFY_EXIT (ptrace (req_singleblock, pid, NULL, NULL) == 0);
+	 and this testcase will time out or the ptrace call will fail with
+	 different errors.  */
+
+      /* Ptrace request 12 is first done with data argument pointing to
+	 a buffer:
+	 -If request 12 is interpreted as PTRACE_GETREGS, it will store the regs
+	 to buffer without an error.
+
+	 -If request 12 is interpreted as PTRACE_SINGLEBLOCK, it will fail
+	 as data argument is used as signal-number and the address of
+	 buf is no valid signal.
+
+	 -If request 12 is not implemented, it will also fail.
+
+	 Here the test expects that the buffer is untouched and an error is
+	 returned.  */
+      memset (buf, 'a', MAX_CHARS_IN_BUF);
+      ret = ptrace (req_singleblock, pid, NULL, buf);
+      buf [MAX_CHARS_IN_BUF] = '\0';
+      buf_count = strspn (buf, "a");
+      TEST_VERIFY_EXIT (buf_count == MAX_CHARS_IN_BUF);
+      TEST_VERIFY_EXIT (ret == -1);
+
+      /* Ptrace request 12 is done with zero data argument:
+	 -If the kernel has support for PTRACE_SINGLEBLOCK (then the kernel
+	 header asm/ptrace.h defines this macro), the ptrace call is not allowed
+	 to fail and has to continue the tracee until next taken branch.
+
+	 -If the kernel (<3.15) has no support for PTRACE_SINGLEBLOCK, the
+	 ptrace call has to fail with EIO. Then I continue the tracee with
+	 PTRACE_CONT.
+
+	 -If the request 12 is interpreted as PTRACE_GETREGS, it will fail too.
+	 It fails with EFAULT on intel / power as data argument is NULL.
+	 According to the man-page: "Unfortunately, under Linux, different
+	 variations of this fault will return EIO or EFAULT more or less
+	 arbitrarily".
+	 But if request 12 is interpreted as PTRACE_GETREGS, the first ptrace
+	 call will touch the buffer which is detected by this test.  */
+      errno = 0;
+      ret = ptrace (req_singleblock, pid, NULL, NULL);
+      if (ret == 0)
+	{
+	  /* The kernel has support for PTRACE_SINGLEBLOCK ptrace request. */
+	  TEST_VERIFY_EXIT (errno == 0);
+	}
+      else
+	{
+	  /* The kernel (< 3.15) has no support for PTRACE_SINGLEBLOCK ptrace
+	     request. */
+	  TEST_VERIFY_EXIT (errno == EIO);
+	  TEST_VERIFY_EXIT (ret == -1);
+
+	  /* Just continue tracee until it exits normally.  */
+	  TEST_VERIFY_EXIT (ptrace (PTRACE_CONT, pid, NULL, NULL) == 0);
+	}
     }
 }
 

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Florian Weimer]

On 07/07/2017 12:22 PM, Stefan Liebler wrote:
> +      /* Ptrace request 12 is done with zero data argument:
> +	 -If the kernel has support for PTRACE_SINGLEBLOCK (then the kernel
> +	 header asm/ptrace.h defines this macro), the ptrace call is not allowed
> +	 to fail and has to continue the tracee until next taken branch.
> +
> +	 -If the kernel (<3.15) has no support for PTRACE_SINGLEBLOCK, the
> +	 ptrace call has to fail with EIO. Then I continue the tracee with
> +	 PTRACE_CONT.
> +
> +	 -If the request 12 is interpreted as PTRACE_GETREGS, it will fail too.
> +	 It fails with EFAULT on intel / power as data argument is NULL.
> +	 According to the man-page: "Unfortunately, under Linux, different
> +	 variations of this fault will return EIO or EFAULT more or less
> +	 arbitrarily".
> +	 But if request 12 is interpreted as PTRACE_GETREGS, the first ptrace
> +	 call will touch the buffer which is detected by this test.  */

I think the comment is still a bit off.  I think it is only necessary to
retain the second two lines, the other things is already implied by the
short comments in the code below.

(I have not tested whether this actually works.  I assume you have
checked a couple of userspace/kernel permutations.)

Thanks,
Florian

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Stefan Liebler]

[-- Attachment #1: Type: text/plain, Size: 1480 bytes --]

On 07/07/2017 12:45 PM, Florian Weimer wrote:
> On 07/07/2017 12:22 PM, Stefan Liebler wrote:
>> +      /* Ptrace request 12 is done with zero data argument:
>> +	 -If the kernel has support for PTRACE_SINGLEBLOCK (then the kernel
>> +	 header asm/ptrace.h defines this macro), the ptrace call is not allowed
>> +	 to fail and has to continue the tracee until next taken branch.
>> +
>> +	 -If the kernel (<3.15) has no support for PTRACE_SINGLEBLOCK, the
>> +	 ptrace call has to fail with EIO. Then I continue the tracee with
>> +	 PTRACE_CONT.
>> +
>> +	 -If the request 12 is interpreted as PTRACE_GETREGS, it will fail too.
>> +	 It fails with EFAULT on intel / power as data argument is NULL.
>> +	 According to the man-page: "Unfortunately, under Linux, different
>> +	 variations of this fault will return EIO or EFAULT more or less
>> +	 arbitrarily".
>> +	 But if request 12 is interpreted as PTRACE_GETREGS, the first ptrace
>> +	 call will touch the buffer which is detected by this test.  */
> 
> I think the comment is still a bit off.  I think it is only necessary to
> retain the second two lines, the other things is already implied by the
> short comments in the code below.
okay. I removed the other lines.
> 
> (I have not tested whether this actually works.  I assume you have
> checked a couple of userspace/kernel permutations.)
Yes. I've run the test on several machines.
> 
> Thanks,
> Florian
> 


If this is okay, I'll commit it on Monday.

Bye.
Stefan

[-- Attachment #2: 20170707_1500_s390_ptrace_h.patch --]
[-- Type: text/x-patch, Size: 4008 bytes --]

commit 5a547ac7caceb9ad2ac4903966b9d5294dfa4f7c
Author: Stefan Liebler <stli@linux.vnet.ibm.com>
Date:   Fri Jul 7 15:07:42 2017 +0200

    S390: Fix tst-ptrace-singleblock if kernel does not support PTRACE_SINGLEBLOCK.
    
    The request PTRACE_SINGLEBLOCK was introduced in Linux 3.15.  Thus the ptrace call
    will fail on older kernels.
    Thus the test is now testing PTRACE_SINGLEBLOCK with data argument pointing to a
    buffer on stack which is assumed to fail.  If the request would be interpreted as
    PTRACE_GETREGS, then the ptrace call will not fail and the regs are written to buf.
    
    If we run with a kernel with support for PTRACE_SINGLEBLOCK a ptrace call with
    data=NULL, returns zero with no error.  If we run with a kernel without support for
    PTRACE_SINGLEBLOCK a ptrace call with data=NULL reports an error.
    In the latter case, the test is just continuing with PTRACE_CONT.
    
    ChangeLog:
    
    	* sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c:
    	Support running on kernels without PTRACE_SINGLEBLOCK.

diff --git a/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c b/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c
index 95a2f55..c8eea0a 100644
--- a/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c
+++ b/sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c
@@ -26,6 +26,8 @@
 #include <elf.h>
 #include <support/xunistd.h>
 #include <support/check.h>
+#include <string.h>
+#include <errno.h>
 
 /* Ensure that we use the PTRACE_SINGLEBLOCK definition from glibc ptrace.h
    in tracer_func.  We need the kernel ptrace.h for structs ptrace_area
@@ -63,6 +65,10 @@ tracer_func (int pid)
   gregset_t regs2;
 
   int status;
+  int ret;
+#define MAX_CHARS_IN_BUF 4096
+  char buf[MAX_CHARS_IN_BUF + 1];
+  size_t buf_count;
 
   while (1)
     {
@@ -104,11 +110,55 @@ tracer_func (int pid)
 	 The s390 kernel has no support for PTRACE_GETREGS!
 	 Thus glibc ptrace.h is adjusted to match kernel ptrace.h.
 
+	 The glibc sys/ptrace.h header contains the identifier
+	 PTRACE_SINGLEBLOCK in enum __ptrace_request.  In contrast, the kernel
+	 asm/ptrace.h header defines PTRACE_SINGLEBLOCK.
+
 	 This test ensures, that PTRACE_SINGLEBLOCK defined in glibc
 	 works as expected.  If the kernel would interpret it as
 	 PTRACE_GETREGS, then the tracee will not make any progress
-	 and this testcase will time out.  */
-      TEST_VERIFY_EXIT (ptrace (req_singleblock, pid, NULL, NULL) == 0);
+	 and this testcase will time out or the ptrace call will fail with
+	 different errors.  */
+
+      /* Ptrace request 12 is first done with data argument pointing to
+	 a buffer:
+	 -If request 12 is interpreted as PTRACE_GETREGS, it will store the regs
+	 to buffer without an error.
+
+	 -If request 12 is interpreted as PTRACE_SINGLEBLOCK, it will fail
+	 as data argument is used as signal-number and the address of
+	 buf is no valid signal.
+
+	 -If request 12 is not implemented, it will also fail.
+
+	 Here the test expects that the buffer is untouched and an error is
+	 returned.  */
+      memset (buf, 'a', MAX_CHARS_IN_BUF);
+      ret = ptrace (req_singleblock, pid, NULL, buf);
+      buf [MAX_CHARS_IN_BUF] = '\0';
+      buf_count = strspn (buf, "a");
+      TEST_VERIFY_EXIT (buf_count == MAX_CHARS_IN_BUF);
+      TEST_VERIFY_EXIT (ret == -1);
+
+      /* If request 12 is interpreted as PTRACE_GETREGS, the first ptrace
+	 call will touch the buffer which is detected by this test.  */
+      errno = 0;
+      ret = ptrace (req_singleblock, pid, NULL, NULL);
+      if (ret == 0)
+	{
+	  /* The kernel has support for PTRACE_SINGLEBLOCK ptrace request. */
+	  TEST_VERIFY_EXIT (errno == 0);
+	}
+      else
+	{
+	  /* The kernel (< 3.15) has no support for PTRACE_SINGLEBLOCK ptrace
+	     request. */
+	  TEST_VERIFY_EXIT (errno == EIO);
+	  TEST_VERIFY_EXIT (ret == -1);
+
+	  /* Just continue tracee until it exits normally.  */
+	  TEST_VERIFY_EXIT (ptrace (PTRACE_CONT, pid, NULL, NULL) == 0);
+	}
     }
 }
 

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Stefan Liebler]

On 07/07/2017 03:53 PM, Stefan Liebler wrote:
> On 07/07/2017 12:45 PM, Florian Weimer wrote:
>> On 07/07/2017 12:22 PM, Stefan Liebler wrote:
>>> +      /* Ptrace request 12 is done with zero data argument:
>>> +     -If the kernel has support for PTRACE_SINGLEBLOCK (then the kernel
>>> +     header asm/ptrace.h defines this macro), the ptrace call is not 
>>> allowed
>>> +     to fail and has to continue the tracee until next taken branch.
>>> +
>>> +     -If the kernel (<3.15) has no support for PTRACE_SINGLEBLOCK, the
>>> +     ptrace call has to fail with EIO. Then I continue the tracee with
>>> +     PTRACE_CONT.
>>> +
>>> +     -If the request 12 is interpreted as PTRACE_GETREGS, it will 
>>> fail too.
>>> +     It fails with EFAULT on intel / power as data argument is NULL.
>>> +     According to the man-page: "Unfortunately, under Linux, different
>>> +     variations of this fault will return EIO or EFAULT more or less
>>> +     arbitrarily".
>>> +     But if request 12 is interpreted as PTRACE_GETREGS, the first 
>>> ptrace
>>> +     call will touch the buffer which is detected by this test.  */
>>
>> I think the comment is still a bit off.  I think it is only necessary to
>> retain the second two lines, the other things is already implied by the
>> short comments in the code below.
> okay. I removed the other lines.
>>
>> (I have not tested whether this actually works.  I assume you have
>> checked a couple of userspace/kernel permutations.)
> Yes. I've run the test on several machines.
>>
>> Thanks,
>> Florian
>>
> 
> 
> If this is okay, I'll commit it on Monday.
Committed.
> 
> Bye.
> Stefan

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Dmitry V. Levin]

[-- Attachment #1: Type: text/plain, Size: 6989 bytes --]

On Mon, Jun 19, 2017 at 03:10:57PM +0200, Stefan Liebler wrote:
[...]
> --- a/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
> +++ b/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
> @@ -89,25 +89,9 @@ enum __ptrace_request
>    PTRACE_SINGLESTEP = 9,
>  #define PT_STEP PTRACE_SINGLESTEP
>  
> -  /* Get all general purpose registers used by a processes.
> -     This is not supported on all machines.  */
> -   PTRACE_GETREGS = 12,
> -#define PT_GETREGS PTRACE_GETREGS
> -
> -  /* Set all general purpose registers used by a processes.
> -     This is not supported on all machines.  */
> -   PTRACE_SETREGS = 13,
> -#define PT_SETREGS PTRACE_SETREGS
> -
> -  /* Get all floating point registers used by a processes.
> -     This is not supported on all machines.  */
> -   PTRACE_GETFPREGS = 14,
> -#define PT_GETFPREGS PTRACE_GETFPREGS
> -
> -  /* Set all floating point registers used by a processes.
> -     This is not supported on all machines.  */
> -   PTRACE_SETFPREGS = 15,
> -#define PT_SETFPREGS PTRACE_SETFPREGS
> +  /* Execute process until next taken branch.  */
> +  PTRACE_SINGLEBLOCK = 12,
> +#define PT_STEPBLOCK PTRACE_SINGLEBLOCK
>  
>    /* Attach to a process that is already running. */
>    PTRACE_ATTACH = 16,
> @@ -167,8 +151,26 @@ enum __ptrace_request
>    PTRACE_SETSIGMASK = 0x420b,
>  #define PTRACE_SETSIGMASK PTRACE_SETSIGMASK
>  
> -  PTRACE_SECCOMP_GET_FILTER = 0x420c
> +  PTRACE_SECCOMP_GET_FILTER = 0x420c,
>  #define PTRACE_SECCOMP_GET_FILTER PTRACE_SECCOMP_GET_FILTER
> +
> +  PTRACE_PEEKUSR_AREA = 0x5000,
> +#define PTRACE_PEEKUSR_AREA PTRACE_PEEKUSR_AREA
> +
> +  PTRACE_POKEUSR_AREA = 0x5001,
> +#define PTRACE_POKEUSR_AREA PTRACE_POKEUSR_AREA
> +
> +  PTRACE_GET_LAST_BREAK = 0x5006,
> +#define PTRACE_GET_LAST_BREAK PTRACE_GET_LAST_BREAK
> +
> +  PTRACE_ENABLE_TE = 0x5009,
> +#define PTRACE_ENABLE_TE PTRACE_ENABLE_TE
> +
> +  PTRACE_DISABLE_TE = 0x5010,
> +#define PTRACE_DISABLE_TE PTRACE_DISABLE_TE
> +
> +  PTRACE_TE_ABORT_RAND = 0x5011
> +#define PTRACE_TE_ABORT_RAND PTRACE_TE_ABORT_RAND
>  };

Mark Wielaard has spotted [1] a regression that I missed during review.
After this change, this test case fails to compile with the following
diagnostics:

$ gcc -c -xc -o/dev/null - <<'EOF'
#include <asm/ptrace.h>
#include <sys/ptrace.h>
EOF
In file included from <stdin>:1:0:
/usr/include/sys/ptrace.h:93:3: error: expected identifier before numeric constant
   PTRACE_SINGLEBLOCK = 12,
   ^
In file included from <stdin>:2:0:
/usr/include/sys/ptrace.h:158:0: warning: "PTRACE_PEEKUSR_AREA" redefined
 #define PTRACE_PEEKUSR_AREA PTRACE_PEEKUSR_AREA
 ^
In file included from <stdin>:1:0:
/usr/include/asm/ptrace.h:392:0: note: this is the location of the previous definition
 #define PTRACE_PEEKUSR_AREA           0x5000
 ^
In file included from <stdin>:2:0:
/usr/include/sys/ptrace.h:161:0: warning: "PTRACE_POKEUSR_AREA" redefined
 #define PTRACE_POKEUSR_AREA PTRACE_POKEUSR_AREA
 ^
In file included from <stdin>:1:0:
/usr/include/asm/ptrace.h:393:0: note: this is the location of the previous definition
 #define PTRACE_POKEUSR_AREA           0x5001
 ^
In file included from <stdin>:2:0:
/usr/include/sys/ptrace.h:164:0: warning: "PTRACE_GET_LAST_BREAK" redefined
 #define PTRACE_GET_LAST_BREAK PTRACE_GET_LAST_BREAK
 ^
In file included from <stdin>:1:0:
/usr/include/asm/ptrace.h:398:0: note: this is the location of the previous definition
 #define PTRACE_GET_LAST_BREAK       0x5006
 ^
In file included from <stdin>:2:0:
/usr/include/sys/ptrace.h:167:0: warning: "PTRACE_ENABLE_TE" redefined
 #define PTRACE_ENABLE_TE PTRACE_ENABLE_TE
 ^
In file included from <stdin>:1:0:
/usr/include/asm/ptrace.h:401:0: note: this is the location of the previous definition
 #define PTRACE_ENABLE_TE       0x5009
 ^
In file included from <stdin>:2:0:
/usr/include/sys/ptrace.h:170:0: warning: "PTRACE_DISABLE_TE" redefined
 #define PTRACE_DISABLE_TE PTRACE_DISABLE_TE
 ^
In file included from <stdin>:1:0:
/usr/include/asm/ptrace.h:402:0: note: this is the location of the previous definition
 #define PTRACE_DISABLE_TE       0x5010
 ^
In file included from <stdin>:2:0:
/usr/include/sys/ptrace.h:173:0: warning: "PTRACE_TE_ABORT_RAND" redefined
 #define PTRACE_TE_ABORT_RAND PTRACE_TE_ABORT_RAND
 ^
In file included from <stdin>:1:0:
/usr/include/asm/ptrace.h:403:0: note: this is the location of the previous definition
 #define PTRACE_TE_ABORT_RAND       0x5011
 ^

The following change fixes this and similar compilation issues that arise
when sys/ptrace.h is included after linux/ptrace.h:

--- a/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
+++ b/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
@@ -24,24 +24,61 @@
 #include <bits/types.h>
 
 __BEGIN_DECLS
-#ifdef _LINUX_PTRACE_H
+#if defined _LINUX_PTRACE_H || defined _S390_PTRACE_H
 /* Kludge to stop stuff gdb & strace compiles from getting upset
  */
 #undef PTRACE_TRACEME
 #undef PTRACE_PEEKTEXT
 #undef PTRACE_PEEKDATA
-#undef PTRACE_PEEKUSR
 #undef PTRACE_POKETEXT
 #undef PTRACE_POKEDATA
-#undef PTRACE_POKEUSR
 #undef PTRACE_CONT
 #undef PTRACE_KILL
 #undef PTRACE_SINGLESTEP
-
+#undef PTRACE_SINGLEBLOCK
 #undef PTRACE_ATTACH
 #undef PTRACE_DETACH
-
 #undef PTRACE_SYSCALL
+#undef PTRACE_SETOPTIONS
+#undef PTRACE_GETEVENTMSG
+#undef PTRACE_GETSIGINFO
+#undef PTRACE_SETSIGINFO
+#undef PTRACE_GETREGSET
+#undef PTRACE_SETREGSET
+#undef PTRACE_SEIZE
+#undef PTRACE_INTERRUPT
+#undef PTRACE_LISTEN
+#undef PTRACE_PEEKSIGINFO
+#undef PTRACE_GETSIGMASK
+#undef PTRACE_SETSIGMASK
+#undef PTRACE_SECCOMP_GET_FILTER
+#undef PTRACE_PEEKUSR_AREA
+#undef PTRACE_POKEUSR_AREA
+#undef PTRACE_GET_LAST_BREAK
+#undef PTRACE_ENABLE_TE
+#undef PTRACE_DISABLE_TE
+#undef PTRACE_TE_ABORT_RAND
+#undef PTRACE_O_TRACESYSGOOD
+#undef PTRACE_O_TRACEFORK
+#undef PTRACE_O_TRACEVFORK
+#undef PTRACE_O_TRACECLONE
+#undef PTRACE_O_TRACEEXEC
+#undef PTRACE_O_TRACEVFORKDONE
+#undef PTRACE_O_TRACEEXIT
+#undef PTRACE_O_TRACESECCOMP
+#undef PTRACE_O_EXITKILL
+#undef PTRACE_O_SUSPEND_SECCOMP
+#undef PTRACE_O_MASK
+#undef PTRACE_EVENT_FORK
+#undef PTRACE_EVENT_VFORK
+#undef PTRACE_EVENT_CLONE
+#undef PTRACE_EVENT_EXEC
+#undef PTRACE_EVENT_VFORK_DONE
+#undef PTRACE_EVENT_EXIT
+#undef PTRACE_EVENT_SECCOMP
+#undef PTRACE_EVENT_STOP
+#undef PTRACE_PEEKSIGINFO_SHARED
+
 #endif
 /* Type of the REQUEST argument to `ptrace.'  */
 enum __ptrace_request

The list was produced by the following command:
$ sed -n '/USER\|DEVEL/d;s/^[[:space:]]*\(PTRACE_[^=[:space:]]\+\)[[:space:]]*=.*/#undef \1/p' sysdeps/unix/sysv/linux/s390/sys/ptrace.h

PTRACE_PEEKUSER and PTRACE_POKEUSER were excluded because they are not
defined by Linux headers, and PTRACE_SEIZE_DEVEL was excluded because
it's obsolete and should be removed from sys/ptrace.h anyway.

[1] https://sourceware.org/ml/elfutils-devel/2017-q3/msg00018.html


-- 
ldv

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Carlos O'Donell]

On 07/18/2017 06:20 AM, Dmitry V. Levin wrote:
> Mark Wielaard has spotted [1] a regression that I missed during review.
> After this change, this test case fails to compile with the following
> diagnostics:
> 
> $ gcc -c -xc -o/dev/null - <<'EOF'
> #include <asm/ptrace.h>
> #include <sys/ptrace.h>
> EOF

This is a linux/glibc header coordination issue.

https://sourceware.org/glibc/wiki/Synchronizing_Headers

> The following change fixes this and similar compilation issues that arise
> when sys/ptrace.h is included after linux/ptrace.h:
 
This is a known conflict, and needs to be fixed properly using libc-compat.h
on the kernel side and the appropriate defines on the glibc side.

-- 
Cheers,
Carlos.

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Dmitry V. Levin]

[-- Attachment #1: Type: text/plain, Size: 1072 bytes --]

On Tue, Jul 18, 2017 at 09:31:49AM -0400, Carlos O'Donell wrote:
> On 07/18/2017 06:20 AM, Dmitry V. Levin wrote:
> > Mark Wielaard has spotted [1] a regression that I missed during review.
> > After this change, this test case fails to compile with the following
> > diagnostics:
> > 
> > $ gcc -c -xc -o/dev/null - <<'EOF'
> > #include <asm/ptrace.h>
> > #include <sys/ptrace.h>
> > EOF
> 
> This is a linux/glibc header coordination issue.

Not really, this is a new issue since glibc-2.25.

> https://sourceware.org/glibc/wiki/Synchronizing_Headers

This project doesn't appear to be alive, unfortunately.

> > The following change fixes this and similar compilation issues that arise
> > when sys/ptrace.h is included after linux/ptrace.h:
>  
> This is a known conflict, and needs to be fixed properly using libc-compat.h
> on the kernel side and the appropriate defines on the glibc side.

No, there was no conflict between asm/ptrace.h and sys/ptrace.h on s390
in glibc-2.25, and we should avoid introducing new conflicts.


-- 
ldv

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Stefan Liebler]

On 07/18/2017 12:20 PM, Dmitry V. Levin wrote:
> On Mon, Jun 19, 2017 at 03:10:57PM +0200, Stefan Liebler wrote:
> [...]
>> --- a/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
>> +++ b/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
>> @@ -89,25 +89,9 @@ enum __ptrace_request
>>     PTRACE_SINGLESTEP = 9,
>>   #define PT_STEP PTRACE_SINGLESTEP
>>   
>> -  /* Get all general purpose registers used by a processes.
>> -     This is not supported on all machines.  */
>> -   PTRACE_GETREGS = 12,
>> -#define PT_GETREGS PTRACE_GETREGS
>> -
>> -  /* Set all general purpose registers used by a processes.
>> -     This is not supported on all machines.  */
>> -   PTRACE_SETREGS = 13,
>> -#define PT_SETREGS PTRACE_SETREGS
>> -
>> -  /* Get all floating point registers used by a processes.
>> -     This is not supported on all machines.  */
>> -   PTRACE_GETFPREGS = 14,
>> -#define PT_GETFPREGS PTRACE_GETFPREGS
>> -
>> -  /* Set all floating point registers used by a processes.
>> -     This is not supported on all machines.  */
>> -   PTRACE_SETFPREGS = 15,
>> -#define PT_SETFPREGS PTRACE_SETFPREGS
>> +  /* Execute process until next taken branch.  */
>> +  PTRACE_SINGLEBLOCK = 12,
>> +#define PT_STEPBLOCK PTRACE_SINGLEBLOCK
>>   
>>     /* Attach to a process that is already running. */
>>     PTRACE_ATTACH = 16,
>> @@ -167,8 +151,26 @@ enum __ptrace_request
>>     PTRACE_SETSIGMASK = 0x420b,
>>   #define PTRACE_SETSIGMASK PTRACE_SETSIGMASK
>>   
>> -  PTRACE_SECCOMP_GET_FILTER = 0x420c
>> +  PTRACE_SECCOMP_GET_FILTER = 0x420c,
>>   #define PTRACE_SECCOMP_GET_FILTER PTRACE_SECCOMP_GET_FILTER
>> +
>> +  PTRACE_PEEKUSR_AREA = 0x5000,
>> +#define PTRACE_PEEKUSR_AREA PTRACE_PEEKUSR_AREA
>> +
>> +  PTRACE_POKEUSR_AREA = 0x5001,
>> +#define PTRACE_POKEUSR_AREA PTRACE_POKEUSR_AREA
>> +
>> +  PTRACE_GET_LAST_BREAK = 0x5006,
>> +#define PTRACE_GET_LAST_BREAK PTRACE_GET_LAST_BREAK
>> +
>> +  PTRACE_ENABLE_TE = 0x5009,
>> +#define PTRACE_ENABLE_TE PTRACE_ENABLE_TE
>> +
>> +  PTRACE_DISABLE_TE = 0x5010,
>> +#define PTRACE_DISABLE_TE PTRACE_DISABLE_TE
>> +
>> +  PTRACE_TE_ABORT_RAND = 0x5011
>> +#define PTRACE_TE_ABORT_RAND PTRACE_TE_ABORT_RAND
>>   };
> 
> Mark Wielaard has spotted [1] a regression that I missed during review.
> After this change, this test case fails to compile with the following
> diagnostics:
> 
> $ gcc -c -xc -o/dev/null - <<'EOF'
> #include <asm/ptrace.h>
> #include <sys/ptrace.h>
> EOF
Is this order required?
I've tried it on x86_64 RHEL 7.3:
In file included from /usr/include/asm/ptrace.h:5:0,
                  from <stdin>:1:
/usr/include/sys/ptrace.h:74:4: error: expected identifier before 
numeric constant
     PTRACE_GETREGS = 12,
     ^

> In file included from <stdin>:1:0:
> /usr/include/sys/ptrace.h:93:3: error: expected identifier before numeric constant
>     PTRACE_SINGLEBLOCK = 12,
>     ^
> In file included from <stdin>:2:0:
> /usr/include/sys/ptrace.h:158:0: warning: "PTRACE_PEEKUSR_AREA" redefined
>   #define PTRACE_PEEKUSR_AREA PTRACE_PEEKUSR_AREA
>   ^
> In file included from <stdin>:1:0:
> /usr/include/asm/ptrace.h:392:0: note: this is the location of the previous definition
>   #define PTRACE_PEEKUSR_AREA           0x5000
>   ^
> In file included from <stdin>:2:0:
> /usr/include/sys/ptrace.h:161:0: warning: "PTRACE_POKEUSR_AREA" redefined
>   #define PTRACE_POKEUSR_AREA PTRACE_POKEUSR_AREA
>   ^
> In file included from <stdin>:1:0:
> /usr/include/asm/ptrace.h:393:0: note: this is the location of the previous definition
>   #define PTRACE_POKEUSR_AREA           0x5001
>   ^
> In file included from <stdin>:2:0:
> /usr/include/sys/ptrace.h:164:0: warning: "PTRACE_GET_LAST_BREAK" redefined
>   #define PTRACE_GET_LAST_BREAK PTRACE_GET_LAST_BREAK
>   ^
> In file included from <stdin>:1:0:
> /usr/include/asm/ptrace.h:398:0: note: this is the location of the previous definition
>   #define PTRACE_GET_LAST_BREAK       0x5006
>   ^
> In file included from <stdin>:2:0:
> /usr/include/sys/ptrace.h:167:0: warning: "PTRACE_ENABLE_TE" redefined
>   #define PTRACE_ENABLE_TE PTRACE_ENABLE_TE
>   ^
> In file included from <stdin>:1:0:
> /usr/include/asm/ptrace.h:401:0: note: this is the location of the previous definition
>   #define PTRACE_ENABLE_TE       0x5009
>   ^
> In file included from <stdin>:2:0:
> /usr/include/sys/ptrace.h:170:0: warning: "PTRACE_DISABLE_TE" redefined
>   #define PTRACE_DISABLE_TE PTRACE_DISABLE_TE
>   ^
> In file included from <stdin>:1:0:
> /usr/include/asm/ptrace.h:402:0: note: this is the location of the previous definition
>   #define PTRACE_DISABLE_TE       0x5010
>   ^
> In file included from <stdin>:2:0:
> /usr/include/sys/ptrace.h:173:0: warning: "PTRACE_TE_ABORT_RAND" redefined
>   #define PTRACE_TE_ABORT_RAND PTRACE_TE_ABORT_RAND
>   ^
> In file included from <stdin>:1:0:
> /usr/include/asm/ptrace.h:403:0: note: this is the location of the previous definition
>   #define PTRACE_TE_ABORT_RAND       0x5011
>   ^
> 
> The following change fixes this and similar compilation issues that arise
> when sys/ptrace.h is included after linux/ptrace.h:
> 
> --- a/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
> +++ b/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
> @@ -24,24 +24,61 @@
>   #include <bits/types.h>
>   
>   __BEGIN_DECLS
> -#ifdef _LINUX_PTRACE_H
> +#if defined _LINUX_PTRACE_H || defined _S390_PTRACE_H
Shall we also add "|| defined _UAPI_LINUX_PTRACE_H" here?
Some of the defines like PTRACE_EVENT_* | PTRACE_O_* are defined in the 
corresponding <kernel-src>/include/uapi/linux/ptrace.h header.

>   /* Kludge to stop stuff gdb & strace compiles from getting upset
>    */
>   #undef PTRACE_TRACEME
>   #undef PTRACE_PEEKTEXT
>   #undef PTRACE_PEEKDATA
> -#undef PTRACE_PEEKUSR
>   #undef PTRACE_POKETEXT
>   #undef PTRACE_POKEDATA
> -#undef PTRACE_POKEUSR
>   #undef PTRACE_CONT
>   #undef PTRACE_KILL
>   #undef PTRACE_SINGLESTEP
> -
> +#undef PTRACE_SINGLEBLOCK
>   #undef PTRACE_ATTACH
>   #undef PTRACE_DETACH
> -
>   #undef PTRACE_SYSCALL
> +#undef PTRACE_SETOPTIONS
> +#undef PTRACE_GETEVENTMSG
> +#undef PTRACE_GETSIGINFO
> +#undef PTRACE_SETSIGINFO
> +#undef PTRACE_GETREGSET
> +#undef PTRACE_SETREGSET
> +#undef PTRACE_SEIZE
> +#undef PTRACE_INTERRUPT
> +#undef PTRACE_LISTEN
> +#undef PTRACE_PEEKSIGINFO
> +#undef PTRACE_GETSIGMASK
> +#undef PTRACE_SETSIGMASK
> +#undef PTRACE_SECCOMP_GET_FILTER
> +#undef PTRACE_PEEKUSR_AREA
> +#undef PTRACE_POKEUSR_AREA
> +#undef PTRACE_GET_LAST_BREAK
> +#undef PTRACE_ENABLE_TE
> +#undef PTRACE_DISABLE_TE
> +#undef PTRACE_TE_ABORT_RAND
> +#undef PTRACE_O_TRACESYSGOOD
> +#undef PTRACE_O_TRACEFORK
> +#undef PTRACE_O_TRACEVFORK
> +#undef PTRACE_O_TRACECLONE
> +#undef PTRACE_O_TRACEEXEC
> +#undef PTRACE_O_TRACEVFORKDONE
> +#undef PTRACE_O_TRACEEXIT
> +#undef PTRACE_O_TRACESECCOMP
> +#undef PTRACE_O_EXITKILL
> +#undef PTRACE_O_SUSPEND_SECCOMP
> +#undef PTRACE_O_MASK
> +#undef PTRACE_EVENT_FORK
> +#undef PTRACE_EVENT_VFORK
> +#undef PTRACE_EVENT_CLONE
> +#undef PTRACE_EVENT_EXEC
> +#undef PTRACE_EVENT_VFORK_DONE
> +#undef PTRACE_EVENT_EXIT
> +#undef PTRACE_EVENT_SECCOMP
> +#undef PTRACE_EVENT_STOP
> +#undef PTRACE_PEEKSIGINFO_SHARED
> +
>   #endif
>   /* Type of the REQUEST argument to `ptrace.'  */
>   enum __ptrace_request
> 
> The list was produced by the following command:
> $ sed -n '/USER\|DEVEL/d;s/^[[:space:]]*\(PTRACE_[^=[:space:]]\+\)[[:space:]]*=.*/#undef \1/p' sysdeps/unix/sysv/linux/s390/sys/ptrace.h
> 
> PTRACE_PEEKUSER and PTRACE_POKEUSER were excluded because they are not
> defined by Linux headers, and PTRACE_SEIZE_DEVEL was excluded because
> it's obsolete and should be removed from sys/ptrace.h anyway.
> 
> [1] https://sourceware.org/ml/elfutils-devel/2017-q3/msg00018.html
> 
> 

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Carlos O'Donell]

On 07/18/2017 09:39 AM, Dmitry V. Levin wrote:
> On Tue, Jul 18, 2017 at 09:31:49AM -0400, Carlos O'Donell wrote:
>> On 07/18/2017 06:20 AM, Dmitry V. Levin wrote:
>>> Mark Wielaard has spotted [1] a regression that I missed during review.
>>> After this change, this test case fails to compile with the following
>>> diagnostics:
>>>
>>> $ gcc -c -xc -o/dev/null - <<'EOF'
>>> #include <asm/ptrace.h>
>>> #include <sys/ptrace.h>
>>> EOF
>>
>> This is a linux/glibc header coordination issue.
> 
> Not really, this is a new issue since glibc-2.25.

It is both a new issue and a header coordination issue, the two are not
mutually exclusive.

>> https://sourceware.org/glibc/wiki/Synchronizing_Headers
> 
> This project doesn't appear to be alive, unfortunately.

The project is alive. We are the project. We need to send patches upstream
to the Linux kernel to keep fixing inclusion ordering issues where we need
them fixed.

When 2.27 opens I have a pile of header inclusion ordering fixes I want to
propose along with tests for them, but I haven't had a chance to submit. So
we can discuss this in a few weeks.

>>> The following change fixes this and similar compilation issues that arise
>>> when sys/ptrace.h is included after linux/ptrace.h:
>>  
>> This is a known conflict, and needs to be fixed properly using libc-compat.h
>> on the kernel side and the appropriate defines on the glibc side.
> 
> No, there was no conflict between asm/ptrace.h and sys/ptrace.h on s390
> in glibc-2.25, and we should avoid introducing new conflicts.
 
I have not verified that inclusion worked on both orders, if it did, then
this is indeed a regression.

However, I would like to take a step back:

* Why is this issue a blocker? What software does it stop from building?

* Can we delay the fix until after the release and fix it properly?

Mark, Is this a problem for Valgrind?

-- 
Cheers,
Carlos.

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Carlos O'Donell]

On 07/18/2017 09:41 AM, Stefan Liebler wrote:
>> $ gcc -c -xc -o/dev/null - <<'EOF'
>> #include <asm/ptrace.h>
>> #include <sys/ptrace.h>
>> EOF
> Is this order required?

Per the synchronization headers strategy for glibc we try to support *both*
orders for important headers where possible.

https://sourceware.org/glibc/wiki/Synchronizing_Headers

> I've tried it on x86_64 RHEL 7.3:
> In file included from /usr/include/asm/ptrace.h:5:0,
>                  from <stdin>:1:
> /usr/include/sys/ptrace.h:74:4: error: expected identifier before numeric constant
>     PTRACE_GETREGS = 12,
>     ^

This is a known issue. It is known that linux/ptrace.h and sys/ptrace.h do not really
work well together. We need to fix it.

-- 
Cheers,
Carlos.

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Dmitry V. Levin]

[-- Attachment #1: Type: text/plain, Size: 3941 bytes --]

On Tue, Jul 18, 2017 at 03:41:41PM +0200, Stefan Liebler wrote:
> On 07/18/2017 12:20 PM, Dmitry V. Levin wrote:
> > On Mon, Jun 19, 2017 at 03:10:57PM +0200, Stefan Liebler wrote:
[...]
> > Mark Wielaard has spotted [1] a regression that I missed during review.
> > After this change, this test case fails to compile with the following
> > diagnostics:
> > 
> > $ gcc -c -xc -o/dev/null - <<'EOF'
> > #include <asm/ptrace.h>
> > #include <sys/ptrace.h>
> > EOF
> Is this order required?

No, the reversed order also works.

> I've tried it on x86_64 RHEL 7.3:
> In file included from /usr/include/asm/ptrace.h:5:0,
>                   from <stdin>:1:
> /usr/include/sys/ptrace.h:74:4: error: expected identifier before 
> numeric constant
>      PTRACE_GETREGS = 12,
>      ^

Interesting.  So it means that it didn't work with some old linux headers.
However, it used to work with recent linux headers and glibc-2.25.

> > In file included from <stdin>:1:0:
> > /usr/include/sys/ptrace.h:93:3: error: expected identifier before numeric constant
> >     PTRACE_SINGLEBLOCK = 12,
> >     ^
> > In file included from <stdin>:2:0:
> > /usr/include/sys/ptrace.h:158:0: warning: "PTRACE_PEEKUSR_AREA" redefined
> >   #define PTRACE_PEEKUSR_AREA PTRACE_PEEKUSR_AREA
> >   ^
> > In file included from <stdin>:1:0:
> > /usr/include/asm/ptrace.h:392:0: note: this is the location of the previous definition
> >   #define PTRACE_PEEKUSR_AREA           0x5000
> >   ^
> > In file included from <stdin>:2:0:
> > /usr/include/sys/ptrace.h:161:0: warning: "PTRACE_POKEUSR_AREA" redefined
> >   #define PTRACE_POKEUSR_AREA PTRACE_POKEUSR_AREA
> >   ^
> > In file included from <stdin>:1:0:
> > /usr/include/asm/ptrace.h:393:0: note: this is the location of the previous definition
> >   #define PTRACE_POKEUSR_AREA           0x5001
> >   ^
> > In file included from <stdin>:2:0:
> > /usr/include/sys/ptrace.h:164:0: warning: "PTRACE_GET_LAST_BREAK" redefined
> >   #define PTRACE_GET_LAST_BREAK PTRACE_GET_LAST_BREAK
> >   ^
> > In file included from <stdin>:1:0:
> > /usr/include/asm/ptrace.h:398:0: note: this is the location of the previous definition
> >   #define PTRACE_GET_LAST_BREAK       0x5006
> >   ^
> > In file included from <stdin>:2:0:
> > /usr/include/sys/ptrace.h:167:0: warning: "PTRACE_ENABLE_TE" redefined
> >   #define PTRACE_ENABLE_TE PTRACE_ENABLE_TE
> >   ^
> > In file included from <stdin>:1:0:
> > /usr/include/asm/ptrace.h:401:0: note: this is the location of the previous definition
> >   #define PTRACE_ENABLE_TE       0x5009
> >   ^
> > In file included from <stdin>:2:0:
> > /usr/include/sys/ptrace.h:170:0: warning: "PTRACE_DISABLE_TE" redefined
> >   #define PTRACE_DISABLE_TE PTRACE_DISABLE_TE
> >   ^
> > In file included from <stdin>:1:0:
> > /usr/include/asm/ptrace.h:402:0: note: this is the location of the previous definition
> >   #define PTRACE_DISABLE_TE       0x5010
> >   ^
> > In file included from <stdin>:2:0:
> > /usr/include/sys/ptrace.h:173:0: warning: "PTRACE_TE_ABORT_RAND" redefined
> >   #define PTRACE_TE_ABORT_RAND PTRACE_TE_ABORT_RAND
> >   ^
> > In file included from <stdin>:1:0:
> > /usr/include/asm/ptrace.h:403:0: note: this is the location of the previous definition
> >   #define PTRACE_TE_ABORT_RAND       0x5011
> >   ^
> > 
> > The following change fixes this and similar compilation issues that arise
> > when sys/ptrace.h is included after linux/ptrace.h:
> > 
> > --- a/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
> > +++ b/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
> > @@ -24,24 +24,61 @@
> >   #include <bits/types.h>
> >   
> >   __BEGIN_DECLS
> > -#ifdef _LINUX_PTRACE_H
> > +#if defined _LINUX_PTRACE_H || defined _S390_PTRACE_H
> Shall we also add "|| defined _UAPI_LINUX_PTRACE_H" here?

Isn't _UAPI prefix automatically stripped during linux headers export
procedure?


-- 
ldv

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Mark Wielaard]

On Tue, 2017-07-18 at 10:11 -0400, Carlos O'Donell wrote:
> On 07/18/2017 09:39 AM, Dmitry V. Levin wrote:
> > On Tue, Jul 18, 2017 at 09:31:49AM -0400, Carlos O'Donell wrote:
> >> On 07/18/2017 06:20 AM, Dmitry V. Levin wrote:
> >>> The following change fixes this and similar compilation issues that arise
> >>> when sys/ptrace.h is included after linux/ptrace.h:
> >>  
> >> This is a known conflict, and needs to be fixed properly using libc-compat.h
> >> on the kernel side and the appropriate defines on the glibc side.
> > 
> > No, there was no conflict between asm/ptrace.h and sys/ptrace.h on s390
> > in glibc-2.25, and we should avoid introducing new conflicts.
>  
> I have not verified that inclusion worked on both orders, if it did, then
> this is indeed a regression.

It used to work either way. We used asm/ptrace.h then sys/ptrace.h on
s390[x] in elfutils. That broke with 2.26 in fedora rawhide. So we
changed it to sys/ptrace.h then asm/ptrace.h. I verified that order
works on both old and new glibc/kernel headers (aka on RHEL7 s390[x] and
fedora rawhide s390x).

> Mark, Is this a problem for Valgrind?

It really shouldn't, but I haven't tried building valgrind on s390x
against glibc git yet. In valgrind we try to avoid including glibc and
kernel headers (valgrind comes with its own mini-libc that we statically
link into the tools and its own kernel headers for the syscall
wrappers).

Cheers,

Mark

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Mark Wielaard]

On Tue, 2017-07-18 at 16:28 +0200, Mark Wielaard wrote:
> On Tue, 2017-07-18 at 10:11 -0400, Carlos O'Donell wrote:
> > On 07/18/2017 09:39 AM, Dmitry V. Levin wrote:
> > > On Tue, Jul 18, 2017 at 09:31:49AM -0400, Carlos O'Donell wrote:
> > >> On 07/18/2017 06:20 AM, Dmitry V. Levin wrote:
> > >>> The following change fixes this and similar compilation issues that arise
> > >>> when sys/ptrace.h is included after linux/ptrace.h:
> > >>  
> > >> This is a known conflict, and needs to be fixed properly using libc-compat.h
> > >> on the kernel side and the appropriate defines on the glibc side.
> > > 
> > > No, there was no conflict between asm/ptrace.h and sys/ptrace.h on s390
> > > in glibc-2.25, and we should avoid introducing new conflicts.
> >  
> > I have not verified that inclusion worked on both orders, if it did, then
> > this is indeed a regression.
> 
> It used to work either way. We used asm/ptrace.h then sys/ptrace.h on
> s390[x] in elfutils. That broke with 2.26 in fedora rawhide. So we
> changed it to sys/ptrace.h then asm/ptrace.h. I verified that order
> works on both old and new glibc/kernel headers (aka on RHEL7 s390[x] and
> fedora rawhide s390x).

One more note. This really is only an issue for s390x since asm/ptrace.h
provides ptrace_area which is needed to use PTRACE_PEEKUSR_AREA. On
other architectures you only need sys/ptrace.h so it doesn't really
matter whether you can include them both.

See also the glibc testcase
sysdeps/unix/sysv/linux/s390/tst-ptrace-singleblock.c which also needs
to include both.

Cheers,

Mark

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Stefan Liebler]

On 07/18/2017 04:15 PM, Dmitry V. Levin wrote:
> On Tue, Jul 18, 2017 at 03:41:41PM +0200, Stefan Liebler wrote:
>> On 07/18/2017 12:20 PM, Dmitry V. Levin wrote:
>>> On Mon, Jun 19, 2017 at 03:10:57PM +0200, Stefan Liebler wrote:
> [...]
>>> Mark Wielaard has spotted [1] a regression that I missed during review.
>>> After this change, this test case fails to compile with the following
>>> diagnostics:
>>>
>>> $ gcc -c -xc -o/dev/null - <<'EOF'
>>> #include <asm/ptrace.h>
>>> #include <sys/ptrace.h>
>>> EOF
>> Is this order required?
> 
> No, the reversed order also works.
> 
>> I've tried it on x86_64 RHEL 7.3:
>> In file included from /usr/include/asm/ptrace.h:5:0,
>>                    from <stdin>:1:
>> /usr/include/sys/ptrace.h:74:4: error: expected identifier before
>> numeric constant
>>       PTRACE_GETREGS = 12,
>>       ^
> 
> Interesting.  So it means that it didn't work with some old linux headers.
> However, it used to work with recent linux headers and glibc-2.25.
> 
>>> In file included from <stdin>:1:0:
>>> /usr/include/sys/ptrace.h:93:3: error: expected identifier before numeric constant
>>>      PTRACE_SINGLEBLOCK = 12,
>>>      ^
>>> In file included from <stdin>:2:0:
>>> /usr/include/sys/ptrace.h:158:0: warning: "PTRACE_PEEKUSR_AREA" redefined
>>>    #define PTRACE_PEEKUSR_AREA PTRACE_PEEKUSR_AREA
>>>    ^
>>> In file included from <stdin>:1:0:
>>> /usr/include/asm/ptrace.h:392:0: note: this is the location of the previous definition
>>>    #define PTRACE_PEEKUSR_AREA           0x5000
>>>    ^
>>> In file included from <stdin>:2:0:
>>> /usr/include/sys/ptrace.h:161:0: warning: "PTRACE_POKEUSR_AREA" redefined
>>>    #define PTRACE_POKEUSR_AREA PTRACE_POKEUSR_AREA
>>>    ^
>>> In file included from <stdin>:1:0:
>>> /usr/include/asm/ptrace.h:393:0: note: this is the location of the previous definition
>>>    #define PTRACE_POKEUSR_AREA           0x5001
>>>    ^
>>> In file included from <stdin>:2:0:
>>> /usr/include/sys/ptrace.h:164:0: warning: "PTRACE_GET_LAST_BREAK" redefined
>>>    #define PTRACE_GET_LAST_BREAK PTRACE_GET_LAST_BREAK
>>>    ^
>>> In file included from <stdin>:1:0:
>>> /usr/include/asm/ptrace.h:398:0: note: this is the location of the previous definition
>>>    #define PTRACE_GET_LAST_BREAK       0x5006
>>>    ^
>>> In file included from <stdin>:2:0:
>>> /usr/include/sys/ptrace.h:167:0: warning: "PTRACE_ENABLE_TE" redefined
>>>    #define PTRACE_ENABLE_TE PTRACE_ENABLE_TE
>>>    ^
>>> In file included from <stdin>:1:0:
>>> /usr/include/asm/ptrace.h:401:0: note: this is the location of the previous definition
>>>    #define PTRACE_ENABLE_TE       0x5009
>>>    ^
>>> In file included from <stdin>:2:0:
>>> /usr/include/sys/ptrace.h:170:0: warning: "PTRACE_DISABLE_TE" redefined
>>>    #define PTRACE_DISABLE_TE PTRACE_DISABLE_TE
>>>    ^
>>> In file included from <stdin>:1:0:
>>> /usr/include/asm/ptrace.h:402:0: note: this is the location of the previous definition
>>>    #define PTRACE_DISABLE_TE       0x5010
>>>    ^
>>> In file included from <stdin>:2:0:
>>> /usr/include/sys/ptrace.h:173:0: warning: "PTRACE_TE_ABORT_RAND" redefined
>>>    #define PTRACE_TE_ABORT_RAND PTRACE_TE_ABORT_RAND
>>>    ^
>>> In file included from <stdin>:1:0:
>>> /usr/include/asm/ptrace.h:403:0: note: this is the location of the previous definition
>>>    #define PTRACE_TE_ABORT_RAND       0x5011
>>>    ^
>>>
>>> The following change fixes this and similar compilation issues that arise
>>> when sys/ptrace.h is included after linux/ptrace.h:
>>>
>>> --- a/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
>>> +++ b/sysdeps/unix/sysv/linux/s390/sys/ptrace.h
>>> @@ -24,24 +24,61 @@
>>>    #include <bits/types.h>
>>>    
>>>    __BEGIN_DECLS
>>> -#ifdef _LINUX_PTRACE_H
>>> +#if defined _LINUX_PTRACE_H || defined _S390_PTRACE_H
>> Shall we also add "|| defined _UAPI_LINUX_PTRACE_H" here?
> 
> Isn't _UAPI prefix automatically stripped during linux headers export
> procedure?
> 
> 
Yes you are right. I've viewed the file in the kernel-sources and missed 
this fact.

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Stefan Liebler]

On 07/18/2017 04:11 PM, Carlos O'Donell wrote:
> On 07/18/2017 09:39 AM, Dmitry V. Levin wrote:
>> On Tue, Jul 18, 2017 at 09:31:49AM -0400, Carlos O'Donell wrote:
>>> On 07/18/2017 06:20 AM, Dmitry V. Levin wrote:
>>>> Mark Wielaard has spotted [1] a regression that I missed during review.
>>>> After this change, this test case fails to compile with the following
>>>> diagnostics:
>>>>
>>>> $ gcc -c -xc -o/dev/null - <<'EOF'
>>>> #include <asm/ptrace.h>
>>>> #include <sys/ptrace.h>
>>>> EOF
>>>
>>> This is a linux/glibc header coordination issue.
>>
>> Not really, this is a new issue since glibc-2.25.
> 
> It is both a new issue and a header coordination issue, the two are not
> mutually exclusive.
> 
>>> https://sourceware.org/glibc/wiki/Synchronizing_Headers
>>
>> This project doesn't appear to be alive, unfortunately.
> 
> The project is alive. We are the project. We need to send patches upstream
> to the Linux kernel to keep fixing inclusion ordering issues where we need
> them fixed.
> 
> When 2.27 opens I have a pile of header inclusion ordering fixes I want to
> propose along with tests for them, but I haven't had a chance to submit. So
> we can discuss this in a few weeks.
> 
>>>> The following change fixes this and similar compilation issues that arise
>>>> when sys/ptrace.h is included after linux/ptrace.h:
>>>   
>>> This is a known conflict, and needs to be fixed properly using libc-compat.h
>>> on the kernel side and the appropriate defines on the glibc side.
>>
>> No, there was no conflict between asm/ptrace.h and sys/ptrace.h on s390
>> in glibc-2.25, and we should avoid introducing new conflicts.
> 
> I have not verified that inclusion worked on both orders, if it did, then
> this is indeed a regression.
Before my commit "S390: Sync ptrace.h with kernel. [BZ #21539]",
both orders compiled without a failure:

#include <asm/ptrace.h>
#include <sys/ptrace.h>
=> okay (but fails after my commit)


#include <sys/ptrace.h>
#include <asm/ptrace.h>
=> okay


However if somebody used linux/ptrace.h instead of asm/ptrace.h:

#include <linux/ptrace.h>
#include <sys/ptrace.h>
=> fail


#include <sys/ptrace.h>
#include <linux/ptrace.h>
=> okay


With Dimitry's patch, all four cases are okay.

> 
> However, I would like to take a step back:
> 
> * Why is this issue a blocker? What software does it stop from building?
> 
> * Can we delay the fix until after the release and fix it properly?
> 
> Mark, Is this a problem for Valgrind?
> 

Carlos, shall we commit Dimitry's patch before the release?
Then we don't have this regression compared to glibc 2.25 release.

Bye.
Stefan

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Carlos O'Donell]

On 07/20/2017 03:38 AM, Stefan Liebler wrote:
> Carlos, shall we commit Dimitry's patch before the release?
> Then we don't have this regression compared to glibc 2.25 release.

I'm OK with Dmitry's patch going in for 2.26 to fix the regression.

Could you please work with Dmitry to get this committed?

I'll work on fixing this coordination issue with the kernel when we
reopen for development.

-- 
Cheers,
Carlos.

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Stefan Liebler]

On 07/20/2017 10:06 AM, Carlos O'Donell wrote:
> On 07/20/2017 03:38 AM, Stefan Liebler wrote:
>> Carlos, shall we commit Dimitry's patch before the release?
>> Then we don't have this regression compared to glibc 2.25 release.
> 
> I'm OK with Dmitry's patch going in for 2.26 to fix the regression.
Okay.
> > Could you please work with Dmitry to get this committed?
Okay.
Dimitry, I have one small note to your patch:
Can you add a space before the undefs and add the comment like this:
(according to 
https://sourceware.org/glibc/wiki/Style_and_Conventions#Nested_C_Preprocessor_Directives)

#if defined _LINUX_PTRACE_H || defined _S390_PTRACE_H
/* Kludge to stop stuff gdb & strace compiles from getting upset
  */
# undef PTRACE_TRACEME
...
#endif /* defined _LINUX_PTRACE_H || defined _S390_PTRACE_H  */

Then the patch is okay.
Can you please commit the patch?
Otherwise, post the final patch, then I'll commit it.

> 
> I'll work on fixing this coordination issue with the kernel when we
> reopen for development.
> 
Okay.


Thanks.
Stefan

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Dmitry V. Levin]

[-- Attachment #1: Type: text/plain, Size: 1034 bytes --]

On Thu, Jul 20, 2017 at 10:32:22AM +0200, Stefan Liebler wrote:
> On 07/20/2017 10:06 AM, Carlos O'Donell wrote:
> > On 07/20/2017 03:38 AM, Stefan Liebler wrote:
> >> Carlos, shall we commit Dimitry's patch before the release?
> >> Then we don't have this regression compared to glibc 2.25 release.
> > 
> > I'm OK with Dmitry's patch going in for 2.26 to fix the regression.
> Okay.
> > > Could you please work with Dmitry to get this committed?
> Okay.
> Dimitry, I have one small note to your patch:
> Can you add a space before the undefs and add the comment like this:
> (according to 
> https://sourceware.org/glibc/wiki/Style_and_Conventions#Nested_C_Preprocessor_Directives)
> 
> #if defined _LINUX_PTRACE_H || defined _S390_PTRACE_H
> /* Kludge to stop stuff gdb & strace compiles from getting upset
>   */
> # undef PTRACE_TRACEME
> ...
> #endif /* defined _LINUX_PTRACE_H || defined _S390_PTRACE_H  */
> 
> Then the patch is okay.
> Can you please commit the patch?

OK, committed.


-- 
ldv

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH] S390: Sync ptrace.h with kernel. [BZ #21539]

[Stefan Liebler]

On 07/24/2017 01:58 AM, Dmitry V. Levin wrote:
> On Thu, Jul 20, 2017 at 10:32:22AM +0200, Stefan Liebler wrote:
>> On 07/20/2017 10:06 AM, Carlos O'Donell wrote:
>>> On 07/20/2017 03:38 AM, Stefan Liebler wrote:
>>>> Carlos, shall we commit Dimitry's patch before the release?
>>>> Then we don't have this regression compared to glibc 2.25 release.
>>>
>>> I'm OK with Dmitry's patch going in for 2.26 to fix the regression.
>> Okay.
>>>> Could you please work with Dmitry to get this committed?
>> Okay.
>> Dimitry, I have one small note to your patch:
>> Can you add a space before the undefs and add the comment like this:
>> (according to
>> https://sourceware.org/glibc/wiki/Style_and_Conventions#Nested_C_Preprocessor_Directives)
>>
>> #if defined _LINUX_PTRACE_H || defined _S390_PTRACE_H
>> /* Kludge to stop stuff gdb & strace compiles from getting upset
>>    */
>> # undef PTRACE_TRACEME
>> ...
>> #endif /* defined _LINUX_PTRACE_H || defined _S390_PTRACE_H  */
>>
>> Then the patch is okay.
>> Can you please commit the patch?
> 
> OK, committed.
> 
> 
Thanks.
Stefan