php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #77519 Regression in updating PHP arrays by functions provided by extensions.
Submitted: 2019-01-25 14:35 UTC Modified: 2019-01-25 19:45 UTC
From: chris dot e dot munt at gmail dot com Assigned:
Status: Not a bug Package: Arrays related
PHP Version: 7.3.1 OS: Ubuntu 18.04; CentOS 7.3; Window
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: chris dot e dot munt at gmail dot com
New email:
PHP Version: OS:

 

 [2019-01-25 14:35 UTC] chris dot e dot munt at gmail dot com
Description:
------------
There appears to be a regression (between PHP 7.2.x and 7.3.x) involving arrays passed by reference between PHP script and extensions.

I have not been able to get a back-trace but include a very simple example that demonstrates the problem, together with the results obtained for 7.2.12 (expected) and the crash that occurs when the same code is run through 7.3.1.

The flow is that an array is initialized in PHP script using 'array()', then passed as an input argument to an extension function which attempts to add records to that array.  This worked fine for all PHP versions prior to 7.3.  The extension code is included below.

Interestingly, if the initialization of the array is coded as 'array(0)' then the script runs, albeit with an extra '0' record in the data.   It would appear that something isn't being initialized properly within the PHP 7.3 engine when the 'array()' construct is processed.

Extension C Code
================

#include "php.h"
#include "ext/standard/info.h"
#include "SAPI.h"
#include "php_ini.h"

#define PHP_ATEST_EXTNAME "atest"
#define PHP_ATEST_VERSION "1.0.0"

static PHP_FUNCTION(atest_function);

static const zend_function_entry atest_functions[] =
{
    PHP_FE(atest_function, NULL)
    {NULL, NULL, NULL}
};

zend_module_entry atest_module_entry =
{
   STANDARD_MODULE_HEADER,
   PHP_ATEST_EXTNAME,
   atest_functions,
   NULL,
   NULL,
   NULL,
   NULL,
   NULL,
   PHP_ATEST_VERSION,
   STANDARD_MODULE_PROPERTIES
};

ZEND_GET_MODULE(atest)

ZEND_FUNCTION(atest_function)
{
   int argument_count;
   zval parameter_array_a[32] = {0}, *parameter_array = parameter_array_a;

   zend_printf("\r\natest_function() PHP Version: %d.%d.%d\r\n", PHP_MAJOR_VERSION, PHP_MINOR_VERSION, PHP_RELEASE_VERSION);

   argument_count = ZEND_NUM_ARGS();

   if (argument_count < 1)
      WRONG_PARAM_COUNT;

   if(zend_get_parameters_array_ex(argument_count, parameter_array) != SUCCESS)
      WRONG_PARAM_COUNT;

   add_assoc_string(&(parameter_array_a[0]), "1", "value 1");
   add_assoc_string(&(parameter_array_a[0]), "2", "value 2");
   add_assoc_string(&(parameter_array_a[0]), "3", "value 3");

   RETVAL_NULL();
}



Test script:
---------------
<html>
<head><title>Array test</title></head>
<?php
   $arr = array();
   atest_function($arr);
   print("\r\n<p>Array passed by reference ...<p>\r\n");
   foreach($arr as $k1=>$v1) {
      echo "\r\n<br>",$k1," = ",$v1;
   }
?>
</html>


Expected result:
----------------
Result for v7.2.12
==================

<html>
<head><title>Array test</title></head>

atest_function() PHP Version: 7.2.12

<p>Array passed by reference ...<p>

<br>1 = value 1
<br>2 = value 2
<br>3 = value 3</html>


Result for v7.3.1
=================

<html>
<head><title>Array test</title></head>

atest_function() PHP Version: 7.3.1
Segmentation fault (core dumped)



Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2019-01-25 15:09 UTC] chris dot e dot munt at gmail dot com
A 'gdb' back-trace from the crash ...


<html>
<head><title>Array test</title></head>

atest_function() PHP Version: 7.3.1

Program received signal SIGSEGV, Segmentation fault.
zend_hash_real_init_packed_ex (ht=0x5555563956a0 <zend_empty_array>)
    at /opt/php-7.3.1/Zend/zend_hash.c:123
123             HT_FLAGS(ht) |= HASH_FLAG_INITIALIZED | HASH_FLAG_PACKED;
(gdb) bt
#0  zend_hash_real_init_packed_ex (ht=0x5555563956a0 <zend_empty_array>)
    at /opt/php-7.3.1/Zend/zend_hash.c:123
#1  _zend_hash_index_add_or_update_i (flag=1, pData=0x7fffffffb840, h=1,
    ht=0x5555563956a0 <zend_empty_array>, ht@entry=0x1)
    at /opt/php-7.3.1/Zend/zend_hash.c:955
#2  zend_hash_index_update (ht=ht@entry=0x5555563956a0 <zend_empty_array>,
    h=1, pData=pData@entry=0x7fffffffb840)
    at /opt/php-7.3.1/Zend/zend_hash.c:1016
#3  0x0000555555998e7b in zend_symtable_str_update (pData=0x7fffffffb840,
    len=1, str=0x7ffff39dea33 "1", ht=0x5555563956a0 <zend_empty_array>)
    at /opt/php-7.3.1/Zend/zend_hash.h:496
