Fix #79015: undefined-behavior in php_date.c#5031
Conversation
We check that the given microsecond fraction is in the valid range [0, 1000000[, and otherwise mark it as invalid. We also drop the useless do loop; a plain block is sufficient here.
| do { | ||
| { | ||
| zval *z_arg = zend_hash_str_find(myht, "f", sizeof("f") - 1); | ||
| (*intobj)->diff->us = -1000000; |
There was a problem hiding this comment.
How did you pick this value? I see some other code in this file using -99999. @derickr What's the correct value for an unset us?
There was a problem hiding this comment.
Okay, it's clear how you picked it: Used in the existing code two lines below... Still, I'm not sure this is the right value, as TIMELIB_UNSET is -99999.
There was a problem hiding this comment.
Hmm, now I'm confused, since the other properties seem to use -1 as unset default? @derickr, please clarify. Thanks.
There was a problem hiding this comment.
I'm going to look into this when I return from Christmas holiday. Ping me the first week of Jan if I haven't replied yet
There was a problem hiding this comment.
-1000000 is correct, as in these lines:
4220 if (strcmp(Z_STRVAL_P(member), "f") == 0) {
4221 fvalue = obj->diff->us / 1000000.0;
4222 break;
4223 }
and
4238 if (fvalue != -1) {
4239 ZVAL_DOUBLE(retval, fvalue);
The fvalue ends up being -1 during the division, which would result in FALSE being returned.
|
Thanks! Applied as b48f262. |
3 participants
We check that the given microsecond fraction is in the valid range
[0, 1000000[, and otherwise mark it as invalid. We also drop the
useless do loop; a plain block is sufficient here.