Date: Sun, 22 Mar 2015 16:21:48 -0700 From: Craig Rodrigues <rodrigc@FreeBSD.org> To: Dimitry Andric <dim@freebsd.org> Cc: "freebsd-testing@freebsd.org" <freebsd-testing@freebsd.org>, freebsd-current Current <freebsd-current@freebsd.org>, "jenkins-admin@freebsd.org" <jenkins-admin@freebsd.org>, freebsd-toolchain@freebsd.org Subject: Re: Jenkins build is still unstable: FreeBSD_HEAD-tests2 #867 Message-ID: <CAG=rPVcTa3Cni8uO9AfT0UPBF015_yb_%2B%2BzJz%2BE132hM_Cs9sQ@mail.gmail.com> In-Reply-To: <CAG=rPVdP_TAmerKh_82EravEw=sCj4LyEBu-NfGCK0Obg=HMVw@mail.gmail.com> References: <1669399171.13.1427029129760.JavaMail.jenkins@jenkins-9.freebsd.org> <799490341.14.1427048792932.JavaMail.jenkins@jenkins-9.freebsd.org> <CAG=rPVeyamLPnC5i05_=Ub0D%2BV256U_8t8R5tfDitC=NkjBOFQ@mail.gmail.com> <BE5A3694-E1D1-4A47-8707-D6214E997363@FreeBSD.org> <494AEF4B-0AF8-449A-9B41-9AC4F4552AF0@FreeBSD.org> <CAG=rPVchdm_VaTshq%2BRN%2BkHX0YC0_Tsx22oJVNNnoOamdm00mQ@mail.gmail.com> <864EB4DB-2DF7-4294-9498-95E54E6B49CC@FreeBSD.org> <CAG=rPVdP_TAmerKh_82EravEw=sCj4LyEBu-NfGCK0Obg=HMVw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Mar 22, 2015 at 3:01 PM, Craig Rodrigues <rodrigc@freebsd.org>
wrote:
>
>
> On Sun, Mar 22, 2015 at 2:36 PM, Dimitry Andric <dim@freebsd.org> wrote:
>
>> On 22 Mar 2015, at 22:32, Craig Rodrigues <rodrigc@FreeBSD.org> wrote:
>> >
>> > On Sun, Mar 22, 2015 at 2:29 PM, Dimitry Andric <dim@freebsd.org>
>> wrote:
>> >
>> > Ah right, that was on i386, on amd64 it does result in -2^63. It is
>> indeed caused by reliance on signed integer wrapping.
>> >
>> > This diff should fix it, without rewriting the utility:
>> >
>> > Index: bin/expr/Makefile
>> > ===================================================================
>> > --- bin/expr/Makefile (revision 280156)
>> > +++ bin/expr/Makefile (working copy)
>> > @@ -6,6 +6,9 @@ PROG= expr
>> > SRCS= expr.y
>> > YFLAGS=
>> >
>> > +# expr relies on signed integer wrapping
>> > +CFLAGS+= -fwrapv
>> > +
>> > NO_WMISSING_VARIABLE_DECLARATIONS=
>> >
>> > .if ${MK_TESTS} != "no"
>> >
>> >
>> > Well, another alternative is to patch expr.y:
>> >
>> > Index: expr.y
>> > ===================================================================
>> > --- expr.y (revision 280353)
>> > +++ expr.y (working copy)
>> > @@ -393,7 +393,7 @@
>> > }
>> >
>> > void
>> > -assert_plus(intmax_t a, intmax_t b, intmax_t r)
>> > +assert_plus(intmax_t a, intmax_t b, volatile intmax_t r)
>> > {
>> > /*
>> > * sum of two positive numbers must be positive,
>> > @@ -420,7 +420,7 @@
>> > }
>> >
>> > void
>> > -assert_minus(intmax_t a, intmax_t b, intmax_t r)
>> > +assert_minus(intmax_t a, intmax_t b, volatile intmax_t r)
>> > {
>> > /* special case subtraction of INTMAX_MIN */
>> > if (b == INTMAX_MIN && a < 0)
>> >
>> >
>> > There were already some patches previously done to this
>> > file to add "volatile", so maybe this would be OK to do.
>> >
>> > What do you think?
>>
>> Volatile is not the solution, it is completely orthogonal. The correct
>> way would be to use unsigned integers, for which wrapping is defined,
>> then convert those back and forth when presenting the results to the
>> user.
>>
>
> OK, converting expr.y to use unsigned integers would require a bit of work.
>
> Can you commit your patch to the Makefile? It fixes the problem for now.
>
>
Thanks for committing the fix. I wasn't aware of this topic, but it is
explained
quite nicely in this LLVM blog post:
http://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html#signed_overflow
Do you think we should further change expr.y with something like this:
Index: expr.y
===================================================================
--- expr.y (revision 280357)
+++ expr.y (working copy)
@@ -445,12 +445,13 @@
}
/*
- * We depend on undefined behaviour giving a result (in r).
- * To test this result, pass it as volatile. This prevents
- * optimizing away of the test based on the undefined behaviour.
+ * We depend on undefined signed integer overflow behaviour
+ * giving a result (in r).
+ * This file must be compiled with the "-fwrapv" compiler
+ * flag which forces defined behavior for signed integer overflow.
*/
void
-assert_times(intmax_t a, intmax_t b, volatile intmax_t r)
+assert_times(intmax_t a, intmax_t b, intmax_t r)
{
/*
* If the first operand is 0, no overflow is possible,
--
Craig
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAG=rPVcTa3Cni8uO9AfT0UPBF015_yb_%2B%2BzJz%2BE132hM_Cs9sQ>