php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #64966 segfault in zend_do_fcall_common_helper_SPEC
Submitted: 2013-06-03 19:41 UTC Modified: 2013-06-08 18:20 UTC
From: bfrance@php.net Assigned: laruence
Status: Closed Package: Scripting Engine problem
PHP Version: Irrelevant OS:
Private report: No CVE-ID:
 [2013-06-03 19:41 UTC] bfrance@php.net
Description:
------------
I don't think this is reflection related, as the issue started with this Exception patch:

zend_do_fcall_common_helper_SPEC does not handle exceptions properly
  
  https://bugs.php.net/bug.php?id=63914

but have do not have a good way to describe this bug, so I used the backtrace info.


5.3.24+  : core dumps
5.4.15+  : doesn't execute the code right (same with 5.5.0RC2)

Build and install either version of php with pear and intl support.  You will need icu installed (pkg: icu, libicu, libicu-devel) for intl support.  Install phpunit:

pear config-set auto_discover 1
pear install pear.phpunit.de/PHPUnit

cd php-5.x.x/ext/intl/tests

setup env:

% export TZ=US/Pacific
% export LANG=en_US.UTF-8
% export LC_ALL=

Copy test case:

curl -O http://www.brianfrance.com/php/phpIntlTest02.txt

mv phpIntlTest02.txt phpIntlTest02.php

php -dopen_basedir= /usr/local/bin/phpunit --log-junit results.xml phoIntlTest02.php


For 5.3.24+ you will get a core dump with the following backtrace:

#0  _zval_ptr_dtor (zval_ptr=0x7ffff7ebfe70) at php-5.3.24/Zend/zend_execute_API.c:441
#1  0x00000000007038a6 in zend_do_fcall_common_helper_SPEC (execute_data=0x7ffff7ebfa98) at php-5.3.24/Zend/zend_vm_execute.h:418
#2  0x00000000006dc948 in execute (op_array=0xfb6508) at php-5.3.24/Zend/zend_vm_execute.h:107
#3  0x00000000006ae1b0 in zend_call_function (fci=0x7fffffffaab0, fci_cache=<value optimized out>) at php-5.3.24/Zend/zend_execute_API.c:969
#4  0x0000000000583a8a in zim_reflection_method_invokeArgs (ht=<value optimized out>, return_value=0x115dab0, return_value_ptr=<value optimized out>, this_ptr=<value optimized out>, return_value_used=<value optimized out>)
    at php-5.3.24/ext/reflection/php_reflection.c:2753
#5  0x0000000000703d37 in zend_do_fcall_common_helper_SPEC (execute_data=0x7ffff7ebed68) at php-5.3.24/Zend/zend_vm_execute.h:322
#6  0x00000000006dc948 in execute (op_array=0x10f0d48) at php-5.3.24/Zend/zend_vm_execute.h:107
#7  0x00000000006b758a in zend_execute_scripts (type=8, retval=0x0, file_count=3) at php-5.3.24/Zend/zend.c:1259
#8  0x0000000000666ace in php_execute_script (primary_file=0x7fffffffe170) at php-5.3.24/main/main.c:2316
#9  0x000000000073de34 in main (argc=6, argv=0x7fffffffe3e8) at php-5.3.24/sapi/cli/php_cli.c:1189


For 5.4.15+ you get a weird code execution happening.  collator_sort is never called on line 17.  You can test this by gdb'ing and setting a break point on zif_collator_sort, it will never hit. It is like something triggered the exception before collator_sort is called.  This means that callator_sort didn't setup intl_get_error_message() error message about the bad param, so then the test fails on line 33.

This test works with 5.3.23 with no core dump and works with 5.3.24 if you revert the bug #63914 patch.


Test script:
---------------
http://www.brianfrance.com/php/phpIntlTest02.txt


Patches

bug64966.phpt (last revision 2013-06-08 09:19 UTC) by laruence@php.net)
bug64966.patch (last revision 2013-06-08 09:19 UTC) by laruence@php.net)
exception.diff (last revision 2013-06-07 19:53 UTC) by bfrance@php.net)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2013-06-04 03:52 UTC] laruence@php.net
-Status: Open +Status: Feedback
 [2013-06-04 03:52 UTC] laruence@php.net
