Skip to content

PHP 8.2 Segmentation fault on nesbot/carbon #11455

Closed

Description

@zeriyoshi

Description

There is a bug in nesbot/carbon that causes a segmentation violation when serialize / unserialize is executed in PHP 8.2 and later.

In response to this bug, nesbot/carbon has a workaround that implements a userland serialization algorithm, which significantly worse performance. (briannesbitt/Carbon#2644)

This can be reproduced by following these steps:

$ git clone --branch=php82_segv --depth=1 "https://github.com/zeriyoshi/Carbon.git" "Carbon"
$ cd "Carbon"
$ composer install
$ composer exec -- phpunit tests/Carbon/SerializationTest.php --debug --filter testSerialization
PHPUnit 9.6.9 by Sebastian Bergmann and contributors.

Warning:       Your XML configuration validates against a deprecated schema.
Suggestion:    Migrate your XML configuration using "--migrate-configuration"!

Test 'Tests\Carbon\SerializationTest::testSerialization' started
Segmentation fault
Script phpunit handling the __exec_command event returned with error code 139

backtrace (gdb):

0x0000aaaac274195c in zend_mm_alloc_small (heap=0xffff93a00040, bin_num=9, __zend_filename=0xaaaac31d94e0 "/usr/src/php/Zend/zend_string.h", __zend_lineno=152, __zend_orig_filename=0x0, __zend_orig_lineno=0) at /usr/src/php/Zend/zend_alloc.c:1313
1313			heap->free_slot[bin_num] = p->next_free_slot;
(gdb) backtrace
#0  0x0000aaaac274195c in zend_mm_alloc_small (heap=0xffff93a00040, bin_num=9, __zend_filename=0xaaaac31d94e0 "/usr/src/php/Zend/zend_string.h", __zend_lineno=152, __zend_orig_filename=0x0, __zend_orig_lineno=0) at /usr/src/php/Zend/zend_alloc.c:1313
#1  0x0000aaaac2741c38 in zend_mm_alloc_heap (heap=0xffff93a00040, size=88, __zend_filename=0xaaaac31d94e0 "/usr/src/php/Zend/zend_string.h", __zend_lineno=152, __zend_orig_filename=0x0, __zend_orig_lineno=0) at /usr/src/php/Zend/zend_alloc.c:1384
#2  0x0000aaaac2744ae4 in _emalloc (size=56, __zend_filename=0xaaaac31d94e0 "/usr/src/php/Zend/zend_string.h", __zend_lineno=152, __zend_orig_filename=0x0, __zend_orig_lineno=0) at /usr/src/php/Zend/zend_alloc.c:2594
#3  0x0000aaaac28775c4 in zend_string_alloc (len=29, persistent=false) at /usr/src/php/Zend/zend_string.h:152
#4  0x0000aaaac2877634 in zend_string_init (str=0xffff92e4dcd8 "Symfony\\Component\\Translation", len=29, persistent=false) at /usr/src/php/Zend/zend_string.h:174
#5  0x0000aaaac2878244 in zend_new_interned_string_request (str=0xffff92e4dcc0) at /usr/src/php/Zend/zend_string.c:249
#6  0x0000aaaac275e75c in zend_compile_use (ast=0xffff92edd108) at /usr/src/php/Zend/zend_compile.c:8172
#7  0x0000aaaac2764528 in zend_compile_stmt (ast=0xffff92edd108) at /usr/src/php/Zend/zend_compile.c:10174
#8  0x0000aaaac2764168 in zend_compile_top_stmt (ast=0xffff92edd108) at /usr/src/php/Zend/zend_compile.c:10078
#9  0x0000aaaac2764094 in zend_compile_top_stmt (ast=0xffff92edd390) at /usr/src/php/Zend/zend_compile.c:10064
#10 0x0000aaaac2711f80 in zend_compile (type=2) at /usr/src/php/Zend/zend_language_scanner.c:620
#11 0x0000aaaac27120ac in compile_file (file_handle=0xffffdd80d960, type=2) at /usr/src/php/Zend/zend_language_scanner.c:655
#12 0x0000aaaac2493ab0 in phar_compile_file (file_handle=0xffffdd80d960, type=2) at /usr/src/php/ext/phar/phar.c:3355
#13 0x0000aaaac2712258 in compile_filename (type=2, filename=0xffff92e57480) at /usr/src/php/Zend/zend_language_scanner.c:706
#14 0x0000aaaac27c6c04 in zend_include_or_eval (inc_filename_zv=0xffff93a1cdb0, type=2) at /usr/src/php/Zend/zend_execute.c:4799
#15 0x0000aaaac282d7c8 in ZEND_INCLUDE_OR_EVAL_SPEC_CV_HANDLER () at /usr/src/php/Zend/zend_vm_execute.h:38948
#16 0x0000aaaac2854fd4 in execute_ex (ex=0xffff93a1cc90) at /usr/src/php/Zend/zend_vm_execute.h:59395
#17 0x0000aaaac276caf4 in zend_call_function (fci=0xffffdd80df18, fci_cache=0xffffdd80def8) at /usr/src/php/Zend/zend_execute_API.c:947
#18 0x0000aaaac276cfdc in zend_call_known_function (fn=0xffff93a08b78, object=0xffff93a78400, called_scope=0xffff93a07440, retval_ptr=0x0, param_count=1, params=0xffffdd80dfa0, named_params=0x0) at /usr/src/php/Zend/zend_execute_API.c:1041
#19 0x0000aaaac25040c0 in spl_perform_autoload (class_name=0xffff9344b230, lc_name=0xffff9344baa0) at /usr/src/php/ext/spl/php_spl.c:445
#20 0x0000aaaac276d78c in zend_lookup_class_ex (name=0xffff9344b230, key=0xffff9344baa0, flags=512) at /usr/src/php/Zend/zend_execute_API.c:1207
#21 0x0000aaaac276e4f0 in zend_fetch_class_by_name (class_name=0xffff9344b230, key=0xffff9344baa0, fetch_type=512) at /usr/src/php/Zend/zend_execute_API.c:1703
#22 0x0000aaaac27e36ec in ZEND_NEW_SPEC_CONST_UNUSED_HANDLER () at /usr/src/php/Zend/zend_vm_execute.h:10271
#23 0x0000aaaac28528fc in execute_ex (ex=0xffff93a17020) at /usr/src/php/Zend/zend_vm_execute.h:56937
#24 0x0000aaaac2855fd4 in zend_execute (op_array=0xffff93a85000, return_value=0x0) at /usr/src/php/Zend/zend_vm_execute.h:60396
#25 0x0000aaaac27868ac in zend_execute_scripts (type=8, retval=0x0, file_count=3) at /usr/src/php/Zend/zend.c:1827
#26 0x0000aaaac26d3bcc in php_execute_script (primary_file=0xffffdd8109f0) at /usr/src/php/main/main.c:2542
#27 0x0000aaaac291ccfc in do_cli (argc=5, argv=0xaaaaecc05270) at /usr/src/php/sapi/cli/php_cli.c:964
#28 0x0000aaaac291d950 in main (argc=5, argv=0xaaaaecc05270) at /usr/src/php/sapi/cli/php_cli.c:1333

zbacktrace (gdb)

[0xffff93a1cd60] Composer\Autoload\{closure}("/root/Carbon/vendor/composer/../../src/Carbon/Translator.php") /root/Carbon/vendor/composer/ClassLoader.php:576
[0xffff93a1cc90] Composer\Autoload\ClassLoader->loadClass("Carbon\Translator") /root/Carbon/vendor/composer/ClassLoader.php:427
[0xffff93a1ca60] Tests\AbstractTestCase->tearDown() /root/Carbon/tests/AbstractTestCase.php:99
[0xffff93a1c340] PHPUnit\Framework\TestCase->runBare() /root/Carbon/vendor/phpunit/phpunit/src/Framework/TestCase.php:1260
[0xffff93a1b140] PHPUnit\Framework\TestResult->run(object[0xffff93a1b190]) /root/Carbon/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
[0xffff93a1a530] PHPUnit\Framework\TestCase->run(object[0xffff93a1a580]) /root/Carbon/vendor/phpunit/phpunit/src/Framework/TestCase.php:964
[0xffff93a19e30] PHPUnit\Framework\TestSuite->run(object[0xffff93a19e80]) /root/Carbon/vendor/phpunit/phpunit/src/Framework/TestSuite.php:684
[0xffff93a17a00] PHPUnit\TextUI\TestRunner->run(object[0xffff93a17a50], reference, reference, true) /root/Carbon/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:651
[0xffff93a17630] PHPUnit\TextUI\Command->run(array(4)[0xffff93a17680], true) /root/Carbon/vendor/phpunit/phpunit/src/TextUI/Command.php:144
[0xffff93a17520] PHPUnit\TextUI\Command->main() /root/Carbon/vendor/phpunit/phpunit/src/TextUI/Command.php:97
[0xffff93a171e0] (main) /root/Carbon/vendor/phpunit/phpunit/phpunit:107
[0xffff93a17020] (main) /root/Carbon/vendor/bin/phpunit:122

PHP Version

PHP 8.2.7
PHP 8.2.8-dev
PHP 8.3.0-dev master 96ea06a

Operating System

Debian 11.7

Activity

  1. zeriyoshi commented on Jun 15, 2023

    @zeriyoshi
    ContributorAuthor

    The problem was solved by importing the ext/date of the PHP-8.1 branch into the PHP-8.2 branch. Perhaps this is an ext/date issue. I will investigate further.

  2. zeriyoshi commented on Jun 15, 2023

    @zeriyoshi
    ContributorAuthor
    • PHP 8.2.3 OK (but Test failed)
    • PHP 8.2.4 Segmentation Fault

    Segmentation violation appears to have occurred after this commit: 85fbc6e

  3. zeriyoshi commented on Jun 15, 2023

    @zeriyoshi
    ContributorAuthor

    I am trying to come up with the smallest code that will reproduce the problem, but it is difficult.
    Hopefully the above information will shed some light...
    @derickr @kylekatarnls

  4. zeriyoshi commented on Jun 16, 2023

    @zeriyoshi
    ContributorAuthor

    memo: lldb backtrace

    (lldb) r vendor/bin/phpunit tests/Carbon/SerializationTest.php --debug --filter testSerialization
    Process 96301 launched: '/Users/g-kudo/Development/php-src/sapi/cli/php' (arm64)
    PHPUnit 9.6.9 by Sebastian Bergmann and contributors.
    
    Warning:       Your XML configuration validates against a deprecated schema.
    Suggestion:    Migrate your XML configuration using "--migrate-configuration"!
    
    Test 'Tests\Carbon\SerializationTest::testSerialization' started
    Assertion failed: (zval_gc_type((ref)->gc.u.type_info) == 7 || zval_gc_type((ref)->gc.u.type_info) == 8), function gc_possible_root, file zend_gc.c, line 647.
    Process 96301 stopped
    * thread #1, queue = 'com.apple.main-thread', stop reason = hit program assert
        frame #4: 0x00000001006501c8 php`gc_possible_root(ref=0x00000001033f6ea0) at zend_gc.c:647:2
       644                  return;
       645          }
       646 
    -> 647          ZEND_ASSERT(GC_TYPE(ref) == IS_ARRAY || GC_TYPE(ref) == IS_OBJECT);
       648          ZEND_ASSERT(GC_INFO(ref) == 0);
       649 
       650          newRoot = GC_IDX2PTR(idx);
    Target 0: (php) stopped.
    (lldb) bt
    * thread #1, queue = 'com.apple.main-thread', stop reason = hit program assert
        frame #0: 0x000000019c150724 libsystem_kernel.dylib`__pthread_kill + 8
        frame #1: 0x000000019c187c28 libsystem_pthread.dylib`pthread_kill + 288
        frame #2: 0x000000019c095ae8 libsystem_c.dylib`abort + 180
        frame #3: 0x000000019c094e44 libsystem_c.dylib`__assert_rtn + 272
      * frame #4: 0x00000001006501c8 php`gc_possible_root(ref=0x00000001033f6ea0) at zend_gc.c:647:2
        frame #5: 0x0000000100673854 php`gc_check_possible_root(ref=0x00000001033f6ea0) at zend_gc.h:88:3
        frame #6: 0x0000000100672914 php`i_zval_ptr_dtor(zval_ptr=0x0000000103098450) at zend_variables.h:46:4
        frame #7: 0x00000001006725ac php`zend_object_std_dtor(object=0x0000000103098408) at zend_objects.c:71:5
        frame #8: 0x0000000100020aac php`date_object_free_storage_date(object=0x0000000103098408) at php_date.c:2282:2
        frame #9: 0x000000010067d050 php`zend_objects_store_del(object=0x0000000103098408) at zend_objects_API.c:200:4
        frame #10: 0x0000000100529828 php`rc_dtor_func(p=0x0000000103098408) at zend_variables.c:57:2
        frame #11: 0x000000010056e174 php`zval_ptr_dtor_nogc(zval_ptr=0x000000010221d280) at zend_variables.h:35:3
        frame #12: 0x0000000100628920 php`zend_vm_stack_free_args(call=0x000000010221d230) at zend_execute.h:324:4
        frame #13: 0x00000001005f5678 php`ZEND_DO_FCALL_BY_NAME_SPEC_RETVAL_USED_HANDLER(execute_data=0x000000010221cf90) at zend_vm_execute.h:1680:3
        frame #14: 0x00000001005727f0 php`execute_ex(ex=0x0000000102217020) at zend_vm_execute.h:56856:7
        frame #15: 0x0000000100572bec php`zend_execute(op_array=0x000000010228a000, return_value=0x0000000000000000) at zend_vm_execute.h:61445:2
        frame #16: 0x000000010052dde4 php`zend_execute_scripts(type=8, retval=0x0000000000000000, file_count=3) at zend.c:1862:4
        frame #17: 0x000000010045d6ec php`php_execute_script(primary_file=0x000000016fdfe830) at main.c:2482:13
        frame #18: 0x000000010071b2f4 php`do_cli(argc=6, argv=0x0000000102007670) at php_cli.c:959:5
        frame #19: 0x000000010071a430 php`main(argc=6, argv=0x0000000102007670) at php_cli.c:1328:18
        frame #20: 0x000000019be2ff28 dyld`start + 2236
    
  5. ndossche commented on Jun 17, 2023

    @ndossche
    Member

    @zeriyoshi
    Looks like a refcounting bug in add_common_properties. I modified one of the ext/date testcases a bit to reproduce this issue:

    <?php
    class MyDateTimeImmutable extends DateTimeImmutable {
        public function __construct(
            string $datetime = "now",
            ?DateTimeZone $timezone = null,
            public ?stdClass $myProperty = null,
        ) {
            parent::__construct($datetime, $timezone);
        }
    }
    
    $datetime = new MyDateTimeImmutable('2022-12-22T11:26:00Z', myProperty: new stdClass);
    $datetime->myProperty->field = str_repeat("hello", 3);
    $serialized = serialize($datetime);
    var_dump($datetime->myProperty);
    $unserialized = unserialize($serialized);
    
    var_dump($unserialized);

    This code causes a crash in malloc().

    The fix is simple, it's a matter of adding the refcount if a new entry is added to the hash table.
    EDIT: removed outdated patch, see PR.

    Executing my testcase now works. I tried this with your Carbon fork, and it no longer crashes in malloc(), but the test fails. This is because Carbon tries to serialize a Closure stored in $localTranslator->$ordinal:

    https://github.com/briannesbitt/Carbon/blob/cfb8701c35ade1c9c00cfd8a3a3fe029b5a75e77/src/Carbon/Lang/en.php#L71-L83

    So although my patch fixes the underlying bug in this issue, I'm not so sure Carbon can just get rid of their custom serialization.
    Please let me know if I missed something. If not I can PR this up.

  6. zeriyoshi commented on Jun 18, 2023

    @zeriyoshi
    ContributorAuthor

    @nielsdos
    That seems to make sense!

    We have also identified an issue on the Carbon side that needs to be addressed by Carbon. As soon as this fix is incorporated into PHP 8.2-dev, I will suggest a fix to Carbon (though it may take some time, as I am not familiar with Carbon. A fix by the maintainer would be even better :) ).

  7. kylekatarnls commented on Jun 18, 2023

    @kylekatarnls
    Contributor

    Indeed, on Carbon side, we use the custom serialization to dump/restore some other parameters, also we'll need to support the bugged versions for a while as it makes maintenance more difficult if few users are stuck on an old patch, but putting this out of the way for the next patch is already super useful.

  8. ndossche commented on Jun 18, 2023

    @ndossche
    Member

    Great, thanks for checking. I opened a PR.

  9. zeriyoshi commented on Jun 18, 2023

    @zeriyoshi
    ContributorAuthor

    @nielsdos
    Thanks! Can you tell me how you identified the problem this time as I would like to help in the future?

  10. ndossche commented on Jun 19, 2023

    @ndossche
    Member

    @zeriyoshi
    A crash in the Zend heap usually means a refcounting bug, an overflow or a double free.
    There were no buffers in the commit you linked so I guessed it was probably a refcounting bug.
    I then read the code of the commit and saw there was no refcounting when adding to the HashTable, but in general I expect to see refcounting there unless the zval will be uniquely owned or can't be an object/array/string. So I guessed that must be the problem and created a testcase to check that. I saw it crashed and then checked what Carbon was doing, and I saw Carbon crashed due the localTranslator serialization. I figured that out by just skipping over objects during serialization.
    Then I made the patch and saw the problem went away.

  11. zeriyoshi commented on Jun 19, 2023

    @zeriyoshi
    ContributorAuthor

    @nielsdos Thank you! It was very easy to understand. I will use it as a reference!

  12. added a commit that references this issue on Jun 19, 2023
    12e4628
  13. added a commit that references this issue on Jan 31, 2024
    93becab
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions