NetBSD Problem Report #37631

From martin@duskware.de  Fri Dec 28 02:09:05 2007
Return-Path: <martin@duskware.de>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by narn.NetBSD.org (Postfix) with ESMTP id 065DE63B852
	for <gnats-bugs@gnats.NetBSD.org>; Fri, 28 Dec 2007 02:09:05 +0000 (UTC)
Message-Id: <20071228020902.930EA33B69@mail.duskware.de>
Date: Fri, 28 Dec 2007 03:09:02 +0100 (CET)
From: martin@duskware.de
Reply-To: martin@duskware.de
To: gnats-bugs@NetBSD.org
Subject: m68k libc broken
X-Send-Pr-Version: 3.95

>Number:         37631
>Category:       port-m68k
>Synopsis:       m68k libc broken
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    port-m68k-maintainer
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Dec 28 02:10:00 +0000 2007
>Closed-Date:    Tue Jan 15 11:32:18 +0000 2008
>Last-Modified:  Tue Jan 15 11:32:18 +0000 2008
>Originator:     Martin Husemann
>Release:        NetBSD 4.99.45
>Organization:
>Environment:
System: NetBSD  4.99.45 (MAC-BETH) #15: Thu Dec 27 12:41:04 CET 2007  martin@night-porter.duskware.de:/usr/src/sys/arch/mac68k/compile/MAC-BETH
Architecture: mac68k
Machine: m68k
>Description:

Current libc does not return the correct return value from mmap() - in one
example I tried ktrace says:

    12      1 ls       CALL  mmap(0,0x10be,1,1,3,0,0,0)
    12      1 ls       RET   mmap 67485696/0x405c000, 4286/0x10be

but the return value from the mmap call is 0xffffa530 (an address on the
userland stack in this case). Now imagine what happens if the application
munmaps the returned address :-(

>How-To-Repeat:

Install -current libc and watch everything fail ;-)

>Fix:
n/a

>Release-Note:

>Audit-Trail:
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: port-m68k/37631: m68k libc broken
Date: Fri, 28 Dec 2007 12:19:48 +0100

 Forgot to make it explicit: it is not a kernel problem, old static binaries
 continue to run, also an old libc.so.12.149 still "works". New binaries,
 static or dynamic, fail though.

 Martin

From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: port-m68k/37631: m68k libc broken
Date: Fri, 28 Dec 2007 15:44:08 +0100

 --AhhlLboLdkugWU4S
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: inline

 I was wrong: looking at the libc object code all looked fine to me.
 So I looked at the syscall code in the kernel and found the patch below,
 wich fixes this problem for me. The old libc code just papered over it.

 Is this ok to commit?

 Martin

 --AhhlLboLdkugWU4S
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: attachment; filename=patch

 Index: m68k_syscall.c
 ===================================================================
 RCS file: /cvsroot/src/sys/arch/m68k/m68k/m68k_syscall.c,v
 retrieving revision 1.30
 diff -c -u -r1.30 m68k_syscall.c
 --- m68k_syscall.c	17 Oct 2007 19:55:12 -0000	1.30
 +++ m68k_syscall.c	28 Dec 2007 14:41:35 -0000
 @@ -288,6 +288,8 @@
  			if (p->p_emul != &emul_netbsd_aoutm68k)
  				frame->f_regs[A0] = rval[0];
  		}
 +#else
 +		frame->f_regs[A0] = rval[0];
  #endif
  		break;
  	case ERESTART:

 --AhhlLboLdkugWU4S--

From: Ignatios Souvatzis <is@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: port-m68k-maintainer@netbsd.org, gnats-admin@netbsd.org,
	netbsd-bugs@netbsd.org, martin@duskware.de
