Nullify references once not used to prevent double-frees

mikhainin commented 1 year ago

Basically, it looks self-explaining.

In a couple of places, we check if the value is NULL and free() it. However, if we start/stop the profiler a couple of times, the next time we (potentially) may not allow the value but still will try to free it and corrupt memory.

The issue with XHPROF_G(root) happened on our production, it was corrupting memory in another place (as the freed() chunk was given to another code), the problem with names[i] can be seen if we run tests having PHP built with --enable-debug.

Could you please take a look and let me know if there's something I can improve?

longxinH commented 1 year ago

Can you provide an example of repeated release of memory? Or an error message?

mikhainin commented 1 year ago

Hi, it's unfortunately quite complicated to reproduce - I failed to isolate the example, the crash happened on our production application (about 100,000 source files).

Something similar we can get if we build PHP with Address Sanitizer:

--with-iconv=/opt/homebrew/opt/libiconv --prefix=/opt/phpbuild --enable-debug --enable-debug-assertions --disable-phar --with-pcre-jit --enable-fpm --enable-mbstring=all CFLAGS="-O0 -ggdb3 -fno-omit-frame-pointer -fPIC -fsanitize=address -DZEND_TRACK_ARENA_ALLOC"

Build the extension with this PHP:

/opt/phpbuild/bin/phpize
./configure --with-php-config=/opt/phpbuild/bin/php-config

And then run the following code:


class Test {
};

for ($i = 0; $i < 1000000; ++ $i) {
  xhprof_enable();

  xhprof_disable();
  preg_match("/test$i/", "string");
  $a = new Test;
}

The output looks confusing as the problem is reported in another place. And I couldn't repeat it using USE_ZEND_ALLOC=0 with ASAN

AddressSanitizer:DEADLYSIGNAL
=================================================================
==7178==ERROR: AddressSanitizer: BUS on unknown address (pc 0x010122f29c00 bp 0x00016ef48090 sp 0x00016ef47f80 T0)
==7178==The signal is caused by a UNKNOWN memory access.
==7178==Hint: this fault was caused by a dereference of a high value address (see register values below).  Disassemble the provided pc to learn which register was used.
    #0 0x10122f29c00  (<unknown module>)
    #1 0x10122f684 in php_efree_pcre_cache php_pcre.c:171
    #2 0x101c4866c in zend_hash_destroy zend_hash.c:1577
    #3 0x10122ac70 in zm_deactivate_pcre php_pcre.c:518
    #4 0x101c15cf0 in zend_deactivate_modules zend_API.c:2711
    #5 0x1019cd7ec in php_request_shutdown main.c:1849
    #6 0x101f9f748 in do_cli php_cli.c:1111
    #7 0x101f9c4e0 in main php_cli.c:1341
    #8 0x103635088 in start+0x204 (dyld:arm64e+0x5088)

==7178==Register values:
 x[0] = 0x000000011669a3bf   x[1] = 0x0000000000000000   x[2] = 0x0000000000000008   x[3] = 0x000000000000013c  
 x[4] = 0x0000000000000000   x[5] = 0x0000000000000000   x[6] = 0x000000016af50000   x[7] = 0x0000000000000001  
 x[8] = 0x0000010122f29c00   x[9] = 0x000000011669a3bf  x[10] = 0x0000000000000008  x[11] = 0x0000000102d30560  
x[12] = 0xc0c06443ff0618c7  x[13] = 0x00000000ffffffff  x[14] = 0x000000016ef480c0  x[15] = 0x000000016ef482e0  
x[16] = 0x000000018c7ef0fc  x[17] = 0x0000000103de0720  x[18] = 0x0000000000000000  x[19] = 0x00000001036e4060  
x[20] = 0x0000000101f9b740  x[21] = 0x0000000103690070  x[22] = 0x0000000000000000  x[23] = 0x0000000000000000  
x[24] = 0x0000000000000000  x[25] = 0x0000000000000000  x[26] = 0x0000000000000000  x[27] = 0x0000000000000000  
x[28] = 0x0000000000000000     fp = 0x000000016ef48090     lr = 0x0000000100ff54bc     sp = 0x000000016ef47f80  
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: BUS (<unknown module>) 
==7178==ABORTING

longxinH / xhprof

Nullify references once not used to prevent double-frees #73