From patchwork Mon Mar 23 15:59:08 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Carlos O'Donell X-Patchwork-Id: 453517 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id D872C14017B for ; Tue, 24 Mar 2015 02:59:28 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass reason="1024-bit key; unprotected key" header.d=sourceware.org header.i=@sourceware.org header.b=XGtipKil; dkim-adsp=none (unprotected policy); dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:subject :references:in-reply-to:content-type:content-transfer-encoding; q=dns; s=default; b=WTgg4lbmSKqO4qZf0wKBJ7JCI6DK81UL/RJhZ+Y1Xv9 Jxr3FG7MEChfeSBUDJaJvjF5iHPdDzjx6VwSUHYOCOC0M962K6Zd9GSe+tjrX+xx G/b5oB9o7Y2kW/woPtZ2b2lkw1Hz8dS/YCcEuYTHP8IKbCqFBr/CrI4o61TCG7Ag = DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:subject :references:in-reply-to:content-type:content-transfer-encoding; s=default; bh=c/jm+cizqA0VLKlVkjpgObjaCX8=; b=XGtipKiltFdICOPft nssChjcY+EQDLJGXwBi4n9S1qdLTThBYa64mbcr4QHoU+yefB9YByk6FrJ0N3OgH JbQzY+pcjArdJiPR9zGoDf9xKRSc1NB5JtdJk7DtDGXKpF+Dk0gRfMFLvsAU3O1g 2w54Vg3m1gHpjbMBkRGKVxvibg= Received: (qmail 55598 invoked by alias); 23 Mar 2015 15:59:23 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 55577 invoked by uid 89); 23 Mar 2015 15:59:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Message-ID: <5510384C.5090304@redhat.com> Date: Mon, 23 Mar 2015 11:59:08 -0400 From: "Carlos O'Donell" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: GNU C Library , John David Anglin , Marcus Shawcroft , Chung-Lin Tang , Andreas Jaeger Subject: [PATCH] v3 BZ# 18125: setcontext: Call exit, not _exit, after last linked context executes. References: <55035374.6060906@redhat.com> <550763D9.8010603@redhat.com> <20150317075802.GC8109@vapier> In-Reply-To: <20150317075802.GC8109@vapier> On 03/17/2015 03:58 AM, Mike Frysinger wrote: > On 16 Mar 2015 19:14, Carlos O'Donell wrote: >> * Makefile (tests): Add tst-setcontext3. > > your ChangeLog says Makefile, but the patch doesn't contain it ... Fixed. Attached. >> --- /dev/null >> +++ b/stdlib/tst-setcontext3.c >> >> +char *filename; > > could be static right ? Fixed. Made static. >> +static int >> +do_test (int argc, char **argv) >> +{ >> + int ret; >> + char st1[32768]; >> + ucontext_t tempctx = ctx; >> + >> + if (argc < 2) >> + { >> + printf ("FAIL: Test missing filename argument.\n"); >> + exit (1); > > it's not wrong, just weird, but this func uses exit half the time and return the > other half ... Fixed. All use exit(). >> --- /dev/null >> +++ b/stdlib/tst-setcontext3.sh >> >> +cleanup() { >> + rm -f "${tempfiles[@]}" >> +} > > stylewise, seems like we normally use two space indent ? Fixed. Use two space indent. >> +# We want to run the test program and see if secontext called >> +# exit() and wrote out the test file we specified. If the >> +# test exits with a non-zero status this will fail because we >> +# are using `set -e`. >> +$test_pre $test "$tempfile" > > what about 77 ? the test_pre part really only ever expands into `env VAR=val` > right ? so i think you need to explicitly capture & test the exit value in > order to handle 77 correctly. The script uses `set -e` which means any failure in an executed subshell immediately causes the present shell to exit with that error. The rules that are running tests can detect an exit code of 77 and translate that into UNSUPPORTED. I tested this by hand by making the test exit (77) immediately and it is correctly reported as an UNSUPPORTED test. If nobody objects to this version, this is what I'll commit shortly. It brings us a step closer to correctly exiting the process, but does not resolve the the fact that the standard says "thread" and thus means we should be calling pthread_exit. I don't have any opinion on that right now. My goal here to mainly to align AArch64 with x86_64 in terms of functionality. v3 - Attach Makefile diff. - Correct Changelog. - Make filename static. - Use exit() everywhere. - Use 2 space indent in shell. - Update shell wrapper to call tst-setcontext3. Cheers, Carlos. 2015-03-23 Carlos O'Donell [BZ #18125] * stdlib/tst-setcontext3.c: New file. * stdlib/tst-setcontext3.sh: New file. * stdlib/Makefile (tests): Add tst-setcontext3. (tst-setcontext3.out): Custom rule to run tst-setcontext3.sh to verify test program created output file. * sysdeps/unix/sysv/linux/aarch64/setcontext.S: Call exit. * sysdeps/unix/sysv/linux/arm/setcontext.S: Likewise. * sysdeps/unix/sysv/linux/hppa/setcontext.S: Likewise. * sysdeps/unix/sysv/linux/nios2/setcontext.S: Likewise. diff --git a/stdlib/Makefile b/stdlib/Makefile index c78fdb5..3300dd2 100644 --- a/stdlib/Makefile +++ b/stdlib/Makefile @@ -73,7 +73,8 @@ tests := tst-strtol tst-strtod testmb testrand testsort testdiv \ tst-qsort2 tst-makecontext2 tst-strtod6 tst-unsetenv1 \ tst-makecontext3 bug-getcontext bug-fmtmsg1 \ tst-secure-getenv tst-strtod-overflow tst-strtod-round \ - tst-tininess tst-strtod-underflow tst-tls-atexit + tst-tininess tst-strtod-underflow tst-tls-atexit \ + tst-setcontext3 tests-static := tst-secure-getenv modules-names = tst-tls-atexit-lib @@ -157,3 +158,9 @@ tst-tls-atexit-lib.so-no-z-defs = yes $(objpfx)tst-tls-atexit: $(shared-thread-library) $(libdl) $(objpfx)tst-tls-atexit.out: $(objpfx)tst-tls-atexit-lib.so + +$(objpfx)tst-setcontext3.out: tst-setcontext3.sh $(objpfx)tst-setcontext3 + $(SHELL) $< $(common-objpfx) '$(test-program-prefix-before-env)' \ + '$(run-program-env)' '$(test-program-prefix-after-env)' \ + $(common-objpfx)stdlib/; \ + $(evaluate-test) diff --git a/stdlib/tst-setcontext3.c b/stdlib/tst-setcontext3.c new file mode 100644 index 0000000..fda2128 --- /dev/null +++ b/stdlib/tst-setcontext3.c @@ -0,0 +1,138 @@ +/* Bug 18125: Verify setcontext calls exit() and not _exit(). + Copyright (C) 2015 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/* Please note that depending on the outcome of Bug 18135 this test + may become invalid, and instead of testing for calling exit it + should be reworked to test for the last context calling + pthread_exit(). */ + +static ucontext_t ctx; +static char *filename; + +/* It is intended that this function does nothing. */ +static void +cf (void) +{ + printf ("called context function\n"); +} + +static void +exit_called (void) +{ + int fd; + ssize_t res; + const char buf[] = "Called exit function\n"; + + fd = open (filename, O_WRONLY | O_CREAT, S_IRUSR | S_IWUSR); + if (fd == -1) + { + printf ("FAIL: Unable to create test file %s\n", filename); + exit (1); + } + res = write (fd, buf, sizeof (buf)); + if (res != sizeof (buf)) + { + printf ("FAIL: Expected to write test file in one write call.\n"); + exit (1); + } + res = close (fd); + if (res == -1) + { + printf ("FAIL: Failed to close test file.\n"); + exit (1); + } + printf ("PASS: %s", buf); +} + +/* The test expects a filename given by the wrapper calling script. + The test then registers an atexit handler that will create the + file to indicate that the atexit handler ran. Then the test + creates a context, modifies it with makecontext, and sets it. + The context has only a single context which then must exit. + If it incorrectly exits via _exit then the atexit handler is + not run, the file is not created, and the wrapper detects this + and fails the test. This test cannot be done using an _exit + interposer since setcontext avoids the PLT and calls _exit + directly. */ +static int +do_test (int argc, char **argv) +{ + int ret; + char st1[32768]; + ucontext_t tempctx = ctx; + + if (argc < 2) + { + printf ("FAIL: Test missing filename argument.\n"); + exit (1); + } + + filename = argv[1]; + + atexit (exit_called); + + puts ("making contexts"); + if (getcontext (&ctx) != 0) + { + if (errno == ENOSYS) + { + /* Exit with 77 to mark the test as UNSUPPORTED. */ + printf ("UNSUPPORTED: getcontext not implemented.\n"); + exit (77); + } + + printf ("FAIL: getcontext failed.\n"); + exit (1); + } + + ctx.uc_stack.ss_sp = st1; + ctx.uc_stack.ss_size = sizeof (st1); + ctx.uc_link = 0; + makecontext (&ctx, cf, 0); + + /* Without this check, a stub makecontext can make us spin forever. */ + if (memcmp (&tempctx, &ctx, sizeof ctx) == 0) + { + puts ("UNSUPPORTED: makecontext was a no-op, presuming not implemented"); + exit (77); + } + + ret = setcontext (&ctx); + if (ret != 0) + { + printf ("FAIL: setcontext returned with %d and errno of %d.\n", ret, errno); + exit (1); + } + + printf ("FAIL: Impossibly returned to main.\n"); + exit (1); +} + +#include "../test-skeleton.c" diff --git a/stdlib/tst-setcontext3.sh b/stdlib/tst-setcontext3.sh new file mode 100644 index 0000000..6ad67a8 --- /dev/null +++ b/stdlib/tst-setcontext3.sh @@ -0,0 +1,54 @@ +#! /bin/bash +# Bug 18125: Test the exit functionality of setcontext(). +# Copyright (C) 2015 Free Software Foundation, Inc. +# This file is part of the GNU C Library. + +# The GNU C Library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. + +# The GNU C Library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. + +# You should have received a copy of the GNU Lesser General Public +# License along with the GNU C Library; if not, see +# . + +set -e + +common_objpfx=$1 +test_program_prefix_before_env=$2 +run_program_env=$3 +test_program_prefix_after_env=$4 +objpfx=$5 + +test_pre="${test_program_prefix_before_env} ${run_program_env}" +test="${test_program_prefix_after_env} ${objpfx}tst-setcontext3" +out=${objpfx}tst-setcontext3.out + +tempfiles=() +cleanup() { + rm -f "${tempfiles[@]}" +} +trap cleanup 0 + +tempfile=$(mktemp "tst-setcontext3.XXXXXXXXXX") +tempfiles+=("$tempfile") + +# We want to run the test program and see if secontext called +# exit() and wrote out the test file we specified. If the +# test exits with a non-zero status this will fail because we +# are using `set -e`. +$test_pre $test "$tempfile" + +# Look for resulting file. +if [ -e "$tempfile" ]; then + echo "PASS: tst-setcontext3 an exit() and created $tempfile" + exit 0 +else + echo "FAIL: tst-setcontext3 did not create $tempfile" + exit 1 +fi diff --git a/sysdeps/unix/sysv/linux/aarch64/setcontext.S b/sysdeps/unix/sysv/linux/aarch64/setcontext.S index 6dd7836..ae67581 100644 --- a/sysdeps/unix/sysv/linux/aarch64/setcontext.S +++ b/sysdeps/unix/sysv/linux/aarch64/setcontext.S @@ -125,5 +125,5 @@ weak_alias (__setcontext, setcontext) ENTRY (__startcontext) mov x0, x19 cbnz x0, __setcontext -1: b HIDDEN_JUMPTARGET (_exit) +1: b HIDDEN_JUMPTARGET (exit) END (__startcontext) diff --git a/sysdeps/unix/sysv/linux/arm/setcontext.S b/sysdeps/unix/sysv/linux/arm/setcontext.S index 5268e06..24c7294 100644 --- a/sysdeps/unix/sysv/linux/arm/setcontext.S +++ b/sysdeps/unix/sysv/linux/arm/setcontext.S @@ -91,7 +91,7 @@ ENTRY(__startcontext) bne PLTJMP(__setcontext) @ New context was 0 - exit - b PLTJMP(HIDDEN_JUMPTARGET(_exit)) + b PLTJMP(HIDDEN_JUMPTARGET(exit)) END(__startcontext) #ifdef PIC diff --git a/sysdeps/unix/sysv/linux/hppa/setcontext.S b/sysdeps/unix/sysv/linux/hppa/setcontext.S index 7271410..abe87a9 100644 --- a/sysdeps/unix/sysv/linux/hppa/setcontext.S +++ b/sysdeps/unix/sysv/linux/hppa/setcontext.S @@ -18,6 +18,7 @@ . */ #include +#include #include "ucontext_i.h" @@ -139,7 +140,7 @@ ENTRY(__setcontext) nop /* No further context available. Exit now. */ - bl _exit, %r2 + bl HIDDEN_JUMPTARGET(exit), %r2 ldi -1, %r26 diff --git a/sysdeps/unix/sysv/linux/nios2/setcontext.S b/sysdeps/unix/sysv/linux/nios2/setcontext.S index 9a8dd87..f40b733 100644 --- a/sysdeps/unix/sysv/linux/nios2/setcontext.S +++ b/sysdeps/unix/sysv/linux/nios2/setcontext.S @@ -89,15 +89,15 @@ ENTRY(__startcontext) mov r4, r16 bne r4, zero, __setcontext - /* If uc_link == zero, call _exit. */ + /* If uc_link == zero, call exit. */ #ifdef PIC nextpc r22 1: movhi r8, %hiadj(_gp_got - 1b) addi r8, r8, %lo(_gp_got - 1b) add r22, r22, r8 - ldw r8, %call(HIDDEN_JUMPTARGET(_exit))(r22) + ldw r8, %call(HIDDEN_JUMPTARGET(exit))(r22) jmp r8 #else - jmpi _exit + jmpi exit #endif END(__startcontext)