[PATCH] S/390: Fix makecontext with uc_link == NULL
Andreas Jaeger
aj@suse.com
Tue Jul 17 07:13:00 GMT 2012
On 07/16/2012 04:28 PM, Thomas Schwinge wrote:
> 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.
Yes, this is fine.
> 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
I suggest the sh maintainer to review this, it looks fine to me,
thanks,
Andreas
> 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
>
--
Andreas Jaeger aj@{suse.com,opensuse.org} Twitter/Identica: jaegerandi
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn,Jennifer Guild,Felix Imendörffer,HRB16746 (AG Nürnberg)
GPG fingerprint = 93A3 365E CE47 B889 DF7F FED1 389A 563C C272 A126
More information about the Libc-alpha
mailing list