Subject: Re: port-m68k/37631: m68k libc broken
Date: Fri, 28 Dec 2007 16:26:21 +0100

 On Fri, Dec 28, 2007 at 02:45:02PM +0000, Martin Husemann wrote:
 > The following reply was made to PR port-m68k/37631; it has been noted by GNATS.
 > 
 > From: Martin Husemann <martin@duskware.de>
 > To: gnats-bugs@NetBSD.org
 > Cc: 
 > Subject: Re: port-m68k/37631: m68k libc broken
 > Date: Fri, 28 Dec 2007 15:44:08 +0100
 > 
 >  --AhhlLboLdkugWU4S
 >  Content-Type: text/plain; charset=us-ascii
 >  Content-Disposition: inline
 >  
 >  I was wrong: looking at the libc object code all looked fine to me.
 >  So I looked at the syscall code in the kernel and found the patch below,
 >  wich fixes this problem for me. The old libc code just papered over it.
 >  
 >  Is this ok to commit?
 >  
 >  Martin
 >  
 >  --AhhlLboLdkugWU4S
 >  Content-Type: text/plain; charset=us-ascii
 >  Content-Disposition: attachment; filename=patch
 >  
 >  Index: m68k_syscall.c
 >  ===================================================================
 >  RCS file: /cvsroot/src/sys/arch/m68k/m68k/m68k_syscall.c,v
 >  retrieving revision 1.30
 >  diff -c -u -r1.30 m68k_syscall.c
 >  --- m68k_syscall.c	17 Oct 2007 19:55:12 -0000	1.30
 >  +++ m68k_syscall.c	28 Dec 2007 14:41:35 -0000
 >  @@ -288,6 +288,8 @@
 >   			if (p->p_emul != &emul_netbsd_aoutm68k)
 >   				frame->f_regs[A0] = rval[0];
 >   		}
 >  +#else
 >  +		frame->f_regs[A0] = rval[0];
 >   #endif
 >   		break;

 Hm... I'm not Klaus Klein, but yes, for m68kelf (at least), C functions
 return pointers returned in A0 (possibly in addition to D0).

 Whether to enforce this at the syscall interface or at the libc
 interface is probably a matter of taste, and your way we can't
 forget it.

 Uh... don't forget syscall_fancy (in addition to syscall_plain).

 	-is

From: itohy@NetBSD.org (ITOH Yasufumi)
To: gnats-bugs@NetBSD.org
Cc: port-m68k-maintainer@NetBSD.org, gnats-admin@NetBSD.org,
	netbsd-bugs@NetBSD.org, martin@duskware.de
Subject: Re: port-m68k/37631: m68k libc broken
Date: Sat, 29 Dec 2007 00:45:56 +0900

 Hello,

 martin@duskware.de writes:
 > Is this ok to commit?

 > Index: m68k_syscall.c
 > ===================================================================
 > RCS file: /cvsroot/src/sys/arch/m68k/m68k/m68k_syscall.c,v
 > retrieving revision 1.30
 > diff -c -u -r1.30 m68k_syscall.c
 > --- m68k_syscall.c	17 Oct 2007 19:55:12 -0000	1.30
 > +++ m68k_syscall.c	28 Dec 2007 14:41:35 -0000
 > @@ -288,6 +288,8 @@
 >  			if (p->p_emul != &emul_netbsd_aoutm68k)
 >  				frame->f_regs[A0] = rval[0];
 >  		}
 > +#else
 > +		frame->f_regs[A0] = rval[0];
 >  #endif
 >  		break;
 >  	case ERESTART:

 This should be in libc to avoid adding clock cycles to unrelated
 system calls.
 See shmat(2) stub.

 Every system calls returning pointer requires special stub
 for m68k SVR4 ABI (aka ELF).

 Regards,
 -- 
 ITOH Yasufumi

From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: port-m68k/37631: m68k libc broken
Date: Fri, 28 Dec 2007 17:45:51 +0100

 On Fri, Dec 28, 2007 at 03:50:01PM +0000, ITOH Yasufumi wrote:
 >  This should be in libc to avoid adding clock cycles to unrelated
 >  system calls.
 >  See shmat(2) stub.

 I agree, but that is not how it is done right now - and that it didn't
 work for me was just a side effect of me not having COMPAT_AOUT_M68K
 defined.

 What my change does is just remove the (bogus) dependency on that option.
 All GENERIC kernels already duplicate rval[0] to a0.

 So how about this: I commit the patch (with syscall_fancy added) for now
 and then start collecting a table of missing stubs in the m68k libc.
 Once we have added all missing stubs, we'll make the a0 copy #ifdef COMPAT_50?


 Martin

From: itohy@NetBSD.org (ITOH Yasufumi)
To: gnats-bugs@NetBSD.org
Cc: port-m68k-maintainer@NetBSD.org, gnats-admin@NetBSD.org,
	netbsd-bugs@NetBSD.org, martin@duskware.de
