-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Fix #79015: undefined-behavior in php_date.c #5031
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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.
zval *z_arg = zend_hash_str_find(myht, "f", sizeof("f") - 1); | ||
(*intobj)->diff->us = -1000000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ping ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-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. |
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.