[PATCH-for-2.21-and-2.22] s390-64: remove socketcall syscalls

  From: Stefan Liebler <stli@linux.vnet.ibm.com>

Remove socketcalls in syscalls.list for s390-64. They were never used
on s390x and produce wrong code when glibc is built against 4.3 kernel
headers.

ChangeLog:

	* sysdeps/unix/sysv/linux/s390/s390-64/syscalls.list:
	Remove socketcall syscalls.

(partially cherry picked from commit 016495b818cb61df7d0d10e6db54074271b3e3a5)
---
 ChangeLog                                          |  5 +++++
 sysdeps/unix/sysv/linux/s390/s390-64/syscalls.list | 19 -------------------
 2 files changed, 5 insertions(+), 19 deletions(-)

This is a partial backport of commit 016495b8 which is already on master
and is needed when building glibc against 4.3+ kernel headers. Otherwise
we end up with wrong code, like this one for socket:

00000000000fb5b4 <socket@@GLIBC_2.2>:
   fb5b4:       a7 19 01 67             lghi    %r1,359
   fb5b8:       0a 00                   svc     0
   fb5ba:       a7 49 f0 01             lghi    %r4,-4095
   fb5be:       b9 21 00 24             clgr    %r2,%r4
   fb5c2:       c0 b4 00 00 00 04       jgnl    fb5ca <socket@@GLIBC_2.2+0x16>
   fb5c8:       07 fe                   br      %r14
   fb5ca:       13 02                   lcr     %r0,%r2
   fb5cc:       c0 10 00 05 0e 3e       larl    %r1,19d248 <_IO_file_jumps@@GLIBC_2.2+0x6f0>
   fb5d2:       e3 10 10 00 00 04       lg      %r1,0(%r1)
   fb5d8:       b2 4f 00 20             ear     %r2,%a0
   fb5dc:       eb 22 00 20 00 0d       sllg    %r2,%r2,32
   fb5e2:       b2 4f 00 21             ear     %r2,%a1
   fb5e6:       50 01 20 00             st      %r0,0(%r1,%r2)
   fb5ea:       a7 29 ff ff             lghi    %r2,-1
   fb5ee:       07 fe                   br      %r14
  

On Friday 25 December 2015 03:21:38 Aurelien Jarno wrote:
> From: Stefan Liebler <stli@linux.vnet.ibm.com>
> 
> Remove socketcalls in syscalls.list for s390-64. They were never used
> on s390x and produce wrong code when glibc is built against 4.3 kernel
> headers.
> 
> ChangeLog:
> 
> 	* sysdeps/unix/sysv/linux/s390/s390-64/syscalls.list:
> 	Remove socketcall syscalls.
> 
> (partially cherry picked from commit 016495b818cb61df7d0d10e6db54074271b3e3a5)
> ---
>  ChangeLog                                          |  5 +++++
>  sysdeps/unix/sysv/linux/s390/s390-64/syscalls.list | 19 -------------------
>  2 files changed, 5 insertions(+), 19 deletions(-)
> 
> This is a partial backport of commit 016495b8 which is already on master
> and is needed when building glibc against 4.3+ kernel headers. Otherwise
> we end up with wrong code, like this one for socket:

Hi Stefan,

Can you explain what exactly went wrong? I see that the same change that
was done in the kernel headers also made it into the m68k and x86
architectures:

Author: Geert Uytterhoeven <geert@linux-m68k.org>
Date:   Sun Sep 6 11:59:27 2015 +0200

    m68k: Wire up direct socket calls
    
    Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
    Acked-by: Greg Ungerer <gerg@uclinux.org>
commit 9dea5dc921b5f4045a18c63eb92e84dc274d17eb
Author: Andy Lutomirski <luto@kernel.org>
Date:   Tue Jul 14 15:24:24 2015 -0700

    x86/entry/syscalls: Wire up 32-bit direct socket calls


Do those have the same problem? Is there a way to avoid the problem for
other architectures that want to add the separate calls later?

	Arnd
  

On Fri, 25 Dec 2015, Aurelien Jarno wrote:

> From: Stefan Liebler <stli@linux.vnet.ibm.com>
> 
> Remove socketcalls in syscalls.list for s390-64. They were never used
> on s390x and produce wrong code when glibc is built against 4.3 kernel
> headers.

