From f77650ac2a5fdba0da1ae1e25e22c7a6a4c1c2ed Mon Sep 17 00:00:00 2001 From: Tyson Andre Date: Sat, 1 Aug 2020 13:01:31 -0400 Subject: [PATCH 1/2] [Proposal] Warn about the loss of precision in binary literals Emit an E_COMPILE_WARNING if these are seen in hexadecimal, octal, or binary literals. - E_COMPILE_WARNING is also emitted for "Octal escape sequence overflow" but it's been long enough to consider changing that. See GH-4758. - Making this proposal suddenly a ParseError in php 8.1 seems too soon. I expect this to behave the same on 32-bit and 64-bit builds (floats are 64 bits on both) For example, `0xffff_ffff_ffff_f400` overflows and becomes the **float** `0xffff_ffff_ffff_f000`. This PR will warn about that. Instead of using `0xffff_ffff_ffff_f400` with binary bitwise operands, most code should use the signed 64-bit int `~0xbff`. - E.g. PHP code ported from cryptography algorithms or other C code doing bitwise operations. --- Zend/tests/binary.phpt | 13 ++- Zend/tests/binary_overflow_number.phpt | 42 +++++++++ Zend/tests/hex_overflow_number.phpt | 39 ++++++++ Zend/tests/octal_overflow_number.phpt | 27 ++++++ Zend/zend_language_scanner.l | 91 +++++++++++++++++++ ext/standard/tests/strings/pack64.phpt | 11 ++- .../tests/invalid_octal_dnumber.phpt | 13 ++- 7 files changed, 230 insertions(+), 6 deletions(-) create mode 100644 Zend/tests/binary_overflow_number.phpt create mode 100644 Zend/tests/hex_overflow_number.phpt create mode 100644 Zend/tests/octal_overflow_number.phpt diff --git a/Zend/tests/binary.phpt b/Zend/tests/binary.phpt index 2dcbf184136d0..38a3fec8a55a2 100644 --- a/Zend/tests/binary.phpt +++ b/Zend/tests/binary.phpt @@ -79,7 +79,16 @@ var_dump(-0b1111111111111111111111111111111111111111111111111111111111111111); var_dump(-0b111111111111111111111111111111111111111111111111111111111111111); var_dump(-0b11111111111111111111111111111111111111111111111111111111111111); var_dump(-0b1); ---EXPECT-- +--EXPECTF-- +Warning: Saw imprecise float binary literal - the last 11 non-zero bits were truncated in %sbinary.php on line 66 + +Warning: Saw imprecise float binary literal - the last 11 non-zero bits were truncated in %sbinary.php on line 67 + +Warning: Saw imprecise float binary literal - the last 12 non-zero bits were truncated in %sbinary.php on line 68 + +Warning: Saw imprecise float binary literal - the last 12 non-zero bits were truncated in %sbinary.php on line 69 + +Warning: Saw imprecise float binary literal - the last 11 non-zero bits were truncated in %sbinary.php on line 71 int(1) int(3) int(7) @@ -151,4 +160,4 @@ float(3.68934881474191E+19) float(-1.844674407370955E+19) int(-9223372036854775807) int(-4611686018427387903) -int(-1) +int(-1) \ No newline at end of file diff --git a/Zend/tests/binary_overflow_number.phpt b/Zend/tests/binary_overflow_number.phpt new file mode 100644 index 0000000000000..570f88eec4047 --- /dev/null +++ b/Zend/tests/binary_overflow_number.phpt @@ -0,0 +1,42 @@ +--TEST-- +Octal overflow in numeric literal warning +--FILE-- += str); + } + if ('0' <= *end && *end <= '9') { + last_digit = *end - '0'; + } else if ('a' <= *end && *end <= 'f') { + last_digit = *end - 'a' + 10; + } else { + ZEND_ASSERT('A' <= *end && *end <= 'F'); + last_digit = *end - 'A' + 10; + } + if ((last_digit & 1) == 0) { + bits--; + if ((last_digit & 2) == 0) { + bits--; + if ((last_digit & 4) == 0) { + bits--; + } + } + } + /* Check how many bits the first character started with */ + if (*str < '2') { + bits -= 3; + } else if (*str < '4') { + bits -= 2; + } else if (*str < '8') { + bits -= 1; + } + return bits; +} + +/* Get the number of bits in the representation of an octal literal. Precondition: *str represents a non-zero number that overflowed an int. */ +static size_t bits_in_octal_representation(const char *str, size_t len) +{ + size_t bits = len * 3; + const char *end = str + len - 1; + int last_digit; + while (*str == '0') { + bits -= 3; + str++; + ZEND_ASSERT(end >= str); + } + while (*end == '0') { + bits -= 3; + end--; + ZEND_ASSERT(end >= str); + } + last_digit = *end - '0'; + if ((last_digit & 1) == 0) { + bits--; + if ((last_digit & 2) == 0) { + bits--; + } + } + if (*str < '2') { + bits -= 2; + } else if (*str < '4') { + bits -= 1; + } + return bits; +} + + static size_t encoding_filter_script_to_internal(unsigned char **to, size_t *to_length, const unsigned char *from, size_t from_length) { const zend_encoding *internal_encoding = zend_multibyte_get_internal_encoding(); @@ -1961,9 +2032,17 @@ NEWLINE ("\r"|"\n"|"\r\n") } RETURN_TOKEN_WITH_VAL(T_LNUMBER); } else { + const char* last_one_bit = bin + len - 1; + while (*last_one_bit == '0') { + last_one_bit--; + ZEND_ASSERT(last_one_bit > bin); + } ZVAL_DOUBLE(zendlval, zend_bin_strtod(bin, (const char **)&end)); /* errno isn't checked since we allow HUGE_VAL/INF overflow */ ZEND_ASSERT(end == bin + len); + if (last_one_bit - bin + 1> 53) { + zend_error(E_COMPILE_WARNING, "Saw imprecise float binary literal - the last %zu non-zero bits were truncated", (size_t)(last_one_bit - bin + 1 - 53)); + } if (contains_underscores) { efree(bin); } @@ -1975,6 +2054,7 @@ NEWLINE ("\r"|"\n"|"\r\n") size_t len = yyleng; char *end, *lnum = yytext; zend_bool is_octal = lnum[0] == '0'; + zend_bool is_truncated = 0; zend_bool contains_underscores = (memchr(lnum, '_', len) != NULL); if (contains_underscores) { @@ -1998,6 +2078,7 @@ NEWLINE ("\r"|"\n"|"\r\n") /* Continue in order to determine if this is T_LNUMBER or T_DNUMBER. */ len = i; + is_truncated = 1; break; } } @@ -2016,6 +2097,12 @@ NEWLINE ("\r"|"\n"|"\r\n") errno = 0; if (is_octal) { /* octal overflow */ ZVAL_DOUBLE(zendlval, zend_oct_strtod(lnum, (const char **)&end)); + if (!is_truncated) { + size_t bits_in_representation = bits_in_octal_representation(lnum, len); + if (bits_in_representation > 53) { + zend_error(E_COMPILE_WARNING, "Saw imprecise float octal literal - the last %zu non-zero bits were truncated", bits_in_representation - 53); + } + } } else { ZVAL_DOUBLE(zendlval, zend_strtod(lnum, (const char **)&end)); } @@ -2066,9 +2153,13 @@ NEWLINE ("\r"|"\n"|"\r\n") } RETURN_TOKEN_WITH_VAL(T_LNUMBER); } else { + size_t bits_in_representation = bits_in_hex_representation(hex, len); ZVAL_DOUBLE(zendlval, zend_hex_strtod(hex, (const char **)&end)); /* errno isn't checked since we allow HUGE_VAL/INF overflow */ ZEND_ASSERT(end == hex + len); + if (bits_in_representation > 53) { + zend_error(E_COMPILE_WARNING, "Saw imprecise float hex literal - the last %zu non-zero bits were truncated", bits_in_representation - 53); + } if (contains_underscores) { efree(hex); } diff --git a/ext/standard/tests/strings/pack64.phpt b/ext/standard/tests/strings/pack64.phpt index cccdc1ba6e5f3..7184d1b94e680 100644 --- a/ext/standard/tests/strings/pack64.phpt +++ b/ext/standard/tests/strings/pack64.phpt @@ -32,7 +32,14 @@ print_r(unpack("q", pack("q", 0x8000000000000002))); print_r(unpack("q", pack("q", -1))); print_r(unpack("q", pack("q", 0x8000000000000000))); ?> ---EXPECT-- +--EXPECTF-- +Warning: Saw imprecise float hex literal - the last 10 non-zero bits were truncated in %spack64.php on line 4 + +Warning: Saw imprecise float hex literal - the last 10 non-zero bits were truncated in %spack64.php on line 10 + +Warning: Saw imprecise float hex literal - the last 10 non-zero bits were truncated in %spack64.php on line 16 + +Warning: Saw imprecise float hex literal - the last 10 non-zero bits were truncated in %spack64.php on line 22 Array ( [1] => 281474976710654 @@ -112,4 +119,4 @@ Array Array ( [1] => -9223372036854775808 -) +) \ No newline at end of file diff --git a/ext/tokenizer/tests/invalid_octal_dnumber.phpt b/ext/tokenizer/tests/invalid_octal_dnumber.phpt index fa75d79b064b2..0ed6239c11e34 100644 --- a/ext/tokenizer/tests/invalid_octal_dnumber.phpt +++ b/ext/tokenizer/tests/invalid_octal_dnumber.phpt @@ -4,7 +4,16 @@ Invalid octal number that overflows to double --FILE-- ---EXPECT-- +--EXPECTF-- T_DNUMBER +0177777777777777777777787 + +Warning: Saw imprecise float octal literal - the last 17 non-zero bits were truncated in %sinvalid_octal_dnumber.php on line 6 +0177777777777777777777777 From d479c63b3f2578d130604ef229d6f3dc5b32346b Mon Sep 17 00:00:00 2001 From: Tyson Andre Date: Mon, 3 Aug 2020 00:10:29 -0400 Subject: [PATCH 2/2] Fix miscellaneous test failures/BORKED The libxml borked warning was unrelated Fix 32-bit builds and soap tests. Make it so the same warnings would be emitted both for 32-bit and 64-bit builds --- Zend/tests/binary-32bit.phpt | 7 +++++-- Zend/tests/binary_overflow_number.phpt | 6 ++++-- Zend/tests/hex_overflow_32bit.phpt | 5 +++++ Zend/zend_language_scanner.l | 17 ++++++++++------- ext/libxml/tests/bug79191.phpt | 2 +- 5 files changed, 25 insertions(+), 12 deletions(-) diff --git a/Zend/tests/binary-32bit.phpt b/Zend/tests/binary-32bit.phpt index c66e863eb598a..c46cc02689b31 100644 --- a/Zend/tests/binary-32bit.phpt +++ b/Zend/tests/binary-32bit.phpt @@ -77,7 +77,10 @@ var_dump(-0b11111111111111111111111111111111); var_dump(-0b1111111111111111111111111111111); var_dump(-0b111111111111111111111111111111); var_dump(-0b1); ---EXPECT-- +--EXPECTF-- +Warning: Saw imprecise float binary literal - the last 11 non-zero bits were truncated in %sbinary-32bit.php on line 65 + +Warning: Saw imprecise float binary literal - the last 11 non-zero bits were truncated in %sbinary-32bit.php on line 67 int(1) int(3) int(7) @@ -149,4 +152,4 @@ float(-8589934591) float(-4294967295) int(-2147483647) int(-1073741823) -int(-1) +int(-1) \ No newline at end of file diff --git a/Zend/tests/binary_overflow_number.phpt b/Zend/tests/binary_overflow_number.phpt index 570f88eec4047..440a4d98b9e76 100644 --- a/Zend/tests/binary_overflow_number.phpt +++ b/Zend/tests/binary_overflow_number.phpt @@ -5,11 +5,11 @@ Octal overflow in numeric literal warning // rounding down var_dump(eval('return 0b1011111111111111111111111111111111111111111111111111100000000000;')); var_dump(eval('return 0b1011111111111111111111111111111111111111111111111111100000000001;')); -// rounding up +echo "test rounding up\n"; var_dump(eval('return 0b1011111111111111111111111111111111111111111111111111111111111111;')); var_dump(eval('return 0b1011111111111111111111111111111111111111111111111111110000000000;')); var_dump(eval('return 0b1011111111111111111111111111111111111111111111111111111000000000;')); -// don't count _ or leading 0s +echo "test don't count _ or leading 0s\n"; var_dump(eval('return 0b111_111_111_111_111_111_111_111_111_111_111_111_111_111_111_111_111_111_000_000_000_0;')); var_dump(eval('return 0b000_111_111_111_111_111_111_111_111_111_111_111_111_111_111_111_111_111_111_000_000_000_0;')); var_dump(eval('return 0b1111111111111111111111111111111111111111111111111111111111111111;')); @@ -19,6 +19,7 @@ float(1.383505805528216E+19) Warning: Saw imprecise float binary literal - the last 11 non-zero bits were truncated in %sbinary_overflow_number.php(4) : eval()'d code on line 1 float(1.383505805528216E+19) +test rounding up Warning: Saw imprecise float binary literal - the last 11 non-zero bits were truncated in %sbinary_overflow_number.php(6) : eval()'d code on line 1 float(1.3835058055282164E+19) @@ -28,6 +29,7 @@ float(1.3835058055282164E+19) Warning: Saw imprecise float binary literal - the last 2 non-zero bits were truncated in %sbinary_overflow_number.php(8) : eval()'d code on line 1 float(1.3835058055282164E+19) +test don't count _ or leading 0s Warning: Saw imprecise float binary literal - the last 1 non-zero bits were truncated in %sbinary_overflow_number.php(10) : eval()'d code on line 1 float(1.844674407370955E+19) diff --git a/Zend/tests/hex_overflow_32bit.phpt b/Zend/tests/hex_overflow_32bit.phpt index 71be6dc051529..cb39fa9c2cd04 100644 --- a/Zend/tests/hex_overflow_32bit.phpt +++ b/Zend/tests/hex_overflow_32bit.phpt @@ -22,6 +22,11 @@ foreach ($doubles as $d) { echo "Done\n"; ?> --EXPECTF-- +Warning: Saw imprecise float hex literal - the last 19 non-zero bits were truncated in %shex_overflow_32bit.php on line 5 + +Warning: Saw imprecise float hex literal - the last 51 non-zero bits were truncated in %shex_overflow_32bit.php on line 6 + +Warning: Saw imprecise float hex literal - the last 38 non-zero bits were truncated in %shex_overflow_32bit.php on line 7 float(4.0833602971%dE+14) float(4.7223664828%dE+21) float(1.3521606402%dE+31) diff --git a/Zend/zend_language_scanner.l b/Zend/zend_language_scanner.l index 2d109f4307084..04a317411ad4e 100644 --- a/Zend/zend_language_scanner.l +++ b/Zend/zend_language_scanner.l @@ -2033,14 +2033,15 @@ NEWLINE ("\r"|"\n"|"\r\n") RETURN_TOKEN_WITH_VAL(T_LNUMBER); } else { const char* last_one_bit = bin + len - 1; + ZVAL_DOUBLE(zendlval, zend_bin_strtod(bin, (const char **)&end)); + ZEND_ASSERT(end == bin + len); while (*last_one_bit == '0') { last_one_bit--; ZEND_ASSERT(last_one_bit > bin); } - ZVAL_DOUBLE(zendlval, zend_bin_strtod(bin, (const char **)&end)); /* errno isn't checked since we allow HUGE_VAL/INF overflow */ - ZEND_ASSERT(end == bin + len); - if (last_one_bit - bin + 1> 53) { + /* TODO: Cross-platform macro for 64-bit double */ + if (last_one_bit - bin + 1 > 53 && (SIZEOF_ZEND_LONG >= 8 || Z_DVAL_P(zendlval) >= (double)0x8000000000000000ULL)) { zend_error(E_COMPILE_WARNING, "Saw imprecise float binary literal - the last %zu non-zero bits were truncated", (size_t)(last_one_bit - bin + 1 - 53)); } if (contains_underscores) { @@ -2097,7 +2098,7 @@ NEWLINE ("\r"|"\n"|"\r\n") errno = 0; if (is_octal) { /* octal overflow */ ZVAL_DOUBLE(zendlval, zend_oct_strtod(lnum, (const char **)&end)); - if (!is_truncated) { + if (!is_truncated && (SIZEOF_ZEND_LONG >= 8 || Z_DVAL_P(zendlval) >= (double)0x8000000000000000ULL)) { size_t bits_in_representation = bits_in_octal_representation(lnum, len); if (bits_in_representation > 53) { zend_error(E_COMPILE_WARNING, "Saw imprecise float octal literal - the last %zu non-zero bits were truncated", bits_in_representation - 53); @@ -2153,12 +2154,14 @@ NEWLINE ("\r"|"\n"|"\r\n") } RETURN_TOKEN_WITH_VAL(T_LNUMBER); } else { - size_t bits_in_representation = bits_in_hex_representation(hex, len); ZVAL_DOUBLE(zendlval, zend_hex_strtod(hex, (const char **)&end)); /* errno isn't checked since we allow HUGE_VAL/INF overflow */ ZEND_ASSERT(end == hex + len); - if (bits_in_representation > 53) { - zend_error(E_COMPILE_WARNING, "Saw imprecise float hex literal - the last %zu non-zero bits were truncated", bits_in_representation - 53); + if (SIZEOF_ZEND_LONG >= 8 || Z_DVAL_P(zendlval) >= (double)0x8000000000000000ULL) { + size_t bits_in_representation = bits_in_hex_representation(hex, len); + if (bits_in_representation > 53) { + zend_error(E_COMPILE_WARNING, "Saw imprecise float hex literal - the last %zu non-zero bits were truncated", bits_in_representation - 53); + } } if (contains_underscores) { efree(hex); diff --git a/ext/libxml/tests/bug79191.phpt b/ext/libxml/tests/bug79191.phpt index 7d0dc83f232a0..e66773a9f154f 100644 --- a/ext/libxml/tests/bug79191.phpt +++ b/ext/libxml/tests/bug79191.phpt @@ -3,7 +3,7 @@ Bug #79191 (Error in SoapClient ctor disables DOMDocument::save()) --SKIPIF-- --FILE--