Re: memory leak in the new power operator tests

Hi, On Fri, Feb 7, 2014 at 12:34 AM, Ferenc Kovacs <tyra3l@gmail.com> wrote: > > > > On Thu, Feb 6, 2014 at 4:40 PM, Ferenc Kovacs <tyra3l@gmail.com> wrote: > >> Hi, >> >> After Bob merged the pow related changes, executing the new tests with >> --enable-debug I've got two memory leaks: >> >> [tyrael@Ferencs-MacBook-Pro-135 php-src.git (PHP-5.6.0 ✗)]$ cat >> ext/standard/tests/math/pow_variation1_64bit.diff >> ext/standard/tests/math/pow_variation2.diff >> 083+ [Thu Feb 6 16:20:51 2014] Script: >> >> '/Users/tyrael/checkouts/php-src.git/ext/standard/tests/math/pow_variation1_64bit.php' >> 084+ /Users/tyrael/checkouts/php-src.git/Zend/zend_API.c(314) : Freeing >> 0x10D280C50 (72 bytes), >> script=/Users/tyrael/checkouts/php-src.git/ext/standard/tests/math/pow_variation1_64bit.php >> 085+ /Users/tyrael/checkouts/php-src.git/Zend/zend_variables.c(141) : >> Actual location (location was relayed) >> 086+ === Total 1 memory leaks detected ===083+ [Thu Feb 6 16:20:51 2014] >> Script: >> '/Users/tyrael/checkouts/php-src.git/ext/standard/tests/math/pow_variation2.php' >> 084+ /Users/tyrael/checkouts/php-src.git/Zend/zend_API.c(314) : Freeing >> 0x10CB8CBE0 (72 bytes), >> script=/Users/tyrael/checkouts/php-src.git/ext/standard/tests/math/pow_variation2.php >> 085+ /Users/tyrael/checkouts/php-src.git/Zend/zend_variables.c(141) : >> Actual location (location was relayed) >> 086+ === Total 1 memory leaks detected ===% >> >> not sure if these were originally in the PR or just something went wrong >> with the merge/cherry pick, but would be nice if somebody could look into >> it. >> thanks! >> >> -- >> Ferenc Kovács >> @Tyr43l - http://tyrael.hu >> > > it seems the leak occures when pow() is called with an array: > [tyrael@Ferencs-MacBook-Pro-135 php-src.git (PHP-5.6.0 ✗)]$ > ./sapi/cli/php -r 'pow(array(),2);' > [Thu Feb 6 17:32:43 2014] Script: '-' > Zend/zend_vm_execute.h(28951) : Freeing 0x1100B9ED8 (72 bytes), script=- > /Users/tyrael/checkouts/php-src.git/Zend/zend_API.c(1011) : Actual > location (location was relayed) > === Total 1 memory leaks detected === > Not sure why that didn't pop up earlier; I've attached a patch that resolves the memory leak. One gotcha: the output of pow(0, []) is now int(1) instead of float(1). I hope that's alright :) > > -- > Ferenc Kovács > @Tyr43l - http://tyrael.hu > -- -- Tjerk

diff --git a/Zend/zend_operators.c b/Zend/zend_operators.c index 03dbb15..970bd94 100644 --- a/Zend/zend_operators.c +++ b/Zend/zend_operators.c @@ -1025,13 +1025,15 @@ ZEND_API int pow_function(zval *result, zval *op1, zval *op2 TSRMLS_DC) if (!converted) { ZEND_TRY_BINARY_OBJECT_OPERATION(ZEND_POW); - if (Z_TYPE_P(op1) == IS_ARRAY) { - ZVAL_LONG(op1, 0); + if (Z_TYPE_P(op2) == IS_ARRAY) { + ZVAL_LONG(result, 1L); + return SUCCESS; } else { zendi_convert_scalar_to_number(op1, op1_copy, result); } - if (Z_TYPE_P(op2) == IS_ARRAY) { - ZVAL_LONG(op2, 0); + if (Z_TYPE_P(op1) == IS_ARRAY) { + ZVAL_LONG(result, 0); + return SUCCESS; } else { zendi_convert_scalar_to_number(op2, op2_copy, result); } diff --git a/ext/standard/tests/math/pow_variation2.phpt b/ext/standard/tests/math/pow_variation2.phpt index b1800bb..f571936 100644 --- a/ext/standard/tests/math/pow_variation2.phpt +++ b/ext/standard/tests/math/pow_variation2.phpt @@ -145,7 +145,7 @@ float(1) float(1) -- Iteration 19 -- -float(1) +int(1) -- Iteration 20 -- float(1) @@ -169,4 +169,4 @@ float(1) -- Iteration 26 -- %s -===Done=== \ No newline at end of file +===Done===