Could you explain why they produce wrong code?

Long term, once we can assume a kernel with direct socket syscalls on all 
architectures, we should get rid of socketcall support.  And the preferred 
form of getting rid of it isn't to have lots of C files calling syscall 
macros.  It's to use syscalls.list entries where possible, with C files 
only if needed because e.g. a function doesn't correspond directly to a 
syscall (Adhemerval's cancellation changes would add "syscall is a 
cancellation point" to the cases needing C files; I'm unconvinced that 
reducing the number of cases that can use syscalls.list is a good idea).  
And those syscalls.list entries would where possible be those in 
sysdeps/unix/syscalls.list (appropriately adjusted to be in sync with the 
current Linux-specific entries as needed).

So, if there are cases where listing such a syscall in syscalls.list 
produces wrong code, we need to understand those cases properly so we can 
allow for them in the future when eliminating socketcall support and using 
syscalls.list entries in more cases, architecture-independent.
  

On 2015-12-31 18:21, Joseph Myers wrote:
> On Fri, 25 Dec 2015, Aurelien Jarno wrote:
> 
> > From: Stefan Liebler <stli@linux.vnet.ibm.com>
> > 
> > Remove socketcalls in syscalls.list for s390-64. They were never used
> > on s390x and produce wrong code when glibc is built against 4.3 kernel
> > headers.
> 
> Could you explain why they produce wrong code?

I personally haven't checked. I have seen that compiling glibc against
4.3 kernel headers produced this wrong code. I then looked on master and
then realized these syscalls have been removed from syscalls.list. I
haven't really investigating more given I though the problem was
understood.

All that said we don't want these syscalls to be used as soon as we use
recent enough kernel headers as the resulting libc might be then used on
an older kernel.

> Long term, once we can assume a kernel with direct socket syscalls on all 
> architectures, we should get rid of socketcall support.  And the preferred 
> form of getting rid of it isn't to have lots of C files calling syscall 
> macros.  It's to use syscalls.list entries where possible, with C files 
> only if needed because e.g. a function doesn't correspond directly to a 
> syscall (Adhemerval's cancellation changes would add "syscall is a 
> cancellation point" to the cases needing C files; I'm unconvinced that 
> reducing the number of cases that can use syscalls.list is a good idea).  
> And those syscalls.list entries would where possible be those in 
> sysdeps/unix/syscalls.list (appropriately adjusted to be in sync with the 
> current Linux-specific entries as needed).
> 
> So, if there are cases where listing such a syscall in syscalls.list 
> produces wrong code, we need to understand those cases properly so we can 
> allow for them in the future when eliminating socketcall support and using 
> syscalls.list entries in more cases, architecture-independent.

Ok, I'll try to investigate in the next days.

Aurelien
  

On 2015-12-31 22:13, Aurelien Jarno wrote:
> On 2015-12-31 18:21, Joseph Myers wrote:
> > On Fri, 25 Dec 2015, Aurelien Jarno wrote:
> > 
> > > From: Stefan Liebler <stli@linux.vnet.ibm.com>
> > > 
> > > Remove socketcalls in syscalls.list for s390-64. They were never used
> > > on s390x and produce wrong code when glibc is built against 4.3 kernel
> > > headers.
> > 
> > Could you explain why they produce wrong code?
> 
> I personally haven't checked. I have seen that compiling glibc against
> 4.3 kernel headers produced this wrong code. I then looked on master and
> then realized these syscalls have been removed from syscalls.list. I
> haven't really investigating more given I though the problem was
> understood.
> 
> All that said we don't want these syscalls to be used as soon as we use
> recent enough kernel headers as the resulting libc might be then used on
> an older kernel.
> 
> > Long term, once we can assume a kernel with direct socket syscalls on all 
> > architectures, we should get rid of socketcall support.  And the preferred 
> > form of getting rid of it isn't to have lots of C files calling syscall 
> > macros.  It's to use syscalls.list entries where possible, with C files 
> > only if needed because e.g. a function doesn't correspond directly to a 
> > syscall (Adhemerval's cancellation changes would add "syscall is a 
> > cancellation point" to the cases needing C files; I'm unconvinced that 
> > reducing the number of cases that can use syscalls.list is a good idea).  
> > And those syscalls.list entries would where possible be those in 
> > sysdeps/unix/syscalls.list (appropriately adjusted to be in sync with the 
> > current Linux-specific entries as needed).
> > 
> > So, if there are cases where listing such a syscall in syscalls.list 
> > produces wrong code, we need to understand those cases properly so we can 
> > allow for them in the future when eliminating socketcall support and using 
> > syscalls.list entries in more cases, architecture-independent.
> 
> Ok, I'll try to investigate in the next days.

