NetBSD Problem Report #49597

From www@NetBSD.org  Thu Jan 22 14:04:13 2015
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 9D3BFA5B2E
	for <gnats-bugs@gnats.NetBSD.org>; Thu, 22 Jan 2015 14:04:13 +0000 (UTC)
Message-Id: <20150122140412.26548A654B@mollari.NetBSD.org>
Date: Thu, 22 Jan 2015 14:04:12 +0000 (UTC)
From: oshima-ya@yagoto-urayama.jp
Reply-To: oshima-ya@yagoto-urayama.jp
To: gnats-bugs@NetBSD.org
Subject: swapcontext(3) stack pointer problem on NetBSD/sh3
X-Send-Pr-Version: www-1.0

>Number:         49597
>Category:       port-sh3
>Synopsis:       swapcontext(3) stack pointer problem on NetBSD/sh3
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    port-sh3-maintainer
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Jan 22 14:05:00 +0000 2015
>Closed-Date:    Sun Apr 19 07:56:44 +0000 2015
>Last-Modified:  Sun Apr 19 07:56:44 +0000 2015
>Originator:     Yasushi Oshima
>Release:        NetBSD 7.0_BETA
>Organization:
>Environment:
NetBSD usl5p1 7.0_BETA NetBSD 7.0_BETA (GENERIC) #2: Sun Jan 18 01:57:10 JST 2015  oshima@sweety:/export/netbsd-7/obj/landisk/sys/arch/landisk/compile/GENERIC landisk
>Description:
In src/lib/libc/arch/sh3/gen/swapcontext.S,
Though SP is incremented 4 or 3 times in swapcontext(3)
till getcontext(2), it does not adjust stack pointer
in ocup->uc_mcontext.__gregs[_REG_SP] after that.

When will return to this context, 
PC will be set to a caller address of swapcontext(3).
But SP will show an invalid address. 
This will cause to crash.

The problem tends to occur by a program which is
compiled by gcc 4.8.x.
gcc 4.5.x generates code to save SP to another register
before function call and restore after, but gcc 4.8.x
does not save.
>How-To-Repeat:
On NetBSD/sh3 -7.0_BETA or -current,

cd /usr/tests/lib/libc/sys
atf-run t_swapcontext

This test failes by SIGSEGV.
>Fix:
Adjust SP in oldcontext by getcontext.

In the following patch, add #12 (PIC case, default)
or #8 (no PIC) to UC_MACHINE_SP(oucp).

--- lib/libc/arch/sh3/gen/swapcontext.S 12 Sep 2013 15:36:15 -0000      1.10
+++ lib/libc/arch/sh3/gen/swapcontext.S 21 Jan 2015 15:30:08 -0000
@@ -53,6 +53,11 @@
        /* Note: getcontext does _UC_MACHINE_INTRV(oucp) = 0 for us */
        mov.l   @sp, r0
        mov.l   r0, @(36 + 1 * 4, r1)   /* _UC_MACHINE_SET_PC(oucp, pr) */
+       /* Adjust stack pointer */
+       mov     sp, r2
+#ifdef __PIC__
+       add     #12, r2                 /* for 2 -sp + PIC_PROLOGUE */
+#else
+       add     #8, r2                  /* for 2 -sp */
+#endif
+       mov     #(36 + 21 * 4), r0      /* offset _UC_MACHINE_SP(oucp) */
+       mov.l   r2, @(r0, r1)           /* store SP */
        mov.l   .L_setcontext, r2
 2:     CALL    r2                      /* setcontext(ucp) */
         mov.l  @(4, sp), r4

>Release-Note:

>Audit-Trail:
From: Valery Ushakov <uwe@stderr.spb.ru>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: port-sh3/49597: swapcontext(3) stack pointer problem on NetBSD/sh3
Date: Thu, 22 Jan 2015 17:39:58 +0300

 On Thu, Jan 22, 2015 at 14:05:00 +0000, oshima-ya@yagoto-urayama.jp wrote:

 > In src/lib/libc/arch/sh3/gen/swapcontext.S,
 > Though SP is incremented 4 or 3 times in swapcontext(3)
 > till getcontext(2), it does not adjust stack pointer
 > in ocup->uc_mcontext.__gregs[_REG_SP] after that.

 Hmm, not directly related, but while looking at this... Don't we also
 miss

     mov #0, r0

 in getcontext (src/lib/libc/arch/sh3/sys/getcontext.S)?

 -uwe

