From patchwork Tue May 19 22:11:06 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Roland McGrath X-Patchwork-Id: 474077 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 06EBD1402B3 for ; Wed, 20 May 2015 08:11:16 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=sourceware.org header.i=@sourceware.org header.b=BJlMc+iB; 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:mime-version:content-type :content-transfer-encoding:from:to:cc:subject:message-id:date; q=dns; s=default; b=TJRy8S0Vbsa58ZODXYYLC2xbrOyqUVYuppFzJDHO1ZI GSAjB2lX9geDxfzGYODEgb5/lWGjvX9YIy5fjzo+glWoAS36wKok7UkIpxT3ZVnM OSNTQs+Zei1pahMQMFtYcmpfXhwWRM0zzhAC4P9gxfrJLxTx+2q+1FF+Et7youAU = 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:mime-version:content-type :content-transfer-encoding:from:to:cc:subject:message-id:date; s=default; bh=M6kb6zlNWim2izqvfEow+Oclptc=; b=BJlMc+iBBcuFB3q1Q ImPD1HGeA6J1awgkbIb9I54KhTgoADaPwzsECND8f7Sy2Yf8sBLq9mOaa+M4RqNE KaVOdYvOE2KL9de41zlOllIMKcC7Gv4adhZ7DNowonWl3ly1w3yVFpzasFs+7+vh lj7rd4HeEyrHE8bGsdxb1jfC6M= Received: (qmail 42581 invoked by alias); 19 May 2015 22:11:10 -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 42570 invoked by uid 89); 19 May 2015 22:11:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL, BAYES_00, KAM_LAZY_DOMAIN_SECURITY autolearn=no version=3.3.2 X-HELO: topped-with-meat.com MIME-Version: 1.0 From: Roland McGrath To: "GNU C. Library" CC: Torvald Riegel , Carlos O'Donell Subject: [PATCH roland/sem_post] BZ#18434: Fix sem_post EOVERFLOW check for [!__HAVE_64B_ATOMICS]. Message-Id: <20150519221106.7114B2C3A73@topped-with-meat.com> Date: Tue, 19 May 2015 15:11:06 -0700 (PDT) X-CMAE-Score: 0 X-CMAE-Analysis: v=2.1 cv=SvUDtp+0 c=1 sm=1 tr=0 a=WkljmVdYkabdwxfqvArNOQ==:117 a=14OXPxybAAAA:8 a=kj9zAlcOel0A:10 a=hOe2yjtxAAAA:8 a=mDV3o1hIAAAA:8 a=cQ_HCTco0spIRQm8gM8A:9 a=CjuIK1q_8ugA:10 This bug is a regression introduced by the 2.21 semaphore rewrite. It affects all machines without __HAVE_64B_ATOMICS. Tested i686-linux-gnu and arm-nacl, where the new test case fails before the fix and passes after it. I didn't test the sparc32 code, but the bug and the fix are the same as in the generic code. This seems appropriate for a 2.21 backport, and I've marked the bug that way (I've just added the glibc_2.21 keyword since we didn't have one yet). Thanks, Roland 2015-05-19 Roland McGrath [BZ #18434] * nptl/tst-sem15.c: New file. * nptl/Makefile (tests): Add it. * nptl/sem_post.c (__new_sem_post) [!__HAVE_64B_ATOMICS]: s/<>/ to fix typo in EOVERFLOW check. * sysdeps/sparc/sparc32/sem_post.c (__new_sem_post): Likewise. diff --git a/nptl/Makefile b/nptl/Makefile index d784c8d..fe1ba05 100644 --- a/nptl/Makefile +++ b/nptl/Makefile @@ -228,6 +228,7 @@ tests = tst-typesizes \ tst-key1 tst-key2 tst-key3 tst-key4 \ tst-sem1 tst-sem2 tst-sem3 tst-sem4 tst-sem5 tst-sem6 tst-sem7 \ tst-sem8 tst-sem9 tst-sem10 tst-sem11 tst-sem12 tst-sem13 tst-sem14 \ + tst-sem15 \ tst-barrier1 tst-barrier2 tst-barrier3 tst-barrier4 \ tst-align tst-align3 \ tst-basic1 tst-basic2 tst-basic3 tst-basic4 tst-basic5 tst-basic6 \ diff --git a/nptl/sem_post.c b/nptl/sem_post.c index 6e495ed..b6d30b5 100644 --- a/nptl/sem_post.c +++ b/nptl/sem_post.c @@ -84,14 +84,14 @@ __new_sem_post (sem_t *sem) unsigned int v = atomic_load_relaxed (&isem->value); do { - if ((v << SEM_VALUE_SHIFT) == SEM_VALUE_MAX) + if ((v >> SEM_VALUE_SHIFT) == SEM_VALUE_MAX) { __set_errno (EOVERFLOW); return -1; } } - while (!atomic_compare_exchange_weak_release (&isem->value, - &v, v + (1 << SEM_VALUE_SHIFT))); + while (!atomic_compare_exchange_weak_release + (&isem->value, &v, v + (1 << SEM_VALUE_SHIFT))); /* If there is any potentially blocked waiter, wake one of them. */ if ((v & SEM_NWAITERS_MASK) != 0) diff --git a/nptl/tst-sem15.c b/nptl/tst-sem15.c new file mode 100644 index 0000000..3df9561 --- /dev/null +++ b/nptl/tst-sem15.c @@ -0,0 +1,99 @@ +/* Test for SEM_VALUE_MAX overflow detection: BZ #18434. + 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 + + +static int +do_test (void) +{ + sem_t s; + + if (sem_init (&s, 0, SEM_VALUE_MAX)) + { + printf ("sem_init: %m\n"); + return 1; + } + + int result = 0; + + int value = 0xdeadbeef; + if (sem_getvalue (&s, &value)) + { + printf ("sem_getvalue: %m\n"); + result = 1; + } + else + { + printf ("sem_getvalue after init: %d\n", value); + if (value != SEM_VALUE_MAX) + { + printf ("\tshould be %d\n", SEM_VALUE_MAX); + result = 1; + } + } + + errno = 0; + if (sem_post(&s) == 0) + { + puts ("sem_post at SEM_VALUE_MAX succeeded!"); + result = 1; + } + else + { + printf ("sem_post at SEM_VALUE_MAX: %m (%d)\n", errno); + if (errno != EOVERFLOW) + { + printf ("\tshould be %s (EOVERFLOW = %d)\n", + strerror (EOVERFLOW), EOVERFLOW); + result = 1; + } + } + + value = 0xbad1d00d; + if (sem_getvalue (&s, &value)) + { + printf ("sem_getvalue: %m\n"); + result = 1; + } + else + { + printf ("sem_getvalue after post: %d\n", value); + if (value != SEM_VALUE_MAX) + { + printf ("\tshould be %d\n", SEM_VALUE_MAX); + result = 1; + } + } + + if (sem_destroy (&s)) + { + printf ("sem_destroy: %m\n"); + result = 1; + } + + return result; +} + +#define TEST_FUNCTION do_test () +#include "../test-skeleton.c" diff --git a/sysdeps/sparc/sparc32/sem_post.c b/sysdeps/sparc/sparc32/sem_post.c index 64cd851..c9f85a0 100644 --- a/sysdeps/sparc/sparc32/sem_post.c +++ b/sysdeps/sparc/sparc32/sem_post.c @@ -60,19 +60,19 @@ __new_sem_post (sem_t *sem) int private = isem->private; unsigned int v; - __sparc32_atomic_do_lock24(&isem->pad); + __sparc32_atomic_do_lock24 (&isem->pad); v = isem->value; - if ((v << SEM_VALUE_SHIFT) == SEM_VALUE_MAX) + if ((v >> SEM_VALUE_SHIFT) == SEM_VALUE_MAX) { - __sparc32_atomic_do_unlock24(&isem->pad); + __sparc32_atomic_do_unlock24 (&isem->pad); __set_errno (EOVERFLOW); return -1; } isem->value = v + (1 << SEM_VALUE_SHIFT); - __sparc32_atomic_do_unlock24(&isem->pad); + __sparc32_atomic_do_unlock24 (&isem->pad); if ((v & SEM_NWAITERS_MASK) != 0) futex_wake (&isem->value, 1, private);