I have found some times to investigate. It seems there is no issue, my
bad. I just discovered that on s390x, syscalls numbers above 256 are
actually a a call to syscall 0 with the syscall number passed in
register %r1. My version of strace was not aware of the new syscalls 
and presented them as syscall setup(). Sorry for the false alert.

So it seems all is fine, but we still need the patch to use the
socketcall interface until we can assume the direct socket syscalls are
always available. I'll repost the patch on the libc-stable interface
with the comment fixed, as I have been told in the meantimes it's a
better list for discussing about already released versions.

Aurelien
  

On 31 Dec 2015 23:28, Aurelien Jarno wrote:
> I have found some times to investigate. It seems there is no issue, my
> bad. I just discovered that on s390x, syscalls numbers above 256 are
> actually a a call to syscall 0 with the syscall number passed in
> register %r1. My version of strace was not aware of the new syscalls 
> and presented them as syscall setup(). Sorry for the false alert.

is this a bug in strace ?  does it not handle `svc 0` insns correctly ?
if so, we should fix that.  i assumed the kernel would put the correct
NR into the r1 register regardless of how the syscall was invoked, but
maybe i'm being naive.
-mike
  

On 31-12-2015 16:21, Joseph Myers wrote:
> On Fri, 25 Dec 2015, Aurelien Jarno wrote:
> 
>> From: Stefan Liebler <stli@linux.vnet.ibm.com>
>>
>> Remove socketcalls in syscalls.list for s390-64. They were never used
>> on s390x and produce wrong code when glibc is built against 4.3 kernel
>> headers.
> 
> Could you explain why they produce wrong code?
> 
> Long term, once we can assume a kernel with direct socket syscalls on all 
> architectures, we should get rid of socketcall support.  And the preferred 
> form of getting rid of it isn't to have lots of C files calling syscall 
> macros.  It's to use syscalls.list entries where possible, with C files 
> only if needed because e.g. a function doesn't correspond directly to a 
> syscall (Adhemerval's cancellation changes would add "syscall is a 
> cancellation point" to the cases needing C files; I'm unconvinced that 
> reducing the number of cases that can use syscalls.list is a good idea).  
> And those syscalls.list entries would where possible be those in 
> sysdeps/unix/syscalls.list (appropriately adjusted to be in sync with the 
> current Linux-specific entries as needed).
> 
> So, if there are cases where listing such a syscall in syscalls.list 
> produces wrong code, we need to understand those cases properly so we can 
> allow for them in the future when eliminating socketcall support and using 
> syscalls.list entries in more cases, architecture-independent.
> 

Regarding my recent patches for cancellation changes, I am indifferent if
the syscall implementation is done either by direct C-code or through
syscalls.list auto-generation.  However, current syscall implementation
in GLIBC is cumbersome: it requires three definition for the exactly
same thing (assembly definitions for the syscalls.list, another
assembly definition that does the cancellation hackery and C macros for
inline syscalls), whic adds unnecessary complexity and maintenance.

My view it was required back in time when compiler generated code for the
C inline calls was deficient (either by missing some argument constraints
like i386/arm or by due some performance issues), however currently I 
see we should aim for only one definition: through C-code generated
syscalls (either by auto-generation from the syscalls.list or independent
C files).

So my idea is to first remove all the sysdep-cancel.h assembly hackery
to all ports, so a new port will only require to provide the assembly
macro definition and the C counterpart.  It should provide a clean
way for the new cancellation code and make the missing ports adjustment
simpler.

We can later change the syscalls.list generation to spill C code using
the INLINE_xxx macros.  It adds some complexity regarding the headers
inclusion, but it simplifier even further the requiring platform specific
code required for a new port and allows some cleanup on current ones.
We can even add the cancellation entrypoints in the syscalls.list by
using the SYSCALL_CANCEL macro as well.
  