could you refine this into one simple test script?
 [2013-06-04 04:32 UTC] bfrance@php.net
I wish I could and I have been trying. If you move line:

35                 $GLOBALS['oo-mode']=false;

in the test case to line 9, then the core dumps goes away with 5.3.x.  When I start trimming down the test case, the core dump goes away.  That is the smallest test case I have been able to come up with that core dumps 5.3.x.

For 5.4.x you can trim the test case down to just the first test (line 33) and it still has issues.  Just add two close braces and a closing php tag on line 34 and nuke the rest of the file.  The problem is collator_sort is never called, so the global intl_get_error_message() stuff is never setup.

http://git.php.net/?p=php-src.git;a=blob;f=ext/intl/collator/collator_sort.c;h=0785111c964b476da2c1d169bad65f0ab1048fa9;hb=refs/heads/PHP-5.4#l289

Line 343 PHP_FUNCTION( collator_sort ) calls collator_sort_internal from line 289.  Line 299 should fail and line 302 should setup the intl_get_error_message() stuff with U_ILLEGAL_ARGUMENT_ERROR.  But collator_sort (zif_collator_sort) is never called.
 [2013-06-04 15:26 UTC] laruence@php.net
hmm, maybe you can paste the full backtrace out?

gdb> bt full

thanks
 [2013-06-04 16:43 UTC] bfrance@php.net
-Status: Feedback +Status: Open
 [2013-06-04 16:43 UTC] bfrance@php.net
#0  _zval_ptr_dtor (zval_ptr=0x7ffff7ebfe70) at php-5.3.24/Zend/zend_execute_API.c:441
        zv = 0x600000000
#1  0x00000000007038a6 in zend_do_fcall_common_helper_SPEC (execute_data=0x7ffff7ebfa98) at php-5.3.24/Zend/zend_vm_execute.h:418
        opline = <value optimized out>
        should_change_scope = 3 '\003'
#2  0x00000000006dc948 in execute (op_array=0xfb6510) at php-5.3.24/Zend/zend_vm_execute.h:107
        ret = <value optimized out>
        execute_data = 0x7ffff7ebfa98
        nested = 1 '\001'
        original_in_execution = 1 '\001'
#3  0x00000000006ae1b0 in zend_call_function (fci=0x7fffffffaaa0, fci_cache=<value optimized out>) at php-5.3.24/Zend/zend_execute_API.c:969
        i = <value optimized out>
        original_return_value = 0x7ffff7ebdc60
        calling_symbol_table = 0x0
        original_op_array = 0x10f1548
        original_opline_ptr = 0x7ffff7ebed68
        current_scope = 0x0
        current_called_scope = 0xe67b90
        calling_scope = 0x115e0a8
        called_scope = <value optimized out>
        current_this = 0x115e0a8
        execute_data = {opline = 0x0, function_state = {function = 0xfb6510, arguments = 0x7ffff7ebfa90}, fbc = 0x0, called_scope = 0x0, op_array = 0x0, object = 0x1191d08, Ts = 0x7ffff7ebee70, CVs = 0x7ffff7ebee00, 
          symbol_table = 0x0, prev_execute_data = 0x7ffff7ebed68, old_error_reporting = 0x0, nested = 1 '\001', original_return_value = 0x0, current_scope = 0xfb84e0, current_called_scope = 0xfb60c8, current_this = 0x1191d08, 
          current_object = 0x0, call_opline = 0xfe1e98}
#4  0x0000000000583a8a in zim_reflection_method_invokeArgs (ht=<value optimized out>, return_value=0x115deb8, return_value_ptr=<value optimized out>, this_ptr=<value optimized out>, return_value_used=<value optimized out>)
    at php-5.3.24/ext/reflection/php_reflection.c:2753
        retval_ptr = <value optimized out>
        params = 0x123a3a8
        object = 0x1191d08
        intern = 0x1166fe0
        mptr = 0xfb6510
        argc = 0
        result = <value optimized out>
        fci = {size = 72, function_table = 0x0, function_name = 0x0, symbol_table = 0x0, retval_ptr_ptr = 0x7fffffffab38, param_count = 0, params = 0x123a3a8, object_ptr = 0x1191d08, no_separation = 1 '\001'}
        fcc = {initialized = 1 '\001', function_handler = 0xfb6510, calling_scope = 0xfb60c8, called_scope = 0xfb60c8, object_ptr = 0x1191d08}
        obj_ce = 0xfb60c8
        param_array = 0x115de58
