From owner-freebsd-numerics@freebsd.org Tue Jan 1 15:01:50 2019 Return-Path: Delivered-To: freebsd-numerics@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 7BB46141B17A for ; Tue, 1 Jan 2019 15:01:50 +0000 (UTC) (envelope-from pfg@FreeBSD.org) Received: from sonic316-8.consmr.mail.gq1.yahoo.com (sonic316-8.consmr.mail.gq1.yahoo.com [98.137.69.32]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id CA5A671DA4 for ; Tue, 1 Jan 2019 15:01:49 +0000 (UTC) (envelope-from pfg@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s2048; t=1546354906; bh=jtd7BnKCEum75h9MVzGvY82HkjmVy0CG12EsUpFtQU4=; h=Subject:From:To:Cc:References:Date:In-Reply-To:From:Subject; b=DKNpylPUdnk+QDrDiB+v5g28EAcTSbQFF9Git9zFoe3rCc/GAt9xCvD+PWJGp1xuI0jjVIzodaT+V0/DybyWLATBzNVUiwcx2oNlNZiy4HDo3Hho9C/PIlU/pKYrvjgqwpg6JW2aiE2pPygFywYyez5lN8lA9BpLXUKi3BXcjCCPljeapW/c3FEzAuHk8puAomNZgkHTQGyhK0fOv8KAyTwL4jr9nz4yR9Hn2DtEG1iTBPWBrxI7ZtsfvkNJL1UrWxl+yAW1OU790spLTgw9U0Kpm1ZvAtVTCr7azrGze2vKI7LjR+G5OQjUeJ/X5DuMO0MHouhAaaGp3vFOD/ydnw== X-YMail-OSG: TmguIWkVM1kpuhvfSOnvGmfuqjdHXezTpwJ7kpPb4bKUs2vkMuif0BNjO4o2Xy6 1aE3B8x9d3U4GWjRqhzo23aT0FW1cS9oZdKv9ORGTJzkBy3Ut2ySBPregLB0MCdjGUJKzLslQzHd XOTxvD1e3NiBotCahYbU8rBtKpZFX1KMkqnPnxAebRZo5Abay7NR.oJbPW2MX.sljXGjWKBRXwa0 Z87Ci2PAb.PxQtPL37YYxb5yS5OUS7vIEGYDhwNfbA.FJZdW7HrmDjSb3i3ggH0MAEE7eO1kqrdE wbcuL0kFs22JNNefaaLqscPZKd9uAtA7i6DYWVgfAOeUH3GFbl71RJEOBA_HKsxHJAGQGHlmxdte ZirwNN57l1fwOCS3grQWWPZZj5Uu4RLJE33L5cMHzxter93KJQ_5yRGs1ZgKPTp7lH3fMs2zdfTn GKFfhh0xnImF.DBOc6xmhw_1XtJywSms4RcjyN1dTAwpcNLXEaNu2Cu6cOBBobl31eOVNcoYomjR meoO9AqA5aNVwAjhUxzwBpMI1uLGeHF4K6EuyKutxDmnlvIiepRK1m29S5HyyM3lDa1PjyMFIGEq TS6BJBqknTjzu.mvGYRs5BE15U1w7B72vFiNMKBp28y4XBuDXKdPuVx8NxpJuO4pH_ilz6mYchgm rw8stc0GZYh5x9sxw.xkehX_bgDABMqG_T4rbpE0XvYlWGPd8yNrLebGAajcb4koh.1l6cKijlkN DnSM2kIjsdTz4W6Z7fxmOh5kjiVpbEv8GxaaHa7YMVcR8JuJjfbDy5LOexOSYp_6_p9rVG0Mk8Ky XhGjnki9KJycNHX1vOF04rmfOX_pYjyXuRBq1fDznDPA3DfpxWn70xGLm8zZnaoxfqJVsXkRwTCI vWep1g0V2cyPJYUCRADRPzAxvTBNmyZS0A9z_CF55kAvQ13QDUtRaVMbzbZz6OEougWiD_4vx5i1 dlgJgyGJYq08YuwSM_G4KTXShlTkVDhPS1G7uUEP.BgMHKGPhBMjpE.2rID7iqclVX9zEL.6wNJo KP5D9cfRKrZSBpSrB3qrKeGHhtSqmtr3j5A1aSmIBf.M- Received: from sonic.gate.mail.ne1.yahoo.com by sonic316.consmr.mail.gq1.yahoo.com with HTTP; Tue, 1 Jan 2019 15:01:46 +0000 Received: from 181.52.72.201 (EHLO [192.168.0.5]) ([181.52.72.201]) by smtp403.mail.gq1.yahoo.com (Oath Hermes SMTP Server) with ESMTPA ID 798d012e16fa5a294af7c4216195e738; Tue, 01 Jan 2019 15:01:46 +0000 (UTC) Subject: Re: Undefined Behavior in lib/msun/src/e_pow.c (was Re: New math library from ARM) From: Pedro Giffuni To: sgk@troutmask.apl.washington.edu Cc: freebsd-numerics@freebsd.org References: <797a7755-db93-1b9c-f3b9-8850d948e098@FreeBSD.org> <20181231151904.GB823@troutmask.apl.washington.edu> <20181231152230.GC823@troutmask.apl.washington.edu> <06c8b6a2-ed26-f255-3947-c79b593a9dea@FreeBSD.org> <20190101045425.GA5767@troutmask.apl.washington.edu> <20190101055225.GA5982@troutmask.apl.washington.edu> Message-ID: <9cdce84b-2cff-d752-67a9-4c9ef391cc2c@FreeBSD.org> Date: Tue, 1 Jan 2019 10:01:45 -0500 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------82A09505CD03AE157475B862" Content-Language: en-US X-Rspamd-Queue-Id: CA5A671DA4 X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-2.98 / 15.00]; local_wl_from(0.00)[FreeBSD.org]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; NEURAL_HAM_SHORT(-0.98)[-0.980,0]; ASN(0.00)[asn:36647, ipnet:98.137.64.0/21, country:US]; NEURAL_HAM_LONG(-1.00)[-1.000,0] X-BeenThere: freebsd-numerics@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Discussions of high quality implementation of libm functions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 01 Jan 2019 15:01:50 -0000 This is a multi-part message in MIME format. --------------82A09505CD03AE157475B862 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit On 1/1/19 1:20 AM, Pedro Giffuni wrote: > > On 1/1/19 12:52 AM, Steve Kargl wrote: >> On Tue, Jan 01, 2019 at 12:14:38AM -0500, Pedro Giffuni wrote: >>>> I'll defer to Bruce on this.  My only comments are >>>> 1) declarations belong at the top of the file where all >>>> declarations occur >>> Modern standards let you declare variables within blocks. For style we >>> do prefer to start the function with them but in this case we can't. >> I'am aware of what modern standards allow.  I'm also >> aware of the code style of fdlibm, which I think >> should be maintain. > > Well, the alternative is the musl-libc patch, which declares the > variables upon use (twice). > > I think my patch is a little bit more acceptable. > Actually, perhaps a cast is just enough ... See below ... >>>> 2) j is already declared as int32_t >>> And it has to be a signed integer: we later check for negative values. >> Yeah, I know.  That's why I pointed out the collision. >> >>>> 3) uint32_t should be written as u_int32_t. >>> No, uint32_t is the standard type, u_int32_t is a BSDism. Also, the >>> file >>> consistently uses uint32_t. >> Ah, no.  e_pow.c uses u_int32_t on line 107. > > Duh! I completely misread, sorry about that I probably was still > looking at musl-libc instead. > > Yes, we should use u_int32_t. > > >>    This would be easy >> to see if all declarations are grouped together.   In additional, >> don't you need to include stdint.h to get uint32_t? > > Well, my patch did compile,  but I will match the existing style if > the change is accepted/desired. > > >> % grep uint32_t /usr/src/lib/msun/src/e_pow.c >> % grep u_int32_t /usr/src/lib/msun/src/e_pow.c >>          u_int32_t lx,ly; >>          n = ((u_int32_t)hx>>31)-1; >> >> Code churn to placate lint for an event that cannot occur >> seems dubious to me. >> > UBsan should be able to detect it. I think Undefined Behavior is > considered a portability bug. > > There is no urgency but it is still a bug. > In the new patch: the first j comparison has to be made against an unsigned value anyways and the second comparison will never be against the magical 31 so there is no need to cast that one. Pedro. --------------82A09505CD03AE157475B862 Content-Type: text/x-patch; name="msun-pow-ub.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="msun-pow-ub.diff" Index: lib/msun/src/e_pow.c =================================================================== --- lib/msun/src/e_pow.c (revision 342665) +++ lib/msun/src/e_pow.c (working copy) @@ -133,7 +133,7 @@ k = (iy>>20)-0x3ff; /* exponent */ if(k>20) { j = ly>>(52-k); - if((j<<(52-k))==ly) yisint = 2-(j&1); + if(((u_int32_t)j<<(52-k))==ly) yisint = 2-(j&1); } else if(ly==0) { j = iy>>(20-k); if((j<<(20-k))==iy) yisint = 2-(j&1); --------------82A09505CD03AE157475B862--