[1.3.0] segfault during test suite

[remicollet]

Encoutered on armv7hl, i386 and s390x (bigendian)

PHP 7.4: https://koji.fedoraproject.org/koji/taskinfo?taskID=53430519
PHP 7.3: https://koji.fedoraproject.org/koji/taskinfo?taskID=53430943

[remicollet]

On case, running 'Ds\Tests\QueueTest::testList'

Program received signal SIGSEGV, Segmentation fault.
zval_addref_p (container=<value optimized out>, dim=0xf618b0f0, dim_type=1, opline=0xf63cf4a8, execute_data=0xf74215d0) at /usr/src/debug/php-7.3.23/Zend/zend_types.h:1025
1025		return GC_ADDREF(Z_COUNTED_P(pz));
(gdb) bt
#0  zval_addref_p (container=<value optimized out>, dim=0xf618b0f0, dim_type=1, opline=0xf63cf4a8, execute_data=0xf74215d0) at /usr/src/debug/php-7.3.23/Zend/zend_types.h:1025
#1  zend_fetch_dimension_address_read (container=<value optimized out>, dim=0xf618b0f0, dim_type=1, opline=0xf63cf4a8, execute_data=0xf74215d0)
    at /usr/src/debug/php-7.3.23/Zend/zend_execute.c:2029
#2  zend_fetch_dimension_address_LIST_r (container=<value optimized out>, dim=0xf618b0f0, dim_type=1, opline=0xf63cf4a8, execute_data=0xf74215d0)
    at /usr/src/debug/php-7.3.23/Zend/zend_execute.c:2069
#3  0x082cc475 in ZEND_FETCH_LIST_R_SPEC_TMPVARCV_CONST_HANDLER (execute_data=<value optimized out>) at /usr/src/debug/php-7.3.23/Zend/zend_vm_execute.h:11093
#4  0x082c21e6 in execute_ex (ex=0xf74215d0) at /usr/src/debug/php-7.3.23/Zend/zend_vm_execute.h:55377
#5  0x08274037 in zend_call_function (fci=0xffff9fa8, fci_cache=0xffff9fd0) at /usr/src/debug/php-7.3.23/Zend/zend_execute_API.c:756
#6  0x081576fd in reflection_method_invoke (execute_data=0xf7421580, return_value=0xf7421200, variadic=0) at /usr/src/debug/php-7.3.23/ext/reflection/php_reflection.c:3186
#7  0x08157a75 in zim_reflection_method_invokeArgs (execute_data=<value optimized out>, return_value=<value optimized out>)
    at /usr/src/debug/php-7.3.23/ext/reflection/php_reflection.c:3222
#8  0x082f1461 in ZEND_DO_FCALL_SPEC_RETVAL_USED_HANDLER (execute_data=0xf7421030) at /usr/src/debug/php-7.3.23/Zend/zend_vm_execute.h:1106
#9  0x082c21e6 in execute_ex (ex=0xf741c020) at /usr/src/debug/php-7.3.23/Zend/zend_vm_execute.h:55377
#10 0x08317655 in zend_execute (op_array=0xf7479000, return_value=0x0) at /usr/src/debug/php-7.3.23/Zend/zend_vm_execute.h:60944
#11 0x0827fe62 in zend_execute_scripts (type=8, retval=0x0, file_count=3) at /usr/src/debug/php-7.3.23/Zend/zend.c:1568
#12 0x08219d14 in php_execute_script (primary_file=0xffffc3e8) at /usr/src/debug/php-7.3.23/main/main.c:2637
#13 0x0831af1c in do_cli (argc=8, argv=0x845a870) at /usr/src/debug/php-7.3.23/sapi/cli/php_cli.c:1000
#14 0x0831b77f in main (argc=8, argv=0x845a870) at /usr/src/debug/php-7.3.23/sapi/cli/php_cli.c:1396

[remicollet]

Digging in this extension is really a mess.

There is a BAD assumption that "zend_object *" can be cast to "php_ds_xxx_t *'

Since php 7 zend_object have to be the last entry of the php_ds_xxx_t structure

But this requires quite a lot of change

Ex, for Deque

