Re: Jenkins build is still unstable: FreeBSD_HEAD-tests2 #867

On Sun, Mar 22, 2015 at 2:36 PM, Dimitry Andric <dim_at_freebsd.org> wrote:

> On 22 Mar 2015, at 22:32, Craig Rodrigues <rodrigc_at_FreeBSD.org> wrote:
> >
> > On Sun, Mar 22, 2015 at 2:29 PM, Dimitry Andric <dim_at_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)
> > _at__at_ -6,6 +6,9 _at__at_ 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)
> > _at__at_ -393,7 +393,7 _at__at_
> >  }
> >
> >  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,
> > _at__at_ -420,7 +420,7 _at__at_
> >  }
> >
> >  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.

--
Craig