This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] S/390: Fix makecontext with uc_link == NULL


Hi!

On Mon, 16 Jul 2012 13:25:07 +0200, Andreas Krebbel <krebbel@linux.vnet.ibm.com> wrote:
> On 07/16/2012 10:19 AM, Thomas Schwinge wrote:
> > On Thu, 12 Jul 2012 09:38:35 -0400, Carlos O'Donell <carlos_odonell@mentor.com> wrote:
> >> On 7/12/2012 8:19 AM, Andreas Krebbel wrote:
> >>>> Do we have a testcase that covers this failure?
> >>>>
> >>>> If we don't, could you try to work one up?
> >>>
> >>> stdlib/tst-makecontext already calls makecontext with uc_link == NULL
> >>> but the function invoked in the context does explicitly call exit (0).
> >>> Removing this enables the testcase to cover that problem as well.
> > 
> >>> 	* stdlib/tst-makecontext.c: Remove explicit exit call.
> >>
> >> Excellent. The testcase changes look good to me and match what
> >> SuSv2 says about a returning from a context where uc_link is
> >> zero e.g. "the thread will exit when this context returns."
> >>
> >> I'm happy with this, thanks for enhancing the testcase to cover
> >> the failure scenario.
> > 
> > I already raised this topic in
> > <http://news.gmane.org/find-root.php?message_id=%3C87lijq5h72.fsf@schwinge.name%3E>:
> > 
> > | [a bug is only seen] when returning from a context with Âuc_link ==
> > | NULLÂ, which is not exercised in the testsuite.
> > | 
> > | I first though about simply removing the Âexit (0)Â from
> > | stdlib/tst-makecontext.c:cf (which would then test exactly this case),
> > | but apparently it is not specified which status value to use for exit in
> > | this case -- libc.info: ÂIf `uc_link' was a null pointer the application
> > | terminates in this case. -- so it is not trivial to test for.  (Maybe
> > | worth specifying?  EXIT_SUCCESS (0)?)
> Ok. Thanks for the pointer. I think everything other than 0 doesn't make much sense since
> exiting that way is not caused by any kind of error.

Ack.

> I could check in the testcase change removing the exit and we could make the back-end
> maintainers aware of the problem with their target. What do you think?

How about this one?

manual: setcontext: Clarify termination when uc_link is the null pointer.

2012-07-16  Thomas Schwinge  <thomas@codesourcery.com>
	    Andreas Krebbel  <Andreas.Krebbel@de.ibm.com>

	* manual/setjmp.texi (setcontext): Clarify normal process
	termination when uc_link is the null pointer.
	* stdlib/tst-makecontext.c (cf): Exercise this: remove explicit
	exit call.

diff --git a/manual/setjmp.texi b/manual/setjmp.texi
index a5a7ce6..f13ac7b 100644
--- a/manual/setjmp.texi
+++ b/manual/setjmp.texi
@@ -354,7 +354,8 @@ specified parameters passed.  If this function returns execution is
 resumed in the context which was referenced by the @code{uc_link}
 element of the context structure passed to @code{makecontext} at the
 time of the call.  If @code{uc_link} was a null pointer the application
-terminates in this case.
+terminates normally with an exit status value of @code{EXIT_SUCCESS}
+(@pxref{Program Termination}).
 
 Since the context contains information about the stack no two threads
 should use the same context at the same time.  The result in most cases
diff --git a/stdlib/tst-makecontext.c b/stdlib/tst-makecontext.c
index 3185900..eb6e89b 100644
--- a/stdlib/tst-makecontext.c
+++ b/stdlib/tst-makecontext.c
@@ -35,7 +35,9 @@ cf (int i)
       printf ("i %d thr %d\n", i, thr);
       exit (1);
     }
-  exit (0);
+
+  /* Since uc_link below has been set to NULL, setcontext is supposed to
+     terminate the process normally after this function returns.  */
 }
 
 int

x86_64: makecontext: exit (0) if uc_link is the null pointer.

2012-07-16  Thomas Schwinge  <thomas@codesourcery.com>

	* sysdeps/unix/sysv/linux/x86_64/__start_context.S
	(__start_context): Preserve zero value for regular exit case.

diff --git a/sysdeps/unix/sysv/linux/x86_64/__start_context.S b/sysdeps/unix/sysv/linux/x86_64/__start_context.S
index 77d322e..9f2ee23 100644
--- a/sysdeps/unix/sysv/linux/x86_64/__start_context.S
+++ b/sysdeps/unix/sysv/linux/x86_64/__start_context.S
@@ -39,8 +39,9 @@ ENTRY(__start_context)
 	call	JUMPTARGET(__setcontext)
 	/* If this returns (which can happen if the syscall fails) we'll
 	   exit the program with the return error value (-1).  */
+	movq	%rax,%rdi
 
-2:	movq	%rax,%rdi
+2:
 	call	HIDDEN_JUMPTARGET(exit)
 	/* The 'exit' call should never return.  In case it does cause
 	   the process to terminate.  */

SH: makecontext: exit (0) if uc_link is the null pointer.

2012-07-16  Thomas Schwinge  <thomas@codesourcery.com>

	* sysdeps/unix/sysv/linux/sh/makecontext.S (.Lexitcode): Preserve
	zero value for regular exit case.

diff --git a/sysdeps/unix/sysv/linux/sh/makecontext.S b/sysdeps/unix/sysv/linux/sh/makecontext.S
index a04bc9f..73fca8b 100644
--- a/sysdeps/unix/sysv/linux/sh/makecontext.S
+++ b/sysdeps/unix/sysv/linux/sh/makecontext.S
@@ -128,6 +128,7 @@ ENTRY(__makecontext)
 	cfi_restore (pr)
 	/* If this returns (which can happen if the syscall fails) we'll exit
 	   the program with the return error value (-1).  */
+	mov	r0, r4
 
 2:
 	mov.l	.Lexit, r1
@@ -135,7 +136,7 @@ ENTRY(__makecontext)
 	add	r12, r1
 #endif
 	jsr	@r1
-	 mov	r0, r4
+	 nop
 	/* The 'exit' call should never return.  In case it does cause the
 	   process to terminate.  */
 	ABORT_INSTRUCTION_ASM

Further (incomplete) notes for port maintainers have been gives as
follows:

    i386: sysdeps/unix/sysv/linux/i386/makecontext.S:L(exitcode): seems OK
    x86_64: sysdeps/unix/sysv/linux/x86_64/__start_context.S: 2: is misplaced, should be after the movq %rax, %rdi
    SPARC32: sysdeps/unix/sysv/linux/sparc/sparc32/setcontext.S:__start_context can't tell
    SPARC64: sysdeps/unix/sysv/linux/sparc/sparc64/makecontext.c:__makecontext_ret missing handling completely
    S390-32: sysdeps/unix/sysv/linux/s390/s390-32/makecontext.c:__makecontext_ret missing handling completely
    S390-64: sysdeps/unix/sysv/linux/s390/s390-64/makecontext.c:__makecontext_ret missing handling completely


GrÃÃe,
 Thomas

Attachment: pgp00000.pgp
Description: PGP signature


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]