Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Segmentation fault on script exit #9361

Closed
dvaeversted opened this issue Aug 17, 2022 · 8 comments
Closed

Segmentation fault on script exit #9361

dvaeversted opened this issue Aug 17, 2022 · 8 comments

Comments

@dvaeversted
Copy link

Description

After updating from PHP 8.0.X to 8.1.9, we started experiencing segfaults on a memory heavy script.
We run it using the following command line:
php -d memory_limit=8192M cronscript.php

I was unable to construct a reproducer at this time, but no extensions are used, it is a script that reads in 3 different large files, does some calculations through a bunch of loops, and will crash with a segfault at the end of script execution (either on exit; or just when no more lines are left in the script).
We tested it on latest 8.2.0beta3, which exhibits the same problem, backtrace included as well.

From gdb i grabbed the following backtraces.

PHP 8.1.9:

Program received signal SIGSEGV, Segmentation fault.
0x00005555559825f8 in zend_set_memory_limit (memory_limit=8589934592) at /usr/src/debug/php-8.1.9/Zend/zend_alloc.c:2671
2671					heap->cached_chunks = p->next;
(gdb) bt
#0  0x00005555559825f8 in zend_set_memory_limit (memory_limit=8589934592) at /usr/src/debug/php-8.1.9/Zend/zend_alloc.c:2671
#1  0x00005555559465c0 in php_request_shutdown (dummy=dummy@entry=0x0) at /usr/src/debug/php-8.1.9/main/main.c:1879
#2  0x0000555555a8f2ae in do_cli (argc=4, argv=0x555555f75190) at /usr/src/debug/php-8.1.9/sapi/cli/php_cli.c:1135
#3  0x000055555578e2a5 in main (argc=4, argv=0x555555f75190) at /usr/src/debug/php-8.1.9/sapi/cli/php_cli.c:1367

PHP 8.2.0beta3:

Program received signal SIGSEGV, Segmentation fault.
0x00005555559929f8 in zend_set_memory_limit (memory_limit=8589934592) at /usr/src/debug/php-8.2.0beta3/Zend/zend_alloc.c:2692
2692					heap->cached_chunks = p->next;
Missing separate debuginfos, use: debuginfo-install gd3php-2.3.3-4.el7.remi.x86_64
(gdb) bt
#0  0x00005555559929f8 in zend_set_memory_limit (memory_limit=8589934592) at /usr/src/debug/php-8.2.0beta3/Zend/zend_alloc.c:2692
#1  0x0000555555956260 in php_request_shutdown (dummy=dummy@entry=0x0) at /usr/src/debug/php-8.2.0beta3/main/main.c:1890
#2  0x0000555555aa1e96 in do_cli (argc=4, argv=0x555555f75190) at /usr/src/debug/php-8.2.0beta3/sapi/cli/php_cli.c:1135
#3  0x000055555578f9bc in main (argc=4, argv=0x555555f75190) at /usr/src/debug/php-8.2.0beta3/sapi/cli/php_cli.c:1333

PHP Version

PHP 8.1.9

Operating System

Centos 7, AlmaLinux 8

@iluuu1994
Copy link
Member

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.

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.

@dvaeversted
Copy link
Author

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.

@iluuu1994
Copy link
Member

@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.

@chschneider
Copy link

chschneider commented Aug 18, 2022

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.

@cmb69
Copy link
Member

cmb69 commented Aug 18, 2022

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))

@dvaeversted
Copy link
Author

@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.

@cmb69
Copy link
Member

cmb69 commented Aug 19, 2022

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).

@dvaeversted
Copy link
Author

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

cmb69 added a commit to cmb69/php-src that referenced this issue Aug 19, 2022
Using a lot of memory may overflow some `int` calculations; to avoid
that we make sure that the operands are promoted to `size_t`.

This issue has been analyzed by @chschneider.
@cmb69 cmb69 linked a pull request Aug 19, 2022 that will close this issue
@cmb69 cmb69 closed this as completed in bb34121 Aug 22, 2022
cmb69 added a commit that referenced this issue Aug 22, 2022
* PHP-8.0:
  Fix GH-9361: Segmentation fault on script exit
cmb69 added a commit that referenced this issue Aug 22, 2022
* PHP-8.1:
  Fix GH-9361: Segmentation fault on script exit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
@chschneider @dvaeversted @iluuu1994 @cmb69 and others