On Mon, 4 Jan 2016, Adhemerval Zanella wrote:

> My view it was required back in time when compiler generated code for the
> C inline calls was deficient (either by missing some argument constraints
> like i386/arm or by due some performance issues), however currently I 
> see we should aim for only one definition: through C-code generated
> syscalls (either by auto-generation from the syscalls.list or independent
> C files).

I'm fine with syscalls using generated C code.

I think there was a suggestion in the past that the auto-generated 
syscalls ought to have debug information so you can see argument values 
when you interrupt a debugged program inside read or write or other 
syscalls.  If it could be arranged for auto-generated syscalls to have 
correct argument types and meaningful names, generating them in C might 
help get such debug information, though I don't know how good it would be 
in practice.
  

On 12/31/2015 11:28 PM, Aurelien Jarno wrote:
> I have found some times to investigate. It seems there is no issue, my
> bad. I just discovered that on s390x, syscalls numbers above 256 are
> actually a a call to syscall 0 with the syscall number passed in
> register %r1. My version of strace was not aware of the new syscalls
> and presented them as syscall setup(). Sorry for the false alert.
>
That's correct svc does only support NR up to 255 directly.
For 256 and above %r1=NR and svc 0 is needed.
The svc 0 with setup r1 works for NR < 256, too.

> So it seems all is fine, but we still need the patch to use the
> socketcall interface until we can assume the direct socket syscalls are
> always available. I'll repost the patch on the libc-stable interface
> with the comment fixed, as I have been told in the meantimes it's a
> better list for discussing about already released versions.
On s390x the direct socket syscalls were introduced with kernel 4.3.
Before 4.3, the socket syscalls were always done with c-files in 
sysdeps/unix/sysv/linux/.
The entries in syscalls.list were never used on s390x.

The upstream behaviour activates the direct socket calls if 
__LINUX_KERNEL_VERSION >= 0x040300. Otherwise the 
socketcall-wrapper-syscall is used.

If glibc is build on a system with kernel >= 4.3 and the entries in 
syscalls.list, then those direct syscalls are generated
and those aren't available on kernels < 4.3.
  

On 01/04/2016 10:39 AM, Joseph Myers wrote:
> On Mon, 4 Jan 2016, Adhemerval Zanella wrote:
> 
>> My view it was required back in time when compiler generated code for the
>> C inline calls was deficient (either by missing some argument constraints
>> like i386/arm or by due some performance issues), however currently I 
>> see we should aim for only one definition: through C-code generated
>> syscalls (either by auto-generation from the syscalls.list or independent
>> C files).
> 
> I'm fine with syscalls using generated C code.
> 
> I think there was a suggestion in the past that the auto-generated 
> syscalls ought to have debug information so you can see argument values 
> when you interrupt a debugged program inside read or write or other 
> syscalls.  If it could be arranged for auto-generated syscalls to have 
> correct argument types and meaningful names, generating them in C might 
> help get such debug information, though I don't know how good it would be 
> in practice.

I would also like SDT markers for entry to every syscall to assist in some
userspace tracing projects, and that would be easier in C.

Cheers,
Carlos.
  

On Fri, Jan 01, 2016 at 11:05:04PM -0500, Mike Frysinger wrote:
> On 31 Dec 2015 23:28, Aurelien Jarno wrote:
> > I have found some times to investigate. It seems there is no issue, my
> > bad. I just discovered that on s390x, syscalls numbers above 256 are
> > actually a a call to syscall 0 with the syscall number passed in
> > register %r1. My version of strace was not aware of the new syscalls 
> > and presented them as syscall setup(). Sorry for the false alert.
> 
> is this a bug in strace ?  does it not handle `svc 0` insns correctly ?
> if so, we should fix that.  i assumed the kernel would put the correct
> NR into the r1 register regardless of how the syscall was invoked, but
> maybe i'm being naive.

It's https://bugs.debian.org/485979

strace assumes that syscall number on s390/s390x is available in %r2,
which is correct both for "svc NR" and "svc 0 %r1=NR" methods unless
NR >= kernel's NR_syscalls.  In the latter case, %r2 is set to 0
but the original value that was stored in %r1 is also available
for a tracer.

I've pushed a fix (strace commit v4.11-159-g1763fa5) for this
longstanding issue.