Re: [ZEND-ENGINE-CVS] cvs: ZendEngine2(PHP_5_2) / zend_operators.c

[email protected] ("Matt Wilmas") Tue, 17 Feb 2009 08:58:31 -0600
Newsgroups php.zend-engine.cvs
Message-ID <00b701c99110$33d6cff0$0201a8c0@PC>
Hi Pierre,

----- Original Message -----
From: "Pierre Joye"
Sent: Tuesday, February 17, 2009

> hi Matt,
>
> On Tue, Feb 17, 2009 at 3:16 PM, Matt Wilmas <[email protected]> wrote:
>
> > @@ -936,7 +936,7 @@
> >                return FAILURE;                 /* modulus by zero */
> >        }
> >
> > -       if (abs(op2->value.lval) == 1) {
> > +       if (op2->value.lval == -1) {
> >                ZVAL_LONG(result, 0);
> >                return SUCCESS;
> >        }
>
> Is it not changing the behavior? What happened when lval was 1? It
> would be nice to have some tests too, if they do not exist already.

No, it's not changing the behavior, there's no problem with 1.  Like I said,
only the case of LONG_MIN % -1 causes an overflow.  This check is all that
was needed to fix the original bug #27354 (Modulus operator crashes PHP) and
I thought about changing it when I was fixing the same issue in
div_function() awhile back, but didn't want to change it "just because." :-)

As far as a test, I wasn't sure what to use, and would it just be for 64
bit?  The var_dump(1%4294967295) example in the report gives 0 here, because
of 32 bit overflow, which might not be the same everywhere...  So, I don't
know.

> Cheers,
> --
> Pierre

- Matt