From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 69371 invoked by alias); 27 Apr 2016 14:27:43 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 69360 invoked by uid 89); 27 Apr 2016 14:27:42 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 spammy=pretend, Hx-languages-length:1918 X-HELO: mail-qk0-f178.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:cc:from:organization :message-id:date:user-agent:mime-version:in-reply-to :content-transfer-encoding; bh=s7uXgtby7j/DncklU8MzIEKJryrV5Efv6nZz6wai4gc=; b=Sy00w0+CuHRmAMeN3ni4cxrKkl0C021nXJZnM4LSYgZrcesiUelVR1xT4EEgCTFGLD TBK6G+iOmexFvmN3eLxZ1MsgHBaO6zjyHuZtWHpUjmTWHlIvgZd/UIYk/Zv7lMdlZ/sT uqU2TffNfktaHsVxzGAa6tUhtLU+QLCIhSoYTrIXCNLwR7eBctzIIDZSZ33fodxjkEbI 23EguLHDdZEIffSqZyPEB6M71fiijnmHqSuxk/wH6An6mN5Fnljmy+pHUhzyYzS0KwBu lPU83dKG2ZI+Vgxa9FzaWgK6VUFx9srvYZroDZERqwEquOi0gp2o2dVlXsk5QKKZU9Sg Spjw== X-Gm-Message-State: AOPr4FXOSMmBTgoIytuo9iyoJG3uqm1iIh6TavQITs5KKsnYQhe8dqre1igJmW6SZEGmiOab X-Received: by 10.233.237.135 with SMTP id c129mr9031873qkg.59.1461767250657; Wed, 27 Apr 2016 07:27:30 -0700 (PDT) Subject: Re: [PATCH] Don't divide by zero when trying to destroy an uninitialised barrier. To: Torvald Riegel , Mark Thompson References: <5717B2F4.9050105@starleaf.com> <5717B657.6040007@arm.com> <5717D575.40806@starleaf.com> <1461255829.3869.578.camel@localhost.localdomain> Cc: Szabolcs Nagy , GNU C Library , nd From: Carlos O'Donell Message-ID: <5720CC4F.6060905@redhat.com> Date: Wed, 27 Apr 2016 14:27:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.1 MIME-Version: 1.0 In-Reply-To: <1461255829.3869.578.camel@localhost.localdomain> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2016-04/txt/msg00685.txt.bz2 On 04/21/2016 12:23 PM, Torvald Riegel wrote: > On Wed, 2016-04-20 at 20:16 +0100, Mark Thompson wrote: >> On 20/04/16 18:03, Szabolcs Nagy wrote: >>> On 20/04/16 17:48, Mark Thompson wrote: >>>> --- >>>> nptl/pthread_barrier_destroy.c | 9 +++++++++ >>>> 1 file changed, 9 insertions(+) >>>> >>>> diff --git a/nptl/pthread_barrier_destroy.c b/nptl/pthread_barrier_destroy.c >>>> index 92d2027..d114084 100644 >>>> --- a/nptl/pthread_barrier_destroy.c >>>> +++ b/nptl/pthread_barrier_destroy.c >>>> @@ -36,6 +36,15 @@ pthread_barrier_destroy (pthread_barrier_t *barrier) >>>> they have exited as well. To get the notification, pretend that we have >>>> reached the reset threshold. */ >>>> unsigned int count = bar->count; >>>> + >>>> + /* For an initialised barrier, count must be greater than zero here. An >>>> + uninitialised barrier may still have zero, however, and in this case it is >>>> + preferable to fail immediately rather than to invoke undefined behaviour >>>> + by dividing by zero on the next line. (POSIX allows the implementation to >>>> + diagnose invalid state and return EINVAL in this case.) */ >>>> + if (__glibc_unlikely (count == 0)) >>>> + return EINVAL; >>>> + >>> >>> this case is undefined behaviour in posix, and >>> i think the best way to handle that is crashing. >>> (because no behaviour can be portably relied upon) >> >> The undefined behaviour is not necessarily crashing - systems which >> do not trap on divide by zero (such as aarch64) will do something >> else, such as returning success or hanging forever. Would you >> prefer an abort() be added to make the behavior consistent? > > IMO, abort() would be better than returning EINVAL. See > https://sourceware.org/glibc/wiki/Style_and_Conventions#Bugs_in_the_user_program Agreed. It's easy to detect. We should abort(). -- Cheers, Carlos.