#5  0x0000000000703d37 in zend_do_fcall_common_helper_SPEC (execute_data=0x7ffff7ebed68) at php-5.3.24/Zend/zend_vm_execute.h:322
        opline = <value optimized out>
        should_change_scope = 1 '\001'
#6  0x00000000006dc948 in execute (op_array=0x10f1548) at php-5.3.24/Zend/zend_vm_execute.h:107
        ret = <value optimized out>
        execute_data = 0x7ffff7ebed68
        nested = 1 '\001'
        original_in_execution = 0 '\000'
#7  0x00000000006b758a in zend_execute_scripts (type=8, retval=0x0, file_count=3) at php-5.3.24/Zend/zend.c:1259
        files = {{gp_offset = 40, fp_offset = 0, overflow_arg_area = 0x7fffffffad50, reg_save_area = 0x7ffffffface0}}
        i = <value optimized out>
        file_handle = 0x7fffffffe160
        orig_op_array = 0x0
        orig_retval_ptr_ptr = 0x0
#8  0x0000000000666ace in php_execute_script (primary_file=0x7fffffffe160) at php-5.3.24/main/main.c:2316
        realfile = "/usr/local/bin/phpunit\000\000\210\341\377\367\377\177", '\000' <repeats 42 times>"\340, \344\377\367\377\177\000\000p\276\377\377\377\177\000\000\000\000\000\000\000\000\000\000XԹ\364\377\177\000\000\230\331\374\367\377\177\000\000\000\000\000\000\000\000\000\000\377\377\377\377", '\000' <repeats 12 times>, "\001\000\000\000\000\000\000\000p^\317", '\000' <repeats 13 times>"\213, \322\321\000\000\000\000\000\r", '\000' <repeats 15 times>, "Я\336\367\377\177\000\000\001", '\000' <repeats 23 times>, "XԹ\364\377\177\000\000\210\343\271\364\377\177\000\000\270\356\317\000\000\000\000\000@\317\377\377\377\177\000\000p\374\317\000\000\000\000\000\r\000\000\000\000\000\000\000\225\026\337\367\377\177\000\000\002\000\000\000\000\000\000\000`\201\322\000\000\000\000\000\034\000\000\000\000\000\000\000\200\355\206\353\326O9\253\200g\362\364\377\177\000\000\000\000\000\000\000\000\000\000\200g\362\364\377\177"...
        __orig_bailout = 0x7fffffffdff0
        __bailout = {{__jmpbuf = {15446400, 1175243881897479723, 140737488348848, 0, 140737488348136, 0, 1175243883034136107, -1175244725073261013}, __mask_was_saved = 0, __saved_mask = {__val = {229440404087961, 0, 140737299689793, 
                48, 13753520, 15725344, 7103285, 532575944752, 7, 140737488342704, 153, 7, 140737488342704, 0, 15725216, 0}}}}
        prepend_file_p = <value optimized out>
        append_file_p = 0x0
        prepend_file = {type = ZEND_HANDLE_FILENAME, filename = 0x0, opened_path = 0x0, handle = {fd = 0, fp = 0x0, stream = {handle = 0x0, isatty = 0, mmap = {len = 0, pos = 0, map = 0x0, buf = 0x0, old_handle = 0x0, old_closer = 0}, 
              reader = 0, fsizer = 0, closer = 0}}, free_filename = 0 '\000'}
        append_file = {type = ZEND_HANDLE_FILENAME, filename = 0x0, opened_path = 0x0, handle = {fd = 0, fp = 0x0, stream = {handle = 0x0, isatty = 0, mmap = {len = 0, pos = 0, map = 0x0, buf = 0x0, old_handle = 0x0, old_closer = 0}, 
              reader = 0, fsizer = 0, closer = 0}}, free_filename = 0 '\000'}
        old_cwd = 0x7fffffffad60 ""
        use_heap = 0 '\000'
        retval = 0
