Segmentation fault on script exit #9361

Activity

1. dvaeversted

   added

   Bug

   Status: Needs Triage

   on Aug 17, 2022
2. iluuu1994 commented on Aug 18, 2022

   

   iluuu1994

   on Aug 18, 2022

   Member

   More actions

   I had a look to try and understand what's going on. At the end of the request, the current memory limit is reset to the global one in case ini_set('memory_limit','...'); was set manually during execution. Before setting the memory limit, it is checked whether the current memory usage is larger than the one requested, in which case the operation fails.

   php-src/Zend/zend_alloc.c

   Line 2687 in aa702c5

   if (UNEXPECTED(memory_limit < heap->real_size)) {

   The fact that this check succeeds either means that your current memory usage is actually above 8GB (in which case you would have an even larger ini_set('memory_limit','...'); somewhere) or there is some memory corruption going on. Given that the p->next segfaults the latter is very likely.

   It would be helpful to know:

   • How big these 3 files are
   • Approximately what you're doing with these 3 files

   If you provide this information I'll try to reconstruct a test case. You could also try running this script with Valgrind to see if you can get more information that way.
3. dvaeversted commented on Aug 18, 2022

   

   dvaeversted

   on Aug 18, 2022

   Author

   More actions

   We don't do any script side changes of the memory limit.

   Right before exiting the script, we actually had this call:

   echo number_format(memory_get_peak_usage() / 1024 / 1024, 2) . ' MiB' . PHP_EOL;
   

   Which gave the following output: 6,911.99 MiB.

   It is a script which reads in these 3 files, which contains network ranges along with metadata, that is then combined together to one single list, which is afterwards written out.
   The 3 files are 179 MiB (3,583,155 lines), 15 MiB (403,846 lines) and 9.7 KiB (253 lines)
   And the resulting output is then 419 MiB (14,150,354 lines)

   I'll try to make a simplified version of the script that i can share, but the source data files i won't be able to share publicly.
4. iluuu1994 commented on Aug 18, 2022

   

   iluuu1994

   on Aug 18, 2022

   Member

   More actions

   @dvaeversted Running with Valgrind might be your best bet as you can run your actual script and memory issues should be reported early. Just note that it will be very slow.
5. chschneider commented on Aug 18, 2022

   

   chschneider

   on Aug 18, 2022

   Last edited by chschneider

   Contributor

   More actions

   We encountered the same problem and I tracked it down to an integer overflow with ZEND_MM_CHUNK_SIZE.

   My reproduction code is

   <?php
   $result = [];
   foreach (range(1, 11500000) as $i)
   	$result[] = ['i' => $i];

   when called with php -n -d memory_limit=10G

   I fixed it by patching zend_alloc_sizes.h

   -#define ZEND_MM_CHUNK_SIZE (2 * 1024 * 1024)               /* 2 MB  */
   +#define ZEND_MM_CHUNK_SIZE (2 * 1024 * 1024L)              /* 2 MB  */
   

   The problem is code like commit 5675ebe
   5675ebe Christoph M. Becker 2021-12-09 15:36 +0100 2330│ heap->real_peak = (heap->cached_chunks_count + 1) * ZEND_MM_CHUNK_SIZE;
   and commit c035298
   c035298 Dmitry Stogov 2022-02-08 15:45 +0300 2688│ if (memory_limit >= heap->real_size - heap->cached_chunks_count * ZEND_MM_CHUNK_SIZE) {
   with the second commit actually causing the segfault I think.

   Reason: cached_chunks_count is an int and when ZEND_MM_CHUNK_SIZE was declared int too the multiplication could overflow. As the result in both cases is size_t I think it is safe to declare ZEND_MM_CHUNK_SIZE as long to avoid this.

   👍React with 👍3Reacted by Christoph M. Becker, Ilija Tovilo and Emil Stahl

   🚀React with 🚀1Reacted by Thibault Buathier
6. cmb69 commented on Aug 18, 2022

   

   cmb69

   on Aug 18, 2022

   Member

   More actions

   I fixed it by patching zend_alloc_sizes.h

   -#define ZEND_MM_CHUNK_SIZE (2 * 1024 * 1024)               /* 2 MB  */
   +#define ZEND_MM_CHUNK_SIZE (2 * 1024 * 1024L)              /* 2 MB  */
   

   That wouldn't work on Windows (long is 32bit there, even for x64), and I think it's more appropriate to have that as size_t. Since there is no respective suffix in C, we could do a cast:

   #define ZEND_MM_CHUNK_SIZE ((size_t) (2 * 1024 * 1024))

   👍React with 👍1Reacted by Christian Schneider
7. cmb69

   added

   Category: Engine

   on Aug 18, 2022
8. dvaeversted commented on Aug 19, 2022

   

   dvaeversted

   on Aug 19, 2022

   Author

   More actions

   @iluuu1994

   @dvaeversted Running with Valgrind might be your best bet as you can run your actual script and memory issues should be reported early. Just note that it will be very slow.

   No kidding about it being very slow, took all night to complete.

   ==92006== Memcheck, a memory error detector
   ==92006== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
   ==92006== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
   ==92006== Command: /usr/bin/php -d memory_limit=8192M cronscript-cleaned.php
   ==92006== Parent PID: 15205
   ==92006== 
   ==92006== 
   ==92006== HEAP SUMMARY:
   ==92006==     in use at exit: 69,269 bytes in 191 blocks
   ==92006==   total heap usage: 1,265,689,398 allocs, 1,265,689,207 frees, 316,409,542,965 bytes allocated
   ==92006== 
   ==92006== LEAK SUMMARY:
   ==92006==    definitely lost: 4,464 bytes in 28 blocks
   ==92006==    indirectly lost: 37,085 bytes in 70 blocks
   ==92006==      possibly lost: 0 bytes in 0 blocks
   ==92006==    still reachable: 27,720 bytes in 93 blocks
   ==92006==         suppressed: 0 bytes in 0 blocks
   ==92006== Rerun with --leak-check=full to see details of leaked memory
   ==92006== 
   ==92006== For lists of detected and suppressed errors, rerun with: -s
   ==92006== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
   

   However it did not segfault this time.
   I followed the guide here: https://bugs.php.net/bugs-getting-valgrind-log.php for running Valgrind, with USE_ZEND_ALLOC=0
   I tried running the script normally without Valgrind, but with that env variable set, and it did not crash. Unsetting it, makes it crash again.

   I also tried out the reproducer that @chschneider shared:

   <?php
   $result = [];
   foreach (range(1, 11500000) as $i)
   	$result[] = ['i' => $i];
   

   I can confirm it does not crash on php 8.0.22, but does on 8.1.9 & 8.2.0beta3, also exhibiting the behavior that if setting USE_ZEND_ALLOC=0 it does not crash, while taking around twice the time to run. And it exhibits the exact same backtrace in gdb.

   If still needed, any input with exactly how i should run Valgrind would be much appreciated, not a C developer myself.
9. cmb69 commented on Aug 19, 2022

   

   cmb69

   on Aug 19, 2022

   Member

   More actions

   According to @chschneider's analysis, this is a bug in the Zend memory manager, and as such won't occur when running with USE_ZEND_ALLOC=0. Can you please try with the following patch:

    Zend/zend_alloc_sizes.h | 2 +-
    1 file changed, 1 insertion(+), 1 deletion(-)
   
   diff --git a/Zend/zend_alloc_sizes.h b/Zend/zend_alloc_sizes.h
   index 9f1c00eaad..502b982a50 100644
   --- a/Zend/zend_alloc_sizes.h
   +++ b/Zend/zend_alloc_sizes.h
   @@ -19,7 +19,7 @@
    #ifndef ZEND_ALLOC_SIZES_H
    #define ZEND_ALLOC_SIZES_H
    
   -#define ZEND_MM_CHUNK_SIZE (2 * 1024 * 1024)               /* 2 MB  */
   +#define ZEND_MM_CHUNK_SIZE ((size_t) (2 * 1024 * 1024))    /* 2 MB  */
    #define ZEND_MM_PAGE_SIZE  (4 * 1024)                      /* 4 KB  */
    #define ZEND_MM_PAGES      (ZEND_MM_CHUNK_SIZE / ZEND_MM_PAGE_SIZE)  /* 512 */
    #define ZEND_MM_FIRST_PAGE (1)

   And then run the script as usual (without valgrind).
10. dvaeversted commented on Aug 19, 2022

    

    dvaeversted

    on Aug 19, 2022

    Author

    More actions

    Just tested with the above patch, and it indeeds solves the SEGFAULT. Which also makes me believe that @chschneider's reproducer is accurate :-)
    Both on a patched 8.1.9 and on a patched 8.2.0beta3

    👍React with 👍2Reacted by Ilija Tovilo and Christoph M. Becker
11. cmb69

    added a commit that references this issue on Aug 19, 2022

    Fix phpGH-9361: Segmentation fault on script exit

    ...

    This issue will close once commit 29a66c3 is merged into the 'master' branch.

    29a66c3
12. cmb69

    linked a pull request that will close this issueFix GH-9361: Segmentation fault on script exit #9379on Aug 19, 2022
13. cmb69

    added a commit that references this issue on Aug 22, 2022

    Fix GH-9361: Segmentation fault on script exit

    ...

    This issue will close once commit bb34121 is merged into the 'master' branch.

    bb34121
14. cmb69

    closed this as completedin bb34121on Aug 22, 2022
15. cmb69

    added 2 commits that reference this issue on Aug 22, 2022

    Merge branch 'PHP-8.0' into PHP-8.1

    ...

    This issue will close once commit 6427c4b is merged into the 'master' branch.

    6427c4b

    Merge branch 'PHP-8.1'

    ...

    This issue will close once commit a7d084b is merged into the 'master' branch.

    a7d084b
16. cmb69

    added

    Status: Verified

    and removed

    Status: Needs Triage

    on Aug 22, 2022