dlarchikov / php-amqplib

Automatically exported from code.google.com/p/php-amqplib
GNU Lesser General Public License v2.1
0 stars 0 forks source link

Test for 32/64-bit PHP is broken #7

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. Use a 32-bit PHP
2. See that it follows the 64-bit codepath

What is the expected output? What do you see instead?
Actual:
$ ../../php/php-5.2.10-32/sapi/cli/php all_tests.php 
PHP-AMQP tests (64bit)
OK
Test cases run: 1/1, Passes: 8, Failures: 0, Exceptions: 0
$ ../../php/php-5.2.10-64/sapi/cli/php all_tests.php 
PHP-AMQP tests (64bit)
OK
Test cases run: 1/1, Passes: 8, Failures: 0, Exceptions: 0

Expected:
$ ../../php/php-5.2.10-32/sapi/cli/php all_tests.php 
PHP-AMQP tests (32bit)
OK
Test cases run: 1/1, Passes: 8, Failures: 0, Exceptions: 0
$ ../../php/php-5.2.10-64/sapi/cli/php all_tests.php 
PHP-AMQP tests (64bit)
OK
Test cases run: 1/1, Passes: 8, Failures: 0, Exceptions: 0

What version of the product are you using? On what operating system?
OSX 10.5.8 on Core2Duo, self-compiled PHP 5.2.10 32-bit and 64-bit.

Please provide any additional information below.

There's code in amqp_wire.inc to detect negative return values from
AMQPReader::read_php_int(), so read_php_int() doesn't have to work hard on
its own.  As you can see from the testing below, casting the string
"3735928559" to an int on 32-bit results in 2**31-1 (clamped), which is not
correct or detectable.  The attached patch removes useless arch checking,
removes unhelpful sprintf()ing, and corrects the check in the test runner
(it's good to know what you're testing!).  The best answer to see if you're
on 32-bit or 64-bit seems to be to check PHP_INT_SIZE. :)

$ uname -mrsv
Darwin 9.8.0 Darwin Kernel Version 9.8.0: Wed Jul 15 16:55:01 PDT 2009;
root:xnu-1228.15.4~1/RELEASE_I386 i386

$ cat numtests.php
<?
var_dump("Casting 4294967296 to int", (int)4294967296);

var_dump("PHP_INT_SIZE", PHP_INT_SIZE);

list(,$a) = unpack("N", "\xDE\xAD\xBE\xEF");
$b = sprintf("%u",$a);
$c = (int)$b;
var_dump("Using unpack()", $a, $b, $c);

$ for x in *-32 *-64; do file $x/sapi/cli/php; $x/sapi/cli/php
numtests.php; done
php-5.2.10-32/sapi/cli/php: Mach-O executable i386
string(25) "Casting 4294967296 to int"
int(-1)
string(12) "PHP_INT_SIZE"
int(4)
string(14) "Using unpack()"
int(-559038737)
string(10) "3735928559"
int(2147483647)
php-5.2.10-64/sapi/cli/php: Mach-O 64-bit executable x86_64
string(25) "Casting 4294967296 to int"
int(4294967296)
string(12) "PHP_INT_SIZE"
int(8)
string(14) "Using unpack()"
int(3735928559)
string(10) "3735928559"
int(3735928559)

Original issue reported on code.google.com by taavi.burns on 8 Sep 2009 at 9:21

Attachments:

GoogleCodeExporter commented 8 years ago
That patch is against r24. :)

Original comment by taavi.burns on 8 Sep 2009 at 9:57

GoogleCodeExporter commented 8 years ago
I used the PHP_INT_SIZE check instead of the (int) cast as well, and then 
replaced
all the bcmath calls, for my own environment.  We don't use bcmath, and all
production servers are 64-bit, so it's just added bloat that shouldn't be 
needed. 
Maybe instead it should check if the 64-bit flag is false, before trying the 
bcmath
functions?  It's obvious why the check is needed, and why it needs bcmath on a 
32-bit
machine; but no need to throw an exception on a machine that is fully capable of
handling 64-bit math without the bc extension.

Original comment by madco...@gmail.com on 24 Nov 2009 at 11:06

GoogleCodeExporter commented 8 years ago
Lyolik please review these bugs.

Original comment by kroko...@gmail.com on 21 Mar 2010 at 9:17