#9  0x000000000073de34 in main (argc=6, argv=0x7fffffffe3d8) at php-5.3.24/sapi/cli/php_cli.c:1189
        __orig_bailout = 0x0
        __bailout = {{__jmpbuf = {124, -1175245194508153301, 13630576, 13, 13750923, 13, 1175243881899576875, -1175244831334248917}, __mask_was_saved = 0, __saved_mask = {__val = {140737351936935, 4294967455, 11341735, 45, 
                140737299199672, 0, 140737488347696, 140737299205192, 140737299224056, 1910330751, 140737351934614, 0, 140737488347456, 140733193388095, 140737488347456, 19}}}}
        exit_status = 0
        c = <value optimized out>
        file_handle = {type = ZEND_HANDLE_MAPPED, filename = 0x7fffffffe6b0 "/usr/local/bin/phpunit", opened_path = 0x0, handle = {fd = 15570712, fp = 0xed9718, stream = {handle = 0xed9718, isatty = 0, mmap = {len = 2028, pos = 0, 
                map = 0x7ffff7eb2000, buf = 0x7ffff7eb2015 <Address 0x7ffff7eb2015 out of bounds>, old_handle = 0xefee50, old_closer = 0x6cc460 <zend_stream_stdio_closer>}, reader = 0x6cca60 <zend_stream_stdio_reader>, 
              fsizer = 0x6cc990 <zend_stream_stdio_fsizer>, closer = 0x6cc9e0 <zend_stream_mmap_closer>}}, free_filename = 0 '\000'}
        behavior = 1
        reflection_what = 0x0
        orig_optind = 1
        orig_optarg = 0x0
        arg_free = <value optimized out>
        arg_excp = <value optimized out>
        script_file = <value optimized out>
        translated_path = 0xebb180 "/usr/local/bin/phpunit"
        interactive = <value optimized out>
        module_started = 1
        request_started = 1
        lineno = 2
        exec_direct = 0x0
        exec_run = <value optimized out>
        exec_begin = 0x0
        exec_end = 0x0
        param_error = <value optimized out>
        hide_argv = 0
        ini_entries_len = <value optimized out>
 [2013-06-07 15:02 UTC] bfrance@php.net
Just to give another update, 5.4.13 works!!!!


------ 5.4.13 ------
% php -dopen_basedir= /usr/local/bin/phpunit --log-junit results.xml phpIntlTest02.php
PHPUnit 3.7.21 by Sebastian Bergmann.

.

Time: 0 seconds, Memory: 2.50Mb

OK (1 test, 116 assertions)
------


Where 5.4.14 fails:

------ 5.4.14 ------
& php -dopen_basedir= /usr/local/bin/phpunit --log-junit results.xml phpIntlTest02.php
PHPUnit 3.7.21 by Sebastian Bergmann.

F

Time: 0 seconds, Memory: 3.50Mb

There was 1 failure:

1) YPHPINTLTest::test_collator_sort
Wrong type of arguments
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'collator_sort_internal: unable to parse input params: U_ILLEGAL_ARGUMENT_ERROR'
+'U_USING_FALLBACK_WARNING'

/home/bfrance/php-5.4.14/ext/intl/tests/phpIntlTest02.php:33

FAILURES!
Tests: 1, Assertions: 2, Failures: 1.
------


So while 5.4 doesn't core dump, that patch from 63914 really changed the execution path to the point it breaks the test.
 [2013-06-07 15:58 UTC] laruence@php.net
is there any chance you can give me a access to your box?

I get some problems to setup the PHPUNIT here
 [2013-06-07 16:40 UTC] bfrance@php.net
Sorry, this is for work and I am pretty sure the wouldn't let me give you access to a internal machine.

See if this helps:

sudo pear config-set auto_discover 1
sudo pear channel-discover pear.phpunit.de
sudo pear install pear.phpunit.de/PHPUnit


That worked for me for two clean installs this morning (5.4.13 and 5.4.14).  So I think have figured out what is happening, at least with 5.4 and why it changed.

