From owner-svn-src-stable-11@freebsd.org Sun Sep 4 00:04:45 2016 Return-Path: Delivered-To: svn-src-stable-11@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id DC4DFBCFBBB; Sun, 4 Sep 2016 00:04:45 +0000 (UTC) (envelope-from yaneurabeya@gmail.com) Received: from mail-pa0-x235.google.com (mail-pa0-x235.google.com [IPv6:2607:f8b0:400e:c03::235]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id AC12C18F; Sun, 4 Sep 2016 00:04:45 +0000 (UTC) (envelope-from yaneurabeya@gmail.com) Received: by mail-pa0-x235.google.com with SMTP id fu3so43236909pad.3; Sat, 03 Sep 2016 17:04:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=y0ol6cpD8dvuatRXA0/y6UQk85TXBkrekN2NFRv1VVI=; b=ffchu3h9PWoTWO7nz1PIHlbRBfFFnFnRyxuBmLwv9ieNYM0iWUxiUvHOeIlPCSko0/ UryVE9xEiUsFnI0qaiWPGRLmuPfKUOp+OkfLCzHFzBymV3sPLuQeK8UAwR+xRngLWOKU Yzp+eRvrwpGmLxlFYG8oHHJ9XAO4am8Sc5zSHnhqGvlb1qCXsF2R7UVIzd0UHlW2iyjU vbNBKlIDNd2looIUK6ZoUMBu2+p0uopRLocYwmLQxKi53Xosw4Zj7S9A3dTpMr8aawbs FjF5MtG1wVxFUrnGeAPWrvmZj/F+RaVZvEVYrw+y4wMMpBtliBkFzYqwMnARPU7VL5NC fpHA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=y0ol6cpD8dvuatRXA0/y6UQk85TXBkrekN2NFRv1VVI=; b=ScVefBiuldP4NUzd6rFXvACUWk1sV6PmnlWhvJyty3IjGyijmmjGoA0t6m+ZBIzs/u 644URCB3cNnf6FJO3fhOjYZxYcfEZEwHKQ0EEVJ/jGUhafa8YR+JiDN5GqheIr+C2+9t gC2/jEimAUI1KjjYbkyVuf8fBeGqod6dqCtNRO12/H1lJTlj8V62vtBgI9xtGxGXzTUH NwS4JTYx7TFoexgcp7mCRh5pkQTGI4v8MeqLicWQxuq0O63PkkxSvWgL1ezAyk9rVYcB BUfI/ZB0dxKeHXMiJCWI0GkXCWhZJVWj3bCku5oktf0e/5wbAQEoyoXDjXSq6rUqDr/H avsg== X-Gm-Message-State: AE9vXwNP3/9X4d0Z8UofUnUHrtQ+/bgqxKXZYK04RUCLUFeNewBbcqREtaUCzF0qyutPGQ== X-Received: by 10.66.151.9 with SMTP id um9mr26026722pab.85.1472947485098; Sat, 03 Sep 2016 17:04:45 -0700 (PDT) Received: from ?IPv6:2607:fb90:f32:78bf:2c41:bfdb:c7d9:2099? ([2607:fb90:f32:78bf:2c41:bfdb:c7d9:2099]) by smtp.gmail.com with ESMTPSA id xn11sm24135331pac.38.2016.09.03.17.04.44 (version=TLS1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Sat, 03 Sep 2016 17:04:44 -0700 (PDT) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (1.0) Subject: Re: svn commit: r304947 - stable/11/tests/sys/kern/acct From: Ngie Cooper X-Mailer: iPhone Mail (13G36) In-Reply-To: <20160828174315.E1696@besplex.bde.org> Date: Sat, 3 Sep 2016 17:04:43 -0700 Cc: Garrett Cooper , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Content-Transfer-Encoding: quoted-printable Message-Id: References: <201608280710.u7S7Am7X057748@repo.freebsd.org> <20160828174315.E1696@besplex.bde.org> To: Bruce Evans X-BeenThere: svn-src-stable-11@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: SVN commit messages for only the 11-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 04 Sep 2016 00:04:46 -0000 > On Aug 28, 2016, at 01:49, Bruce Evans wrote: ... I agree. This commit in effect papered over the problem. More investigation w= ill be done with the PR that introduced the expected failure. Thanks! -Ngie >=20 > This can't depend on 64-bitness. It might depend on FLT_EPSILON, but > IEEE might require a specific representation of floats and we only have > and only support one. >=20 > This is probably a bug in the tests that shows up on arches with extra > precision. Perhaps just a complier bug. >=20 >> Modified: stable/11/tests/sys/kern/acct/acct_test.c >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D >> --- stable/11/tests/sys/kern/acct/acct_test.c Sun Aug 28 07:09:45 2016= (r304946) >> +++ stable/11/tests/sys/kern/acct/acct_test.c Sun Aug 28 07:10:48 2016= (r304947) >> @@ -204,7 +204,10 @@ ATF_TC_BODY(encode_tv_random_million, tc >> struct timeval tv; >> long k; >>=20 >> - atf_tc_expect_fail("the testcase violates FLT_EPSILON"); >> +#ifdef __LP64__ >> + atf_tc_expect_fail("the testcase violates FLT_EPSILON on 64-bit " >> + "platforms, e.g. amd64"); >> +#endif >>=20 >> ATF_REQUIRE_MSG(unsetenv("TZ") =3D=3D 0, "unsetting TZ failed; errno=3D= %d", errno); >=20 > The rest of the function is: >=20 > X for (k =3D 1; k < 1000000L; k++) { > X tv.tv_sec =3D random(); > X tv.tv_usec =3D (random() % 1000000L); > X v.c =3D encode_timeval(tv); > X check_result(atf_tc_get_ident(tc), > X (float)tv.tv_sec * AHZ + tv.tv_usec, v); > X } >=20 > AHZ here is less than an obfuscation of literal 10000000 or just 1e6 or > 1e6F. It doesn't even have the style bug of an L suffix like the nearby > literals. Types are important here, but the L isn't. >=20 > AHZ used to be a constant related to fixed-point conversions in acct.h. > It used to have value 1000. Note much like the AHZ. now > devfines AHZV1 and this has value 64. This is for a legacy API. Not > very compatible. >=20 > This file doesn't include except possibly via namespace > pollution, so it doesn't get any AHZ*. It only uses AHZ to convert > tv_sec to usec. This was magical and may be broken. The file convert.c > is included. This is a converted copy of kern_acct.c. Back when AHZ > existed in acct.h, kern_acct.c used to use AHZ with its different value. > I don't see how overriding that value either didn't cause a redefinition > error or inconsistencies. Now kern_acct.c doesn't use either AHZ* so > this definition is not magical. >=20 > So AHZ should be replaced by literal 1000000 except possibly for type > problems. IIRC, C99 specifies the dubious behaviour of converting > integers to float in float expressions to support the dubious behaviour > of evaluating float expressions in float precision. 1000000 is even > exactly representable in float precision. But the result of the > mutliplication isn't in general. Adding a small tv_usec to a not > very large tv_sec converted to usec is almost certain to not be > representable in a 32-bit float after a few random choices. So > we expect innacuracies. >=20 > The float expression may be evaluated in extra precision, and is on > i386. So we expect smaller inaccuracies on i386. >=20 > It is not clear if remaining bugs are in the test or the compiler. > Probably both. The test asks for inaccuracies and gets them in the > expression sometimes. It doesn't try to force the inaccuracies by > casting to float, and only C90+ compilers do this cast as specified > since the specification specifies behaviour that is too pessimal to > use. C90+ compilers are in short supply, but gcc later than aout > 4.6 properlay pessimizes the cast when instructed to by certain > compiler flags. >=20 > But the test it calls a function which should do the conversion. It > takes excessive inlining combined with the de-pessimization to not > do the conversion. Apparently, clang does do the excessive inlining. > Otherwise the result would be the same on i386 as on amd64. >=20 > The test seems to be quite broken. encode_timeval() does some > conversion which is presumably correct. We calculate a value in > usec to compare against. This is only done in float precision > (possibly higher, but we don't control this). We expect a relative > error of about FLT_EPSILON in this. Later we convert to a relative > error, so this is only slightly broken. encode_timeval() must > have a rounding error, and our calculation has one and the scaling > has more. So we should expect errors of several times FLT_EPSILON. > So the test only seems to be slightly broken. Strictly less than > FLT_EPSILON is too strict if the calculations are actually done in > float precision since it is too difficult to calculate the reference > and do the scaling without increasing the error. The worst case > for the reference is tv_sec =3D 2**31-1 (31 bits) and tv_usec =3D 999999 > (20 bits). That is exactly representable in 53 bits (double precision) > so we should use that. >=20 > Bruce