#4  add_assoc_string_ex (arg=0x7fffffffb8c0, key=0x7ffff39dea33 "1",
    key_len=1, str=0x7ffff39dea2b "value 1")
    at /opt/php-7.3.1/Zend/zend_API.c:1419
#5  0x00007ffff39de943 in zif_atest_function ()
   from /usr/local/lib/php/extensions/no-debug-zts-20180731/php731_atest.so
#6  0x0000555555a28374 in ZEND_DO_ICALL_SPEC_RETVAL_UNUSED_HANDLER ()
    at /opt/php-7.3.1/Zend/zend_vm_execute.h:645
#7  execute_ex (ex=0x7ffff3e00040)
    at /opt/php-7.3.1/Zend/zend_vm_execute.h:55414
#8  0x0000555555a2d7c3 in zend_execute (op_array=<optimized out>,
    return_value=<optimized out>)
    at /opt/php-7.3.1/Zend/zend_vm_execute.h:60834
---Type <return> to continue, or q <return> to quit---
#9  0x0000555555996be2 in zend_execute_scripts (type=type@entry=8,
    retval=retval@entry=0x0, file_count=-203300816, file_count@entry=3)
    at /opt/php-7.3.1/Zend/zend.c:1568
#10 0x0000555555928c3e in php_execute_script (primary_file=0x7fffffffd140)
    at /opt/php-7.3.1/main/main.c:2630
#11 0x0000555555a2ff93 in do_cli (argc=2, argv=0x5555563c59f0)
    at /opt/php-7.3.1/sapi/cli/php_cli.c:997
#12 0x000055555565fbb1 in main (argc=2, argv=0x5555563c59f0)
    at /opt/php-7.3.1/sapi/cli/php_cli.c:1389
(gdb)
 [2019-01-25 15:49 UTC] nikic@php.net
-Status: Open +Status: Not a bug
 [2019-01-25 15:49 UTC] nikic@php.net
There's quite a few things wrong with this code. Here are some pointers:

* If you want to modify a parameter, you need to accept it by reference. This is done by adding arginfo information. It will look something like this:

ZEND_BEGIN_ARGINFO_EX(arginfo_atest, 0, 0, 1)
    ZEND_ARG_INFO(1, parameter_name) // The 1 here means by-reference
ZEND_END_ARGINFO()

* While this is not directly related to your problem, instead of using zend_get_parameters_array_ex(), you should generally use the zend_parse_parameters or FastZPP APIs. For example:

zval *array;
if (zend_parse_parameters(ZEND_NUM_ARGS(), "a/", &array) == FAILURE) {
    return;
}

* Before you modify an array, it is necessary to separate it, because it might be used in multiple places. In your particular case you are trying to modify a shared empty array that lives in read-only memory, thus causing a segmentation fault. You can perform the separation using SEPARATE_ARRAY(array). The separation can also be directly performed in the zend_parse_parameters call: This is what the "/" in the above example does.

Finally, if you are developing extensions, you should build PHP using --enable-debug. This will enable assertions that would have notified you of these problems on earlier PHP versions as well. You just got lucky and this is segfaulting on 7.3 rather than silently corrupting data.

Hope that helps.
 [2019-01-25 18:48 UTC] chris dot e dot munt at gmail dot com
Many thanks for the response.

I have added the 'ZEND_BEGIN_ARG_INFO_EX' section as recommended and, using the same array.php script, I can confirm that the following does work ...


   zval *parameter_array;
   if (zend_parse_parameters(ZEND_NUM_ARGS(), "a/", &parameter_array) == FAILURE) {
      return;
   }
   add_assoc_string(parameter_array, "1", "value 1");


However, the following does not ...


   zval parameter_array_a[32] = {0}, *parameter_array = parameter_array_a;
   if(zend_get_parameters_array_ex(argument_count, parameter_array) == FAILURE) {
      return;
   }
   SEPARATE_ARRAY(&parameter_array_a[0]);
   add_assoc_string(&parameter_array_a[0], "1", "value 1");


The call to add_assoc_string() terminates the script with the following message written to the console ...

HP Fatal error:  Allowed memory size of 134217728 bytes exhausted (tried to allocate 111803507344 bytes) in array.php on line 5

I appreciate that 'zend_get_parameters_array_ex()' isn't recommended for some reason but the actual project is an old extension of 2002 vintage.  This being the case it seems like we've been lucky for quite a long time.  Anyway, in the real project, the PHP array is not always passed to the extension as the same argument number - hence the use of an arguments array that is then parsed programmatically to determine the individual argument types.
 [2019-01-25 19:45 UTC] nikic@php.net
As you now specify that the value should be passed by reference, you receive a reference value rather than an array, which first needs to be dereferenced to access the array it contains:


   zval parameter_array_a[32] = {0}, *parameter_array = parameter_array_a;
   zval *array;
   if(zend_get_parameters_array_ex(argument_count, parameter_array) == FAILURE) {
      return;
   }

   array = &parameter_array_a[0];
   ZVAL_DEREF(array);
   SEPARATE_ARRAY(array);
   add_assoc_string(array, "1", "value 1");

The zend_parse_parameters variant already includes an implicit DEREF.
 
PHP Copyright © 2001-2020 The PHP Group
All rights reserved.
Last updated: Tue Mar 31 19:01:23 2020 UTC