Replace zend_object object with its header to prevent the "using flexible array in the middle of another struct" problem in its inheritted classes (static analyzer report)

[Snape3058]

Class zend_object is defined as a flexible array of length 1. The flexible array defined with size 1 and 0 is not the standard behavior. It is suggested to use the unsized definition (https://people.kernel.org/kees/bounded-flexible-arrays-in-c). Besides, not all its subclasses will use the array field properties_table of the zend_object class. If I understand the code correctly, when the properties_table[0] field is not used, it will store a ZVAL_UNDEF zval indicating the end of the iteration. Whereas when the properties_table[0] field is used, the flags and array length are checked first before accessing the data in the array.

If the properties_table[0] field is not used in these sub-classes, will it be better to replace the zend_object in these classes with only the header part of zend_object?

i.e. (as suggested in case 2 of https://lpc.events/event/18/contributions/1722/attachments/1591/3303/Wfamnae_lpceu2024.pdf)
We can define another struct with only the header part (let's name it zend_object_header_part), but leave the zend_object struct with both the header and the flexible array part.
When only the header is needed, we can use the zend_object_header_part (e.g. in the class inheritance), whereas for those requiring the array part, or using the object through a zend_object pointer, we can still use the full definition.

---

Usages of zend_object in the middle of other structs whose array field is potentially never used through the composite struct:

• php-src/Zend/zend_generators.h

  Lines 58 to 59 in c2fddac

  	struct _zend_generator {
  	zend_object std;

• php-src/Zend/zend_interfaces.c

  Lines 490 to 491 in c2fddac

  	typedef struct {
  	zend_object std;

• php-src/Zend/zend_closures.c

  Lines 31 to 32 in c2fddac

  	typedef struct _zend_closure {
  	zend_object std;

• php-src/Zend/zend_fibers.h

  Lines 102 to 104 in c2fddac

  	struct _zend_fiber {
  	/* PHP object handle. */
  	zend_object std;

• php-src/Zend/zend_iterators.h

  Lines 64 to 65 in c2fddac

  	struct _zend_object_iterator {
  	zend_object std;

• php-src/ext/opcache/jit/zend_jit_ir.c

  Lines 8451 to 8452 in c2fddac

  	typedef struct _zend_closure {
  	zend_object std;

• php-src/ext/com_dotnet/com_saproxy.c

  Lines 35 to 36 in c2fddac

  	typedef struct {
  	zend_object std;

• php-src/ext/com_dotnet/php_com_dotnet_internal.h

  Lines 28 to 29 in c2fddac

  	typedef struct _php_com_dotnet_object {
  	zend_object zo;

• php-src/ext/com_dotnet/com_persist.c

  Lines 278 to 279 in c2fddac

  	typedef struct {
  	zend_object std;

• php-src/ext/ffi/ffi.c

  Lines 169 to 170 in c2fddac

  	typedef struct _zend_ffi {
  	zend_object std;

• php-src/ext/ffi/ffi.c

  Lines 191 to 192 in c2fddac

  	typedef struct _zend_ffi_cdata {
  	zend_object std;

• php-src/ext/ffi/ffi.c

  Lines 199 to 200 in c2fddac

  	typedef struct _zend_ffi_ctype {
  	zend_object std;

• php-src/ext/pdo/php_pdo_driver.h

  Lines 645 to 646 in c2fddac

  	struct _pdo_row_t {
  	zend_object std;

• php-src/ext/zend_test/fiber.h

  Lines 24 to 25 in c2fddac

  	struct _zend_test_fiber {
  	zend_object std;

• php-src/ext/intl/normalizer/normalizer_class.h

  Lines 25 to 26 in c2fddac

  	typedef struct {
  	zend_object zo;

• php-src/ext/intl/locale/locale_class.h

  Lines 25 to 26 in c2fddac

  	typedef struct {
  	zend_object zo;

report ids: 250106-1639:1-6,8-17 (16 reports in total)

[cmb69]

Hmm, as of PHP 7.0.0, zend_object is supposed to be the last member (see https://www.npopov.com/2015/06/19/Internal-value-representation-in-PHP-7-part-2.html#objects-in-php-7). I thought only ext/com hasn't been changed in this regard, but apparently there are several other classes. I'll have a look at ext/com.

[cmb69]

php-src/Zend/zend_object_handlers.h

Line 206 in b14469b

/* offset of real object header (usually zero) */

Appears to be somewhat outdated.

[Snape3058]

Appears to be somewhat outdated.

Should we make changes to them? Or are they left for compatibility with the old code?

[cmb69]

Well, the comment should probably be updated, and the struct layouts should be changed.

[Snape3058]

I am not very familiar with the code base. I can draft patches for them but I still need other developers to help me to prevent possible mistakes.

I will try to resolve one of them in recent days and open a PR.

Thank you for your replies.

[cmb69]

See the following for a yet incomplete patch for com_saproxy`(ignore the changes to build.ninja):

 ext/com_dotnet/com_saproxy.c | 19 ++++++++++++-------
 win32/build/build.ninja      |  2 +-
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/ext/com_dotnet/com_saproxy.c b/ext/com_dotnet/com_saproxy.c
index ea0e9e47a1..3e12d695fe 100644
--- a/ext/com_dotnet/com_saproxy.c
+++ b/ext/com_dotnet/com_saproxy.c
@@ -33,7 +33,6 @@
 #include "Zend/zend_exceptions.h"
 
 typedef struct {
-	zend_object std;
 	/* the object we a proxying for; we hold a refcount to it */
 	php_com_dotnet_object *obj;
 
@@ -42,7 +41,7 @@ typedef struct {
 
 	/* this is an array whose size_is(dimensions) */
 	zval *indices;
-
+	zend_object std;
 } php_com_saproxy;
 
 typedef struct {
@@ -55,13 +54,19 @@ typedef struct {
 	LONG *indices;
 } php_com_saproxy_iter;
 
-#define SA_FETCH(zv)			(php_com_saproxy*)Z_OBJ_P(zv)
+static inline php_com_saproxy *php_com_saproxy_from_obj(zend_object *obj) {
+	return (php_com_saproxy *) ((char *) obj - XtOffsetOf(php_com_saproxy, std));
+}
+
+#define SA_FETCH(zv) php_com_saproxy_from_obj(Z_OBJ_P(zv))
 
 zend_object *php_com_saproxy_create_object(zend_class_entry *class_type)
 {
-	php_com_saproxy *intern = emalloc(sizeof(*intern));
-	memset(intern, 0, sizeof(*intern));
+	size_t block_len = sizeof(php_com_saproxy) + zend_object_properties_size(class_type);
+	php_com_saproxy *intern = emalloc(block_len);
+	memset(intern, 0, block_len);
 	zend_object_std_init(&intern->std, class_type);
+	object_properties_init(&intern->std, class_type);
 	return &intern->std;
 }
 
@@ -364,7 +369,7 @@ static zend_result saproxy_count_elements(zend_object *object, zend_long *count)
 
 static void saproxy_free_storage(zend_object *object)
 {
-	php_com_saproxy *proxy = (php_com_saproxy *)object;
+	php_com_saproxy *proxy = php_com_saproxy_from_obj(object);
 //???	int i;
 //???
 //???	for (i = 0; i < proxy->dimensions; i++) {
@@ -398,7 +403,7 @@ static zend_object* saproxy_clone(zend_object *object)
 }
 
 zend_object_handlers php_com_saproxy_handlers = {
-	0,
+	XtOffsetOf(php_com_saproxy, std),
 	saproxy_free_storage,
 	zend_objects_destroy_object,
 	saproxy_clone,
diff --git a/win32/build/build.ninja b/win32/build/build.ninja
index 71db89e375..45580ce028 100644
--- a/win32/build/build.ninja
+++ b/win32/build/build.ninja
@@ -23,7 +23,7 @@ PGOMGR="${PGOMGR}"
 PHP_BUILD=${PHP_BUILD}
 
 # See https://ninja-build.org/manual.html#_deps
-# msvc_deps_prefix = Hinweis: Einlesen der Datei:
+msvc_deps_prefix = Hinweis: Einlesen der Datei:
 
 MCFILE=${BUILD_DIR}\wsyslog.rc

[Girgias]

I've submitted #17606 for the pdo_row_t struct.

[Snape3058]

To make it clear, I added checkboxes to each case to be fixed.

[Snape3058]

I am working on _zend_generator now. I will check the box when I submit the PR.

[Snape3058]

@cmb69 I tried to make a change to zend_generator (Snape3058@ebe9ffc). However. it seems that the generators returned from a function (related to execute_data) cannot be addressed correctly after the change. Could you please help me with this patch? Where can I find the places creating and assigning the return values?

[cmb69]

@Snape3058, EX(return_value) is of type zval*, so I would assume that a zend_generator* is just cast to zval* somewhere. If I'm right, just sticking with the cast back to zend_generator* should work. Anyway, consider to file a draft PR, where such details can be better discussed.

[Snape3058]

@cmb69 The current version (Snape3058@ebe9ffc) contains the changes to all cases that I can find. But it cannot pass the test cases. Is it OK to draft a PR now?