From: Valery Ushakov <uwe@stderr.spb.ru>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: port-sh3/49597: swapcontext(3) stack pointer problem on NetBSD/sh3
Date: Thu, 22 Jan 2015 17:52:15 +0300

 On Thu, Jan 22, 2015 at 14:05:00 +0000, oshima-ya@yagoto-urayama.jp wrote:

 > --- lib/libc/arch/sh3/gen/swapcontext.S 12 Sep 2013 15:36:15 -0000      1.10
 > +++ lib/libc/arch/sh3/gen/swapcontext.S 21 Jan 2015 15:30:08 -0000
 > @@ -53,6 +53,11 @@
 >         /* Note: getcontext does _UC_MACHINE_INTRV(oucp) = 0 for us */
 >         mov.l   @sp, r0
 >         mov.l   r0, @(36 + 1 * 4, r1)   /* _UC_MACHINE_SET_PC(oucp, pr) */
 > +       /* Adjust stack pointer */
 > +       mov     sp, r2
 > +#ifdef __PIC__
 > +       add     #12, r2                 /* for 2 -sp + PIC_PROLOGUE */
 > +#else
 > +       add     #8, r2                  /* for 2 -sp */
 > +#endif
 > +       mov     #(36 + 21 * 4), r0      /* offset _UC_MACHINE_SP(oucp) */
 > +       mov.l   r2, @(r0, r1)           /* store SP */
 >         mov.l   .L_setcontext, r2
 >  2:     CALL    r2                      /* setcontext(ucp) */
 >          mov.l  @(4, sp), r4

 Have you tested this?

 From a quick look, getcontext() captures the state that has not two,
 but three registers in the stack - including mov.l r4, @-sp in the
 delay slot of the CALL, doesn't it?

 -uwe

From: Yasushi Oshima <oshima-ya@yagoto-urayama.jp>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: port-sh3/49597: swapcontext(3) stack pointer problem on
 NetBSD/sh3
Date: Fri, 23 Jan 2015 01:42:39 +0900 (JST)

 Thu, 22 Jan 2015 14:55:00 +0000 (UTC) 
 Valery Ushakov <uwe@stderr.spb.ru> wrote'

 >  Have you tested this?

 Yes, but __PIC__ case only. 

 The atf-tests in lib/libc/sys/t_swapcontext and fs/psshfs became
 to success by this patch in my environment.

 >  From a quick look, getcontext() captures the state that has not two,
 >  but three registers in the stack - including mov.l r4, @-sp in the
 >  delay slot of the CALL, doesn't it?

 Yes include in the delay slot, and include in PIC_PROLOGUE() macro.

 Stacking register is:
 1) PIC_PROLOGUE(.L_got) ; r12 when __PIC__ only in sh3/asm.h
 2) mov.l r5, @-sp
 3) sts.l pr, @-sp
 4) mov.l r4, @-sp  ; in delay slot (CALL is bsrf or bsr)
 and get from stack after getcontext()
 5) mov.l @sp+ r1

 SP saved in a ucontext stored by getcontext() shows 4 regs(__PIC__)
 or 3 regs (NOT __PIC__) away from swapcontext() caller.
 After getcontext() and pop stack by 5), current SP is 3 (or 2) regs away.

 And I've checked this difference of SP is 12 bytes (=3 regs) by gdb 
 libc/sys/t_swapcontext test program (in __PIC__ case).

 Thanks.

