From df8390790aae6025e95687a37eea81a4757966d0 Mon Sep 17 00:00:00 2001 From: Paul Yang Date: Thu, 10 Nov 2016 11:20:50 -0800 Subject: [PATCH] Fix php c extension on 32-bit machines. (#2348) --- php/ext/google/protobuf/protobuf.h | 3 + php/ext/google/protobuf/storage.c | 24 +++- php/ext/google/protobuf/type_check.c | 139 +++++++++++++++++-- php/src/Google/Protobuf/Internal/Message.php | 10 ++ php/tests/test.sh | 3 +- php/tests/test_base.php | 18 ++- tests.sh | 7 + 7 files changed, 187 insertions(+), 17 deletions(-) diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index 93027bc1e..fb5879dcb 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -39,6 +39,9 @@ #define PHP_PROTOBUF_EXTNAME "protobuf" #define PHP_PROTOBUF_VERSION "3.1.0a1" +#define MAX_LENGTH_OF_INT64 20 +#define SIZEOF_INT64 8 + // ----------------------------------------------------------------------------- // Forward Declaration // ---------------------------------------------------------------------------- diff --git a/php/ext/google/protobuf/storage.c b/php/ext/google/protobuf/storage.c index e94aa3196..1d25a91bb 100644 --- a/php/ext/google/protobuf/storage.c +++ b/php/ext/google/protobuf/storage.c @@ -174,11 +174,31 @@ CASE(FLOAT, DOUBLE, float) CASE(DOUBLE, DOUBLE, double) CASE(BOOL, BOOL, int8_t) CASE(INT32, LONG, int32_t) -CASE(INT64, LONG, int64_t) -CASE(UINT64, LONG, uint64_t) CASE(ENUM, LONG, uint32_t) #undef CASE + +#if SIZEOF_LONG == 4 +#define CASE(upb_type, c_type) \ + case UPB_TYPE_##upb_type: { \ + SEPARATE_ZVAL_IF_NOT_REF(cache); \ + char buffer[MAX_LENGTH_OF_INT64]; \ + sprintf(buffer, "%lld", DEREF(memory, c_type)); \ + ZVAL_STRING(*cache, buffer, 1); \ + return; \ + } +#else +#define CASE(upb_type, c_type) \ + case UPB_TYPE_##upb_type: { \ + SEPARATE_ZVAL_IF_NOT_REF(cache); \ + ZVAL_LONG(*cache, DEREF(memory, c_type)); \ + return; \ + } +#endif +CASE(UINT64, uint64_t) +CASE(INT64, int64_t) +#undef CASE + case UPB_TYPE_UINT32: { // Prepend bit-1 for negative numbers, so that uint32 value will be // consistent on both 32-bit and 64-bit architectures. diff --git a/php/ext/google/protobuf/type_check.c b/php/ext/google/protobuf/type_check.c index c215d72ef..d12d00258 100644 --- a/php/ext/google/protobuf/type_check.c +++ b/php/ext/google/protobuf/type_check.c @@ -34,6 +34,7 @@ #include "utf8.h" static zend_class_entry* util_type; +static const char int64_min_digits[] = "9223372036854775808"; ZEND_BEGIN_ARG_INFO_EX(arg_check_optional, 0, 0, 1) ZEND_ARG_INFO(1, val) @@ -78,8 +79,128 @@ void util_init(TSRMLS_D) { // Type checking/conversion. // ----------------------------------------------------------------------------- +// This is modified from is_numeric_string in zend_operators.h. The behavior of +// this function is the same as is_numeric_string, except that this takes +// int64_t as input instead of long. +static zend_uchar convert_numeric_string( + const char *str, int length, int64_t *lval, double *dval) { + const char *ptr; + int base = 10, digits = 0, dp_or_e = 0; + double local_dval = 0.0; + zend_uchar type; + + if (length == 0) { + return IS_NULL; + } + + while (*str == ' ' || *str == '\t' || *str == '\n' || + *str == '\r' || *str == '\v' || *str == '\f') { + str++; + length--; + } + ptr = str; + + if (*ptr == '-' || *ptr == '+') { + ptr++; + } + + if (ZEND_IS_DIGIT(*ptr)) { + // Handle hex numbers + // str is used instead of ptr to disallow signs and keep old behavior. + if (length > 2 && *str == '0' && (str[1] == 'x' || str[1] == 'X')) { + base = 16; + ptr += 2; + } + + // Skip any leading 0s. + while (*ptr == '0') { + ptr++; + } + + // Count the number of digits. If a decimal point/exponent is found, + // it's a double. Otherwise, if there's a dval or no need to check for + // a full match, stop when there are too many digits for a int64 */ + for (type = IS_LONG; + !(digits >= MAX_LENGTH_OF_INT64 && dval); + digits++, ptr++) { +check_digits: + if (ZEND_IS_DIGIT(*ptr) || (base == 16 && ZEND_IS_XDIGIT(*ptr))) { + continue; + } else if (base == 10) { + if (*ptr == '.' && dp_or_e < 1) { + goto process_double; + } else if ((*ptr == 'e' || *ptr == 'E') && dp_or_e < 2) { + const char *e = ptr + 1; + + if (*e == '-' || *e == '+') { + ptr = e++; + } + if (ZEND_IS_DIGIT(*e)) { + goto process_double; + } + } + } + break; + } + + if (base == 10) { + if (digits >= MAX_LENGTH_OF_INT64) { + dp_or_e = -1; + goto process_double; + } + } else if (!(digits < SIZEOF_INT64 * 2 || + (digits == SIZEOF_INT64 * 2 && ptr[-digits] <= '7'))) { + if (dval) { + local_dval = zend_hex_strtod(str, &ptr); + } + type = IS_DOUBLE; + } + } else if (*ptr == '.' && ZEND_IS_DIGIT(ptr[1])) { +process_double: + type = IS_DOUBLE; + + // If there's a dval, do the conversion; else continue checking + // the digits if we need to check for a full match. + if (dval) { + local_dval = zend_strtod(str, &ptr); + } else if (dp_or_e != -1) { + dp_or_e = (*ptr++ == '.') ? 1 : 2; + goto check_digits; + } + } else { + return IS_NULL; + } + if (ptr != str + length) { + zend_error(E_NOTICE, "A non well formed numeric value encountered"); + return 0; + } + + if (type == IS_LONG) { + if (digits == MAX_LENGTH_OF_INT64 - 1) { + int cmp = strcmp(&ptr[-digits], int64_min_digits); + + if (!(cmp < 0 || (cmp == 0 && *str == '-'))) { + if (dval) { + *dval = zend_strtod(str, NULL); + } + + return IS_DOUBLE; + } + } + if (lval) { + *lval = strtoll(str, NULL, base); + } + return IS_LONG; + } else { + if (dval) { + *dval = local_dval; + } + return IS_DOUBLE; + } +} + #define CONVERT_TO_INTEGER(type) \ - static bool convert_long_to_##type(long val, type##_t* type##_value) { \ + static bool convert_int64_to_##type(int64_t val, type##_t* type##_value) { \ *type##_value = (type##_t)val; \ return true; \ } \ @@ -91,15 +212,15 @@ void util_init(TSRMLS_D) { \ static bool convert_string_to_##type(const char* val, int len, \ type##_t* type##_value) { \ - long lval; \ + int64_t lval; \ double dval; \ \ - switch (is_numeric_string(val, len, &lval, &dval, 0)) { \ + switch (convert_numeric_string(val, len, &lval, &dval)) { \ case IS_DOUBLE: { \ return convert_double_to_##type(dval, type##_value); \ } \ case IS_LONG: { \ - return convert_long_to_##type(lval, type##_value); \ + return convert_int64_to_##type(lval, type##_value); \ } \ default: \ zend_error(E_USER_ERROR, \ @@ -111,7 +232,7 @@ void util_init(TSRMLS_D) { bool protobuf_convert_to_##type(zval* from, type##_t* to) { \ switch (Z_TYPE_P(from)) { \ case IS_LONG: { \ - return convert_long_to_##type(Z_LVAL_P(from), to); \ + return convert_int64_to_##type(Z_LVAL_P(from), to); \ } \ case IS_DOUBLE: { \ return convert_double_to_##type(Z_DVAL_P(from), to); \ @@ -137,7 +258,7 @@ CONVERT_TO_INTEGER(uint64); #undef CONVERT_TO_INTEGER #define CONVERT_TO_FLOAT(type) \ - static bool convert_long_to_##type(long val, type* type##_value) { \ + static bool convert_int64_to_##type(int64_t val, type* type##_value) { \ *type##_value = (type)val; \ return true; \ } \ @@ -149,10 +270,10 @@ CONVERT_TO_INTEGER(uint64); \ static bool convert_string_to_##type(const char* val, int len, \ type* type##_value) { \ - long lval; \ + int64_t lval; \ double dval; \ \ - switch (is_numeric_string(val, len, &lval, &dval, 0)) { \ + switch (convert_numeric_string(val, len, &lval, &dval)) { \ case IS_DOUBLE: { \ *type##_value = (type)dval; \ return true; \ @@ -171,7 +292,7 @@ CONVERT_TO_INTEGER(uint64); bool protobuf_convert_to_##type(zval* from, type* to) { \ switch (Z_TYPE_P(from)) { \ case IS_LONG: { \ - return convert_long_to_##type(Z_LVAL_P(from), to); \ + return convert_int64_to_##type(Z_LVAL_P(from), to); \ } \ case IS_DOUBLE: { \ return convert_double_to_##type(Z_DVAL_P(from), to); \ diff --git a/php/src/Google/Protobuf/Internal/Message.php b/php/src/Google/Protobuf/Internal/Message.php index 38513e914..3d1f1598c 100644 --- a/php/src/Google/Protobuf/Internal/Message.php +++ b/php/src/Google/Protobuf/Internal/Message.php @@ -125,6 +125,16 @@ class Message $oneof = $this->desc->getOneofDecl()[$field->getOneofIndex()]; $oneof_name = $oneof->getName(); $this->$oneof_name = new OneofField($oneof); + } else if ($field->getLabel() === GPBLabel::OPTIONAL && + PHP_INT_SIZE == 4) { + switch ($field->getType()) { + case GPBType::INT64: + case GPBType::UINT64: + case GPBType::FIXED64: + case GPBType::SFIXED64: + case GPBType::SINT64: + $this->$setter("0"); + } } } } diff --git a/php/tests/test.sh b/php/tests/test.sh index 888e93eb0..fe3dc7f64 100755 --- a/php/tests/test.sh +++ b/php/tests/test.sh @@ -7,7 +7,8 @@ pushd ../ext/google/protobuf/ make clean set -e -phpize && ./configure --enable-debug CFLAGS='-g -O0' && make +# Add following in configure for debug: --enable-debug CFLAGS='-g -O0' +phpize && ./configure && make popd tests=( array_test.php encode_decode_test.php generated_class_test.php map_field_test.php ) diff --git a/php/tests/test_base.php b/php/tests/test_base.php index 498860506..d461f0f78 100644 --- a/php/tests/test_base.php +++ b/php/tests/test_base.php @@ -75,20 +75,28 @@ class TestBase extends PHPUnit_Framework_TestCase { $this->assertSame(0, $m->getOptionalInt32()); $this->assertSame(0, $m->getOptionalUint32()); - $this->assertSame(0, $m->getOptionalInt64()); - $this->assertSame(0, $m->getOptionalUint64()); $this->assertSame(0, $m->getOptionalSint32()); - $this->assertSame(0, $m->getOptionalSint64()); $this->assertSame(0, $m->getOptionalFixed32()); - $this->assertSame(0, $m->getOptionalFixed64()); $this->assertSame(0, $m->getOptionalSfixed32()); - $this->assertSame(0, $m->getOptionalSfixed64()); $this->assertSame(0.0, $m->getOptionalFloat()); $this->assertSame(0.0, $m->getOptionalDouble()); $this->assertSame(false, $m->getOptionalBool()); $this->assertSame('', $m->getOptionalString()); $this->assertSame('', $m->getOptionalBytes()); $this->assertNull($m->getOptionalMessage()); + if (PHP_INT_SIZE == 4) { + $this->assertSame("0", $m->getOptionalInt64()); + $this->assertSame("0", $m->getOptionalUint64()); + $this->assertSame("0", $m->getOptionalSint64()); + $this->assertSame("0", $m->getOptionalFixed64()); + $this->assertSame("0", $m->getOptionalSfixed64()); + } else { + $this->assertSame(0, $m->getOptionalInt64()); + $this->assertSame(0, $m->getOptionalUint64()); + $this->assertSame(0, $m->getOptionalSint64()); + $this->assertSame(0, $m->getOptionalFixed64()); + $this->assertSame(0, $m->getOptionalSfixed64()); + } } // This test is to avoid the warning of no test by php unit. diff --git a/tests.sh b/tests.sh index 7a12f267d..4ac6b7c68 100755 --- a/tests.sh +++ b/tests.sh @@ -393,6 +393,12 @@ build_php5.5_32() { ./vendor/bin/phpunit } +build_php5.5_c_32() { + use_php_bc 5.5 + wget https://phar.phpunit.de/phpunit-old.phar -O /usr/bin/phpunit + cd php/tests && /bin/bash ./test.sh && cd ../.. +} + build_php5.6() { use_php 5.6 rm -rf vendor @@ -449,6 +455,7 @@ build_php_all() { build_php_all_32() { build_php5.5_32 + build_php5.5_c_32 } # Note: travis currently does not support testing more than one language so the