Code in question is this:

http://git.php.net/?p=php-src.git;a=blob;f=Zend/zend_vm_execute.h;h=bb50b4803f7143acff1c15647f5f45807d7ced16;hb=HEAD#l525

I can't figure out how to get git to let me show 5.3.13 and 5.2.14 diff, so here is a clean diff:

http://www.brianfrance.com/php/5.4.14.txt


The issue in 5.4.14 is that zend_verify_arg_type is throwing an exception, this means that the real function will never be called as it is now wrapped in a:

if (EXPECTED(EG(exception) == NULL)) {

}

In 5.4.13 there wasn't an exception check, so it would call the function regardless of the zend_verify_arg_type checks (would still have a warning printed).

If this is now the normal flow, I can go back to the intl our team and tell them they need to fix there test cases (in 5.4).

Granted this doesn't fix 5.3 core dump, which is what I am digging into again today.
 [2013-06-07 17:06 UTC] laruence@php.net
where can I get the "results.xml"?
 [2013-06-07 17:14 UTC] bfrance@php.net
5.3.24 is blank as it core dumps and 5.4.14 is blank because it fails.  5.3.23:


<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
  <testsuite name="YPHPINTLTest" file="/home/bfrance/php-5.3.23/ext/intl/tests/phpIntlTest02.php" tests="1" assertions="116" failures="0" errors="0" time="0.010190">
    <testcase name="test_collator_sort" class="YPHPINTLTest" file="/home/bfrance/php-5.3.23/ext/intl/tests/phpIntlTest02.php" line="7" assertions="116" time="0.010190"/>
  </testsuite>
</testsuites>


and 5.4.13:

<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
  <testsuite name="YPHPINTLTest" file="/home/bfrance/php-5.4.13/ext/intl/tests/phpIntlTest03.php" tests="1" assertions="2" failures="0" errors="0" time="22.547478">
    <testcase name="test_collator_sort" class="YPHPINTLTest" file="/home/bfrance/php-5.4.13/ext/intl/tests/phpIntlTest03.php" line="7" assertions="2" time="22.547478"/>
  </testsuite>
</testsuites>


I created a phpIntlTest03.php from phpIntlTest02.php that is only line 1-33, then "} } ?>"
 [2013-06-07 17:29 UTC] bfrance@php.net
Here is the patch that was applied to 5.3.23:

http://www.brianfrance.com/php/5.3.24.txt

If I keep this line:

  ALLOC_INIT_ZVAL(EX_T(opline->result.u.var).var.ptr);

then the core dump goes away and a get the same test failure, like in 5.4.14+.  And that can be explain by the:

 if (EXPECTED(EG(exception) == NULL)) {
 }

from the comment above.

I can't explain why yet, but working on seeing if I can found who uses that value or who might expect it to be allocated.
 [2013-06-07 19:53 UTC] bfrance@php.net
The following patch has been added/updated:

Patch Name: exception.diff
Revision:   1370634825
URL:        https://bugs.php.net/patch-display.php?bug=64966&patch=exception.diff&revision=1370634825
 [2013-06-07 20:04 UTC] bfrance@php.net
I just added a patch that make 5.3.24+ not core dump, but I want somebody to review it.

http://git.php.net/?p=php-src.git;a=blob;f=Zend/zend_vm_execute.h;h=f6220b0f5305924afd7f480f321cae8075b46ab2;hb=refs/heads/PHP-5.3#l303

The issue is allocate for EX_T(opline->result.u.var).var.ptr was moved to line 316 and inside the:

 if (EXPECTED(EG(exception) == NULL)) {
 }

block.  The problem with this is that on line 417, it calls:

  zval_ptr_dtor(&EX_T(opline->result.u.var).var.ptr);

but without allocating it.  

May be the other option would be to add a else option at line 330 to either null the value or set:

  RETURN_VALUE_USED(opline)

to false instead of true (no idea how to do that), which it currently is.

Thoughts?
 [2013-06-08 06:37 UTC] laruence@php.net