--- ../ZTS/src/php/objects/php_deque.c  2020-10-14 06:27:02.000000000 +0200
+++ ./src/php/objects/php_deque.c       2020-10-14 14:55:05.224913291 +0200
@@ -6,7 +6,7 @@
 
 zend_object *php_ds_deque_create_object_ex(ds_deque_t *deque)
 {
-    php_ds_deque_t *obj = ecalloc(1, sizeof(php_ds_deque_t));
+    php_ds_deque_t *obj = zend_object_alloc(sizeof(php_ds_deque_t), php_ds_deque_ce);
     zend_object_std_init(&obj->std, php_ds_deque_ce);
     obj->std.handlers = &php_deque_handlers;
     obj->deque = deque;
--- ../ZTS/src/php/objects/php_deque.h  2020-10-14 06:27:02.000000000 +0200
+++ ./src/php/objects/php_deque.h       2020-10-14 15:29:33.431543315 +0200
@@ -3,7 +3,16 @@
 
 #include "../../ds/ds_deque.h"
 
-#define Z_DS_DEQUE(z)   ((php_ds_deque_t*) Z_OBJ(z))->deque
+typedef struct php_ds_deque {
+    ds_deque_t  *deque;
+    zend_object  std;
+} php_ds_deque_t;
+
+static inline php_ds_deque_t *php_ds_deque_fetch_object(zend_object *obj) {
+       return (php_ds_deque_t *)((char*)(obj) - XtOffsetOf(php_ds_deque_t, std));
+}
+
+#define Z_DS_DEQUE(z)   php_ds_deque_fetch_object(Z_OBJ(z))->deque
 #define Z_DS_DEQUE_P(z) Z_DS_DEQUE(*z)
 #define THIS_DS_DEQUE() Z_DS_DEQUE_P(getThis())
 
@@ -22,14 +31,6 @@
 
 
 /**
- *
- */
-typedef struct php_ds_deque {
-    zend_object  std;
-    ds_deque_t  *deque;
-} php_ds_deque_t;
-
-/**
  * Creates a new zend_object using an existing deque.
  */
 zend_object *php_ds_deque_create_object_ex(ds_deque_t *deque);
--- ../ZTS/src/php/handlers/php_deque_handlers.c        2020-10-14 06:27:02.000000000 +0200
+++ ./src/php/handlers/php_deque_handlers.c     2020-10-14 15:35:55.178833618 +0200
@@ -9,7 +9,7 @@
 static zval *php_ds_deque_read_dimension
 #if PHP_VERSION_ID >= 80000
 (zend_object *obj, zval *offset, int type, zval *return_value) {
-    ds_deque_t *deque = ((php_ds_deque_t*)obj)->deque;
+    ds_deque_t *deque = php_ds_deque_fetch_object(obj)->deque;
 #else
 (zval *obj, zval *offset, int type, zval *return_value) {
     ds_deque_t *deque = Z_DS_DEQUE_P(obj);
@@ -47,7 +47,7 @@
 static void php_ds_deque_write_dimension
 #if PHP_VERSION_ID >= 80000
 (zend_object *obj, zval *offset, zval *value) {
-    ds_deque_t *deque = ((php_ds_deque_t*)obj)->deque;
+    ds_deque_t *deque = php_ds_deque_fetch_object(obj)->deque;
 #else
 (zval *obj, zval *offset, zval *value) {
     ds_deque_t *deque = Z_DS_DEQUE_P(obj);
@@ -69,7 +69,7 @@
 static int php_ds_deque_has_dimension
 #if PHP_VERSION_ID >= 80000
 (zend_object *obj, zval *offset, int check_empty) {
-    ds_deque_t *deque = ((php_ds_deque_t*)obj)->deque;
+    ds_deque_t *deque = php_ds_deque_fetch_object(obj)->deque;
 #else
 (zval *obj, zval *offset, int check_empty) {
     ds_deque_t *deque = Z_DS_DEQUE_P(obj);
@@ -86,7 +86,7 @@
 static void php_ds_deque_unset_dimension
 #if PHP_VERSION_ID >= 80000
 (zend_object *obj, zval *offset) {
-    ds_deque_t *deque = ((php_ds_deque_t*)obj)->deque;
+    ds_deque_t *deque = php_ds_deque_fetch_object(obj)->deque;
 #else
 (zval *obj, zval *offset) {
     ds_deque_t *deque = Z_DS_DEQUE_P(obj);
@@ -111,16 +111,17 @@
 static int php_ds_deque_count_elements
 #if PHP_VERSION_ID >= 80000
 (zend_object *obj, zend_long *count) {
-    *count = ((php_ds_deque_t*)obj)->deque->size; return SUCCESS;
+    *count = php_ds_deque_fetch_object(obj)->deque->size;
 #else
 (zval *obj, zend_long *count) {
-    *count = Z_DS_DEQUE_P(obj)->size; return SUCCESS;
+    *count = Z_DS_DEQUE_P(obj)->size;
 #endif
+    return SUCCESS;
 }
 
 static void php_ds_deque_free_object(zend_object *object)
 {
-    php_ds_deque_t *obj = (php_ds_deque_t*) object;
+    php_ds_deque_t *obj = php_ds_deque_fetch_object(object);
     zend_object_std_dtor(&obj->std);
     ds_deque_free(obj->deque);
 }
@@ -128,7 +129,7 @@
 static HashTable *php_ds_deque_get_debug_info
 #if PHP_VERSION_ID >= 80000
 (zend_object *obj, int *is_temp) {
-    ds_deque_t *deque = ((php_ds_deque_t*)obj)->deque;
+    ds_deque_t *deque = php_ds_deque_fetch_object(obj)->deque;
 #else
 (zval *obj, int *is_temp) {
     ds_deque_t *deque = Z_DS_DEQUE_P(obj);
@@ -142,7 +143,7 @@
 static zend_object *php_ds_deque_clone_obj
 #if PHP_VERSION_ID >= 80000
 (zend_object *obj) {
-    return php_ds_deque_create_clone(((php_ds_deque_t*)obj)->deque);
+    return php_ds_deque_create_clone(php_ds_deque_fetch_object(obj)->deque);
 #else 
 (zval *obj) {
     return php_ds_deque_create_clone(Z_DS_DEQUE_P(obj));
@@ -152,7 +153,7 @@
 static HashTable *php_ds_deque_get_gc
 #if PHP_VERSION_ID >= 80000
 (zend_object *obj, zval **gc_data, int *gc_count) {
-    ds_deque_t *deque = ((php_ds_deque_t*)obj)->deque;
+    ds_deque_t *deque = php_ds_deque_fetch_object(obj)->deque;
 #else
 (zval *obj, zval **gc_data, int *gc_count) {
     ds_deque_t *deque = Z_DS_DEQUE_P(obj);

This probably explain memory corruption and thus segfault
BTW, I was not able to fix everything to have a clean ext... for now...

[rtheunissen]

There is a BAD assumption that "zend_object *" can be cast to "php_ds_xxx_t *'
Since php 7 zend_object have to be the last entry of the php_ds_xxx_t structure

That is only the case when class properties are supported, which none of these structures do. They are also final so there is no risk in having the std object as the first member of the struct.

I'll take a look at these failures, thank you for pointing them out.

This was my first extension and first time programming in C - I agree it's a bit of a mess. I would like to start from scratch for 2.0, considering to only support 8+

[remicollet]

I would like to start from scratch for 2.0

So please, also drop phpunit usage... this is a terrible idea
ALL php extensions use .phpt files ;)

[rtheunissen]

So please, also drop phpunit usage... this is a terrible idea

It was done so that the polyfill could be tested using the same suite, but using .phpt shouldn't prevent us from doing that.
I will use .phpt 👍

[remicollet]

If you want to have a look of WIP:

patch.txt

[rtheunissen]

@remicollet I'd be curious to know if this is still the case for 1.5.0

[remicollet]

@rtheunissen Indeed this is an old report (about 1.3.0)

IIRC 1.4.0 was already OK

I run a Fedora 40 build (PHP 8.3, s390x, i686, aarch64, ppc64le, x86_64)
https://koji.fedoraproject.org/koji/taskinfo?taskID=110588838

Also Fedora 39 (PHP 8.2, s390x, i686, aarch64, ppc64le, x86_64)
https://koji.fedoraproject.org/koji/taskinfo?taskID=110589008

I also run locally full test matrix

• Fedora 37, 38, 39, RHEL 7.9, 8.9 and 9.3
• PHP 7.4 to 8.3
• x86_64 and aarch64

Everything is OK, test suite (v1.5.1) passes everywhere