From: Valery Ushakov <uwe@stderr.spb.ru>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: port-sh3/49597: swapcontext(3) stack pointer problem on NetBSD/sh3
Date: Thu, 22 Jan 2015 19:53:23 +0300

 On Thu, Jan 22, 2015 at 16:45:01 +0000, Yasushi Oshima wrote:

 > From: Yasushi Oshima <oshima-ya@yagoto-urayama.jp>
 > To: gnats-bugs@NetBSD.org
 > Cc: 
 > Subject: Re: port-sh3/49597: swapcontext(3) stack pointer problem on
 >  NetBSD/sh3
 > Date: Fri, 23 Jan 2015 01:42:39 +0900 (JST)
 > 
 >  Thu, 22 Jan 2015 14:55:00 +0000 (UTC) 
 >  Valery Ushakov <uwe@stderr.spb.ru> wrote'
 >  
 >  >  Have you tested this?
 >  
 >  Yes, but __PIC__ case only. 
 >  
 >  The atf-tests in lib/libc/sys/t_swapcontext and fs/psshfs became
 >  to success by this patch in my environment.
 >    
 >  >  From a quick look, getcontext() captures the state that has not two,
 >  >  but three registers in the stack - including mov.l r4, @-sp in the
 >  >  delay slot of the CALL, doesn't it?
 >  
 [...]
 >  And I've checked this difference of SP is 12 bytes (=3 regs) by gdb 
 >  libc/sys/t_swapcontext test program (in __PIC__ case).

 Ah, right, sorry.  You are using live adjusted sp (after r4 is popped)
 to compute the final value, not saved sp from the context.

 -uwe

From: "Valeriy E. Ushakov" <uwe@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/49597 CVS commit: src/lib/libc/arch/sh3/gen
Date: Sun, 25 Jan 2015 01:32:26 +0000

 Module Name:	src
 Committed By:	uwe
 Date:		Sun Jan 25 01:32:26 UTC 2015

 Modified Files:
 	src/lib/libc/arch/sh3/gen: swapcontext.S

 Log Message:
 Adjust _UC_MACHINE_SP(oucp) to "drop" values we pushed to the stack.
 t_swapcontext tests pass now with gcc 4.8.

 From Yasushi Oshima in PR port-sh3/49597


 To generate a diff of this commit:
 cvs rdiff -u -r1.10 -r1.11 src/lib/libc/arch/sh3/gen/swapcontext.S

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

From: Valery Ushakov <uwe@stderr.spb.ru>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: port-sh3/49597: swapcontext(3) stack pointer problem on NetBSD/sh3
Date: Sun, 25 Jan 2015 06:39:09 +0300

 Thanks, I have applied the patch.  But it's got me thinking... We are
 tweaking oucp to resume "returning" from swapcontext, but we have
 already clobbered r12 and pr before/by calling getcontext, so oucp
 will get resumed with wrong r12 and pr values.  It probably goes
 undetected since the caller saves and restores pr itself (since it
 calls swapcontext at least) and since r12 is not used because the test
 is not in a DSO.

 So what about something like:

 ENTRY(swapcontext)
 	PIC_PROLOGUE(.L_got)
 	sts.l	pr, @-sp

 	mov.l	.L__getcontext, r0
 	mov.l	r5, @-sp
 1:	CALL	r0			! _getcontext(oucp)
 	 mov.l	r4, @-sp
 	!! getcontext captures oucp resuming here with r12 (when PIC),
 	!! pr, r5 and r4 pushed onto the stack

 	tst	r0, r0
 	bf.s	3f			! return error from getcontext
 	 mov.l	@sp, r1			! saved oucp

 	!! adjust oucp to resume after setcontext below
 	mova	3f, r0
 	mov.l	r0, @(36 + 1 * 4, r1)	! _UC_MACHINE_SET_PC(oucp, pr)

 	mov.l	.L_setcontext, r2
 2:	CALL	r2			! setcontext(ucp)
 	 mov.l	@(4, sp), r4		! saved ucp

 	.align	2
 3:	!! we get here on errors and when resuming oucp
 	add	#8, sp			! skip r4 and r5
 	lds.l	@sp+, pr
 	rts
 	 PIC_EPILOGUE_SLOT

 though I haven't tested error legs.

 -uwe

From: Yasushi Oshima <oshima-ya@yagoto-urayama.jp>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: port-sh3/49597: swapcontext(3) stack pointer problem on
 NetBSD/sh3
Date: Wed, 28 Jan 2015 23:46:02 +0900 (JST)

 > From: Valery Ushakov <uwe@stderr.spb.ru>

 >  Thanks, I have applied the patch.  But it's got me thinking... We are
 >  tweaking oucp to resume "returning" from swapcontext, but we have
 >  already clobbered r12 and pr before/by calling getcontext, so oucp
 >  will get resumed with wrong r12 and pr values. 

 Ah!
 Yes, certainly, resuming r12 will be necessary. I forgot it. 

 >  It probably goes
 >  undetected since the caller saves and restores pr itself (since it
 >  calls swapcontext at least) and since r12 is not used because the test
 >  is not in a DSO.

 I try your patch,
 then, the atf-test in lib/libpthread/t_swapcontext
 became to work well! (this test-case has failed yet with my patch.)

 >  though I haven't tested error legs.

 It will be not enough but I tests very simple test:

  swapcontext(NULL,&ucp)

 It returns -1 with errno=14(EFAULT), its ok.

 Thank you very much.

 -- 
 Yasushi Oshima