A more simple fix might be like:
diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h
index 02566f3..d471f39 100644
--- a/Zend/zend_vm_def.h
+++ b/Zend/zend_vm_def.h
@@ -2327,6 +2327,8 @@ ZEND_VM_HELPER(zend_do_fcall_common_helper, ANY, ANY)
 			if (!RETURN_VALUE_USED(opline)) {
 				zval_ptr_dtor(&EX_T(opline-
>result.u.var).var.ptr);
 			}
+		} else if (RETURN_VALUE_USED(opline)) {
+			EX_T(opline->result.u.var).var.ptr = NULL;
 		}
 	} else if (EX(function_state).function->type == ZEND_USER_FUNCTION) {
 		EX(original_return_value) = EG(return_value_ptr_ptr);
 [2013-06-08 08:39 UTC] laruence@php.net
here is a small reproduce script, if no segfault, run with valgrind:

<?php
error_reporting(E_ALL | E_STRICT);
set_error_handler(function($error) { throw new Exception(); }, 
E_RECOVERABLE_ERROR);


function test($func) {
    $a = $func("");
    return true;
}
class A {
    public function b() {
        test("strlen");
        test("iterator_apply");
    }
}

$a = new A();
$a->b();
 [2013-06-08 08:39 UTC] laruence@php.net
-Assigned To: +Assigned To: laruence
 [2013-06-08 09:15 UTC] laruence@php.net
-Summary: reflection_method_invokeArgs core dump +Summary: segfault in zend_do_fcall_common_helper_SPEC
 [2013-06-08 09:15 UTC] laruence@php.net
change summary, since not reflection specific bug
 [2013-06-08 09:19 UTC] laruence@php.net
The following patch has been added/updated:

Patch Name: bug64966.patch
Revision:   1370683141
URL:        https://bugs.php.net/patch-display.php?bug=64966&patch=bug64966.patch&revision=1370683141
 [2013-06-08 09:19 UTC] laruence@php.net
The following patch has been added/updated:

Patch Name: bug64966.phpt
Revision:   1370683159
URL:        https://bugs.php.net/patch-display.php?bug=64966&patch=bug64966.phpt&revision=1370683159
 [2013-06-08 18:20 UTC] bfrance@php.net
Your patch fixes the core dump and is cleaner, thanks!

What are your thoughts on:

Major Compatible Change or Simple Bug fix? 
  http://marc.info/?l=php-internals&m=137066248125910&w=2


Your phpunit test does have a warning removed from 5.4.13/5.3.23:

% php test.php 

Warning: iterator_apply() expects at least 2 parameters, 1 given in /home/bfrance/test.php on line 6

Fatal error: Uncaught exception 'Exception' in /home/bfrance/test.php:3
Stack trace:
#0 [internal function]: {closure}(4096, 'Argument 1 pass...', '/home/bfrance/t...', 6, Array)
#1 /home/bfrance/test.php(6): iterator_apply('')
#2 /home/bfrance/test.php(12): test('iterator_apply')
#3 /home/bfrance/test.php(17): A->b()
#4 {main}
  thrown in /home/bfrance/test.php on line 3



compared to 5.4.14/5.2.24 (with this patch):


% php test.php 

Fatal error: Uncaught exception 'Exception' in /home/bfrance/test.php:3
Stack trace:
#0 [internal function]: {closure}(4096, 'Argument 1 pass...', '/home/bfrance/t...', 6, Array)
#1 /home/bfrance/test.php(6): iterator_apply(NULL)
#2 /home/bfrance/test.php(12): test('iterator_apply')
#3 /home/bfrance/test.php(17): A->b()
#4 {main}
  thrown in /home/bfrance/test.php on line 3


But the bigger change is in the email thread.
 [2013-06-09 05:51 UTC] laruence@php.net
Automatic comment on behalf of laruence
Revision: http://git.php.net/?p=php-src.git;a=commit;h=e8f004d54252e0130b88131bdc46a41ed365c51e
Log: Fixed bug #64966 (segfault in zend_do_fcall_common_helper_SPEC)
 [2013-06-09 05:51 UTC] laruence@php.net
-Status: Assigned +Status: Closed
 
PHP Copyright © 2001-2014 The PHP Group
All rights reserved.
Last updated: Wed Apr 16 18:01:53 2014 UTC