Subject: Re: port-m68k/37631: m68k libc broken
Date: Sat, 29 Dec 2007 02:34:39 +0900

 martin@duskware.de writes:

 > On Fri, Dec 28, 2007 at 03:50:01PM +0000, ITOH Yasufumi wrote:
 > >  This should be in libc to avoid adding clock cycles to unrelated
 > >  system calls.
 > >  See shmat(2) stub.
 > 
 > I agree, but that is not how it is done right now - and that it didn't
 > work for me was just a side effect of me not having COMPAT_AOUT_M68K
 > defined.

 Ah, I see.  The code to copy return value to a0
 didn't exist when I wrote userland ELF support.

 > What my change does is just remove the (bogus) dependency on that option.
 > All GENERIC kernels already duplicate rval[0] to a0.
 > 
 > So how about this: I commit the patch (with syscall_fancy added) for now
 > and then start collecting a table of missing stubs in the m68k libc.
 > Once we have added all missing stubs, we'll make the a0 copy #ifdef COMPAT_50?

 That looks fine.

 Thanks,
 -- 
 ITOH Yasufumi

From: Martin Husemann <martin@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: PR/37631 CVS commit: src/sys/arch/m68k/m68k
Date: Sat, 29 Dec 2007 00:02:53 +0000 (UTC)

 Module Name:	src
 Committed By:	martin
 Date:		Sat Dec 29 00:02:53 UTC 2007

 Modified Files:
 	src/sys/arch/m68k/m68k: m68k_syscall.c

 Log Message:
 Make the copy of the syscall return value to register A0 independend of
 options COMPAT_AOUT_M68K. Works around PR port-m68k/37631.
 XXX we will phase out the copy to A0 completely and handle it in userland
 XXX in the future.


 To generate a diff of this commit:
 cvs rdiff -r1.30 -r1.31 src/sys/arch/m68k/m68k/m68k_syscall.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: David Laight <david@l8s.co.uk>
To: gnats-bugs@NetBSD.org
Cc: port-m68k-maintainer@netbsd.org, martin@duskware.de
Subject: Re: port-m68k/37631: m68k libc broken
Date: Sat, 29 Dec 2007 19:46:05 +0000

 On Fri, Dec 28, 2007 at 04:50:02PM +0000, Martin Husemann wrote:
 > 
 >  On Fri, Dec 28, 2007 at 03:50:01PM +0000, ITOH Yasufumi wrote:
 >  >  This should be in libc to avoid adding clock cycles to unrelated
 >  >  system calls.
 >  >  See shmat(2) stub.
 >  
 >  I agree, but that is not how it is done right now - and that it didn't
 >  work for me was just a side effect of me not having COMPAT_AOUT_M68K
 >  defined.

 I suspect I broke it fairly recently when I changed the way the mmap()
 system call got stubbed (in order to avoid using SYS_SYSCALL).
 The m68k libc asm stub for mmap had probably never been used before.

 	David

 -- 
 David Laight: david@l8s.co.uk

From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: port-m68k/37631: m68k libc broken
Date: Sat, 29 Dec 2007 21:17:54 +0100

 Turns out the list is short:

 197     STD             { void *sys_mmap(void *addr, size_t len, int prot, \
 228     STD             { void *sys_shmat(int shmid, const void *shmaddr, \
 316     STD MPSAFE      { void *sys__lwp_getprivate(void); }
 411     STD             { void *sys_mremap(void *old_address, size_t old_size, \

 and only shmat() had the needed userland stub. I'll add the others and
 test...

 Martin

From: Martin Husemann <martin@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: PR/37631 CVS commit: src/lib/libc/arch/m68k/sys
Date: Mon, 31 Dec 2007 07:39:20 +0000 (UTC)

 Module Name:	src
 Committed By:	martin
 Date:		Mon Dec 31 07:39:19 UTC 2007

 Added Files:
 	src/lib/libc/arch/m68k/sys: __mmap.S _lwp_getprivate.S mremap.S

 Log Message:
 Add missing stubs for syscalls returning addresses.
 We can now run on a kernel that does not copy the syscall return value
 to register A0.
 As discussed in PR port-m68k/37631.


 To generate a diff of this commit:
 cvs rdiff -r0 -r1.1 src/lib/libc/arch/m68k/sys/__mmap.S \
     src/lib/libc/arch/m68k/sys/_lwp_getprivate.S \
     src/lib/libc/arch/m68k/sys/mremap.S

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: martin@netbsd.org
State-Changed-When: Tue, 15 Jan 2008 11:32:18 +0000
State-Changed-Why:
fixed


>Unformatted:

Home	PR Database Search