From: "Valeriy E. Ushakov" <uwe@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/49597 CVS commit: src/lib/libc/arch/sh3/gen
Date: Mon, 6 Apr 2015 01:50:47 +0000

 Module Name:	src
 Committed By:	uwe
 Date:		Mon Apr  6 01:50:47 UTC 2015

 Modified Files:
 	src/lib/libc/arch/sh3/gen: swapcontext.S

 Log Message:
 On swapcontext(3) preserve r12 too.  Properly fixes PR port-sh3/49597.


 To generate a diff of this commit:
 cvs rdiff -u -r1.11 -r1.12 src/lib/libc/arch/sh3/gen/swapcontext.S

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

From: "SAITOH Masanobu" <msaitoh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/49597 CVS commit: [netbsd-7] src/lib/libc/arch/sh3/gen
Date: Sun, 19 Apr 2015 05:05:41 +0000

 Module Name:	src
 Committed By:	msaitoh
 Date:		Sun Apr 19 05:05:41 UTC 2015

 Modified Files:
 	src/lib/libc/arch/sh3/gen [netbsd-7]: swapcontext.S

 Log Message:
 Pull up following revision(s) (requested by uwe in ticket #709):
 	lib/libc/arch/sh3/gen/swapcontext.S: revision 1.11
 	lib/libc/arch/sh3/gen/swapcontext.S: revision 1.12
  - Adjust _UC_MACHINE_SP(oucp) to "drop" values we pushed to the stack.
    t_swapcontext tests pass now with gcc 4.8. From Yasushi Oshima in
    PR port-sh3/49597.
  - On swapcontext(3) preserve r12 too.  Properly fixes PR port-sh3/49597.


 To generate a diff of this commit:
 cvs rdiff -u -r1.10 -r1.10.4.1 src/lib/libc/arch/sh3/gen/swapcontext.S

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

From: "SAITOH Masanobu" <msaitoh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/49597 CVS commit: [netbsd-6] src/lib/libc/arch/sh3/gen
Date: Sun, 19 Apr 2015 05:51:57 +0000

 Module Name:	src
 Committed By:	msaitoh
 Date:		Sun Apr 19 05:51:57 UTC 2015

 Modified Files:
 	src/lib/libc/arch/sh3/gen [netbsd-6]: swapcontext.S

 Log Message:
 Pull up following revision(s) (requested by uwe in ticket #1294):

 lib/libc/arch/sh3/gen/swapcontext.S		1.11-1.12 via patch

 	Correctly preserve old sate in swapcontext(3).
 	Fixes PR port-sh3/49597 reported by Yasushi Oshima.


 To generate a diff of this commit:
 cvs rdiff -u -r1.9 -r1.9.24.1 src/lib/libc/arch/sh3/gen/swapcontext.S

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

From: "SAITOH Masanobu" <msaitoh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/49597 CVS commit: [netbsd-5] src/lib/libc/arch/sh3/gen
Date: Sun, 19 Apr 2015 07:43:02 +0000

 Module Name:	src
 Committed By:	msaitoh
 Date:		Sun Apr 19 07:43:02 UTC 2015

 Modified Files:
 	src/lib/libc/arch/sh3/gen [netbsd-5]: swapcontext.S

 Log Message:
 Pull up following revision(s) (requested by uwe in ticket #1960):

 lib/libc/arch/sh3/gen/swapcontext.S		1.11-1.12 via patch

 	Correctly preserve old sate in swapcontext(3).
 	Fixes PR port-sh3/49597 reported by Yasushi Oshima.


 To generate a diff of this commit:
 cvs rdiff -u -r1.9 -r1.9.6.1 src/lib/libc/arch/sh3/gen/swapcontext.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: msaitoh@NetBSD.org
State-Changed-When: Sun, 19 Apr 2015 07:56:44 +0000
State-Changed-Why:
Pulled up.
Thanks.


>Unformatted:

Home	PR Database Search