Changeset 148

Timestamp:

02/06/06 17:39:11 (2 years ago)

Author:

hrak

Message:

PHP_5_1: Quite some major changes, including but probably not limited to:
-Fixed numerous memory leaks
-Fixed the handling of empty string variables (as in ""), causing "String is not zero-terminated" warnings when PHP is compiled with --enable-debug
-More fixes to the handling of static properties (access checking/inheritance)
-Inheritance on restore is now handled by zend_do_inheritance
-Fixed handling of ZEND_OP_DATA and ZEND_INIT_METHOD_CALL by the optimizer
-Cosmetic fixes (spelling etc.)

Files:

• eaccelerator/branches/PHP_5_1/ChangeLog (modified) (1 diff)
• eaccelerator/branches/PHP_5_1/debug.c (modified) (1 diff)
• eaccelerator/branches/PHP_5_1/debug.h (modified) (1 diff)
• eaccelerator/branches/PHP_5_1/ea_restore.c (modified) (19 diffs)
• eaccelerator/branches/PHP_5_1/ea_store.c (modified) (14 diffs)
• eaccelerator/branches/PHP_5_1/eaccelerator.c (modified) (2 diffs)
• eaccelerator/branches/PHP_5_1/opcodes.c (modified) (1 diff)
• eaccelerator/branches/PHP_5_1/optimize.c (modified) (1 diff)

eaccelerator/branches/PHP_5_1/ChangeLog

@@ +1 @@
+2006-02-06  Hans Rakers <hans at parse dot nl>
+
+        * PHP_5_1: Quite some major changes, including but probably not limited to:
+          -Fixed numerous memory leaks
+          -Fixed the handling of empty string variables (as in ""), causing "String is not zero-terminated" warnings when PHP is compiled with --enable-debug
+          -More fixes to the handling of static properties (access checking/inheritance)
+          -Inheritance on restore is now handled by zend_do_inheritance
+          -Fixed handling of ZEND_OP_DATA and ZEND_INIT_METHOD_CALL by the optimizer
+          -Cosmetic fixes (spelling etc.)
+
-2006-01-16  Hans Rakers <hans at parse dot nl>
-

eaccelerator/branches/PHP_5_1/debug.c

@@ -247 +247 @@
-}
-
+/*
+ * This dumps a HashTable to debug output. Taken from zend_hash.c and slightly adapted.
+ */
+void ea_debug_hash_display(HashTable * ht)
+{
+        Bucket *p;
+        uint i;
+
+        fprintf(F_fp, "ht->nTableSize: %d\n", ht->nTableSize);
+        fprintf(F_fp, "ht->nNumOfElements: %d\n", ht->nNumOfElements);
+
+        for (i = 0; i < ht->nTableSize; i++) {
+                p = ht->arBuckets[i];
+                while (p != NULL) {
+                        fprintf(F_fp, "%s <==> 0x%lX\n", p->arKey, p->h);
+                        p = p->pNext;
+                }
+        }
+
+        p = ht->pListTail;
+        while (p != NULL) {
+                fprintf(F_fp, "%s <==> 0x%lX\n", p->arKey, p->h);
+                p = p->pListLast;
+        }
+        fflush(F_fp);
+}
+
-#endif /* #ifdef HAVE_EACCELERATOR */

eaccelerator/branches/PHP_5_1/debug.h

@@ -72 +72 @@
-long ea_debug_elapsed_time (struct timeval *tvstart);
-
+void ea_debug_hash_display(HashTable * ht);
+
-#endif /* INCLUDED_DEBUG_H */

eaccelerator/branches/PHP_5_1/ea_restore.c

@@ -276 +276 @@
-        fixup_hash(&from->properties_info,
-                           (fixup_bucket_t) fixup_property_info TSRMLS_CC);
+#  ifdef ZEND_ENGINE_2_1
+        fixup_zval_hash(&from->default_static_members);
+        if (from->static_members != NULL) {
+                FIXUP(from->static_members);
+                if (from->static_members != &from->default_static_members) {
+                        fixup_zval_hash(from->static_members);
+                }
+        }
+#  else
-        if (from->static_members != NULL) {
-                FIXUP(from->static_members);
-                fixup_zval_hash(from->static_members);
-        }
-#  ifdef ZEND_ENGINE_2_1
-        fixup_zval_hash(&from->default_static_members);
-#  endif
-#else
@@ -305 +312 @@
-        memcpy(p, from, sizeof(zval));
-        restore_zval(p TSRMLS_CC);
+        /* hrak: reset refcount to make sure there is one reference to this val, and prevent memleaks */
+        p->refcount = 1;
-        return p;
-}
@@ -379 +388 @@
-        case IS_STRING:
-                if (zv->value.str.val == NULL || 
-empty_string
+""
-                        zv->value.str.val = empty_string;
-                        return;
@@ -526 +535 @@
-         */
-        if (from->scope_name != NULL) {
-
-
-
+
+
-                        ea_debug_pad(EA_DEBUG TSRMLS_CC);
-hash. use EAG(class_entry).\n", getpid(), from_scope_lc
+class_table. use EAG(class_entry).\n", getpid(), from->scope_name
-                        to->scope = EAG(class_entry);
-                } else {
-                        ea_debug_pad(EA_DEBUG TSRMLS_CC);
-
-
-
-
+
+
+
-        } else {                                        // zoeloelip: is this needed? scope is always stored
-                ea_debug_pad(EA_DEBUG TSRMLS_CC);
@@ -590 +597 @@
-                        ea_debug_pad(EA_DEBUG TSRMLS_CC);
-                        ea_debug_printf(EA_DEBUG, "[%d]                                       can't find\n", getpid());
-
+              
-                return to;
-        }
+#ifdef ZEND_ENGINE_2
+        /* hrak: slight memleak here. dont forget to free the lowercase function name! */
+        if (fname_lc != NULL) {
+                efree(fname_lc);
+        }
+#endif
-        to->opcodes = from->opcodes;
-        to->last = to->size = from->last;
@@ -607 +620 @@
-        to->done_pass_two = 1;
-        to->filename = from->filename;
+/*      if (from->filename != NULL) {
+                size_t len = strlen(from->filename) + 1;
+                to->filename = emalloc(len);
+                memcpy(to->filename, from->filename, len);
+        }*/
+
-#ifdef ZEND_ENGINE_2
-        /* HOESH: try & catch support */
@@ -644 +663 @@
-        to->last_var         = from->last_var;
-        to->size_var         = 0;
-
+
-                zend_uint i;
-                to->vars = (zend_compiled_variable*)emalloc(from->last_var*sizeof(zend_compiled_variable));             
@@ -651 +670 @@
-                        to->vars[i].name = estrndup(from->vars[i].name, from->vars[i].name_len);
-                }
-
+*/
-#endif
-
@@ -689 +708 @@
-                                                  zend_class_entry * to TSRMLS_DC)
-{
-
-
-
+
+
+
+
+
+
-#else
-        if (zend_hash_find(CG(class_table), (void *) parent, len + 1, (void **) &to->parent) != SUCCESS)
@@ -702 +724 @@
-#ifdef ZEND_ENGINE_2
-                /* inherit parent methods */
-
-
+
+
+
+
+
+
-                to->destructor = to->parent->destructor;
-                to->clone = to->parent->clone;
@@ -719 +745 @@
-                to->handle_property_get = to->parent->handle_property_get;
-                to->handle_property_set = to->parent->handle_property_set;
-
-
-
-
-efree(name_lc);
+*/
+
+
+
+/*efree(name_lc);*/
-#endif
-}
@@ -742 +768 @@
-                fname_len = strlen(f->common.function_name);
-                fname_lc = zend_str_tolower_dup(f->common.function_name, fname_len);
+                ea_debug_printf(EA_DEBUG, "restore_class_methods: restoring function:%s scope:%s parent=%x scope=%x\n", fname_lc, ZEND_FN_SCOPE_NAME(f), to->parent, f->common.scope);
-
-                if (fname_len == cname_len && !memcmp(fname_lc, cname_lc, fname_len) && 
@@ -786 +813 @@
-        zend_class_entry *old;
-        zend_function *f = NULL;
-
-
-
-
-
+
+ */
+
+
+ */
-        Bucket *p;
-        union _zend_function *old_ctor;
@@ -815 +842 @@
-        to->num_interfaces = from->num_interfaces;
-        if (to->num_interfaces > 0) {
+                /* hrak: Allocate the slots which will later be populated by ZEND_ADD_INTERFACE */
-                to->interfaces = (zend_class_entry **) emalloc(sizeof(zend_class_entry *) * to->num_interfaces);
-                // should find out class entry. what the hell !!!
-                memset(to->interfaces, 0, sizeof(zend_class_entry *) * to->num_interfaces);
-        } else {
@@ -834 +861 @@
-        }
-
-
+/*
-                restore_class_parent(from->parent, strlen(from->parent), to TSRMLS_CC);
-        } else {
@@ -840 +867 @@
-                ea_debug_printf(EA_DEBUG, "[%d] parent = NULL\n", getpid());
-                to->parent = NULL;
-
+*/
-
-        old = EAG(class_entry);
@@ -850 +877 @@
-        to->line_start = from->line_start;
-        to->line_end = from->line_end;
+        ea_debug_printf(EA_DEBUG, "restore_class_entry: line_start=%d, line_end=%d", to->line_start, to->line_end);
-        to->doc_comment_len = from->doc_comment_len;
-
+/*
-                size_t len = strlen(from->filename) + 1;
-                to->filename = emalloc(len);
-                memcpy(to->filename, from->filename, len);
-
+
+
-        if (from->doc_comment != NULL) {
-                to->doc_comment = emalloc(from->doc_comment_len + 1);
@@ -877 +906 @@
-        to->default_static_members.pDestructor = ZVAL_PTR_DTOR;
-        
-
+
+
+
+
+
+
+
+
+
-#  else
-        if (from->static_members != NULL) {
@@ -904 +941 @@
-        to->function_table.pDestructor = ZEND_FUNCTION_DTOR;
-
+        if (from->parent != NULL) {
+                restore_class_parent(from->parent, strlen(from->parent), to TSRMLS_CC);
+        } else {
+                ea_debug_pad(EA_DEBUG TSRMLS_CC);
+                ea_debug_printf(EA_DEBUG, "[%d] parent = NULL\n", getpid());
+                to->parent = NULL;
+        }
+
-#ifdef ZEND_ENGINE_2
-        restore_class_methods(to TSRMLS_CC);

eaccelerator/branches/PHP_5_1/ea_store.c

@@ -123 +123 @@
-        case IS_CONSTANT:
-        case IS_STRING:
-               if (zv->value.str.val == NULL || zv->value.str.val == empty_string
-
+/*             if (zv->value.str.val == NULL
+*/
-                        calc_string(zv->value.str.val, zv->value.str.len + 1 TSRMLS_CC);
-
+
-                break;
-        case IS_ARRAY:
@@ -285 +285 @@
-                calc_string(from->parent->name, from->parent->name_length + 1 TSRMLS_CC);
-#ifdef ZEND_ENGINE_2
-#if 0
-        // what's problem. why from->interfaces[i] == 0x5a5a5a5a ?
-        for (i = 0; i < from->num_interfaces; i++) {
-                if (from->interfaces[i]) {
-                        calc_string(from->interfaces[i]->name,
-                                                from->interfaces[i]->name_length);
-                }
-        }
-#endif
-        if (from->filename != NULL)
-                calc_string(from->filename, strlen(from->filename) + 1 TSRMLS_CC);
@@ -302 +293 @@
-        calc_zval_hash(&from->default_properties);
-
-
+
+
+
-        calc_zval_hash(&from->default_static_members);
-
-
-
+
+
-        if (from->static_members != NULL) {
+#  endif
-                EACCELERATOR_ALIGN(EAG(mem));
-                EAG(mem) += sizeof(HashTable);
@@ -388 +381 @@
-
-typedef void *(*store_bucket_t) (void *TSRMLS_DC);
-
-
-
-
-
-
+
+
+
+
+
+
+
-
-#define store_zval_hash(to, from) \
-
+, NULL, NULL
-
-#define store_zval_hash_ex(to, from, start) \
-
+, NULL
-
-static zval *store_zval_ptr(zval * from TSRMLS_DC)
@@ -413 +407 @@
-
-static void store_hash_int(HashTable * target, HashTable * source,
-
+
+
-{
-        Bucket *p, *np, *prev_p;
@@ -436 +431 @@
-                np = NULL;
-                while (p) {
+                        /* If a check function has been defined, run it */
+                        if (check_bucket) {
+                                va_list args;
+                                va_start(args, check_bucket);
+
+                                /* If the check function returns ZEND_HASH_APPLY_REMOVE, don't store this record, skip over it */
+                                if(check_bucket(p, args)) {
+                                        p = p->pListNext;
+                                        target->nNumOfElements--;
+                                        /* skip to next itteration */
+                                        continue;
+                                }
+                                va_end(args);
+                        }
+
-                        EACCELERATOR_ALIGN(EAG(mem));
-                        np = (Bucket *) EAG(mem);
@@ -486 +496 @@
-        case IS_CONSTANT:
-        case IS_STRING:
-
-
-
-
-
-
-
+
-                break;
-        case IS_ARRAY:
@@ -631 +635 @@
-#endif
-        to->static_variables = from->static_variables;
-#ifdef ZEND_ENGINE_2_1
-        to->vars             = from->vars;
-        to->last_var         = from->last_var;
-#endif
-        to->return_reference = from->return_reference;
-        to->filename = from->filename;
@@ -742 +742 @@
-        return to;
-}
+
+/* 
+ * The following two functions handle access checking of properties (public/private/protected) 
+ * and control proper inheritance during copying of the properties_info and (default_)static_members hashes
+ *
+ * Both functions return ZEND_HASH_APPLY_REMOVE if the property to be copied needs to be skipped, or
+ * ZEND_HASH_APPLY_KEEP if the property needs to be copied over into the cache.
+ *  
+ * If the property is skipped due to access restrictions, or it needs inheritance of its value from the
+ * parent, the restore phase will take care of that.
+ *
+ * Most of the logic behind all this can be found in zend_compile.c, functions zend_do_inheritance and
+ * zend_do_inherit_property_access_check
+*/
+static int store_property_access_check(Bucket * p, va_list args)
+{
+        zend_class_entry *from = va_arg(args, zend_class_entry*);
+        zend_class_entry *parent = from->parent;
+        
+        ea_debug_printf(EA_DEBUG, "[%d] store_property_access_check enter. from=%x parent=%x arKey=%s\n", getpid(), from, parent, p->arKey);
+        
+        if (parent) {
+                // hra: TODO - do some usefull stuff :)
+                // check for ACC_PRIVATE etc.
+                // for now, just return keep
+        }
+        ea_debug_printf(EA_DEBUG, "[%d] store_property_access_check result: keep\n", getpid());
+        return ZEND_HASH_APPLY_KEEP;
+}
+
+static int store_static_member_access_check(Bucket * p, va_list args)
+{
+        zend_class_entry *from = va_arg(args, zend_class_entry*);
+        zend_class_entry *parent = from->parent;
+        zend_property_info *pinfo, *cinfo = NULL;
+        zval **pprop = NULL;
+        zval **cprop = p->pData;
+        char *mname, *cname = NULL;
+
+        /* Check if this is a parent class. If so, copy unconditionally */
+        if (parent) {
+                /* unpack the \0classname\0membername\0 style property name to seperate vars */
+                zend_unmangle_property_name(p->arKey, &cname, &mname);
+                ea_debug_printf(EA_DEBUG, "[%d] store_static_member_access_check: cname=%s, mname=%s\n", getpid(), cname, mname);
+        
+                /* lookup the member's info in parent and child */
+                if((zend_hash_find(&parent->properties_info, mname, strlen(mname)+1, (void**)&pinfo) == SUCCESS) &&
+                        (zend_hash_find(&from->properties_info, mname, strlen(mname)+1, (void**)&cinfo) == SUCCESS)) {
+                        /* don't copy this static property if protected in parent and static public in child.
+                           inheritance will handle this properly on restore */
+                        if(cinfo->flags & ZEND_ACC_STATIC && (pinfo->flags & ZEND_ACC_PROTECTED && cinfo->flags & ZEND_ACC_PUBLIC)) {
+                                ea_debug_printf(EA_DEBUG, "[%d] store_static_member_access_check: child static public, parent protected, result: skip\n", getpid());
+                                return ZEND_HASH_APPLY_REMOVE;
+                        }
+                        /* If the static member points to the same value in parent and child, remove for proper inheritance during restore */
+                        if(zend_hash_quick_find(&parent->default_static_members, p->arKey, p->nKeyLength, p->h, (void**)&pprop) == SUCCESS) {
+                                ea_debug_printf(EA_DEBUG, "[%d] store_static_member_access_check: SUCCESS looking up arKey\n",getpid());
+                                ea_debug_printf(EA_DEBUG, "[%d] store_static_member_access_check: pprop=%x cprop=%x\n",getpid(), *pprop, *cprop);
+                                if(*pprop == *cprop) {
+                                        ea_debug_printf(EA_DEBUG, "[%d] store_static_member_access_check: pprop == cprop, result: skip\n",getpid());
+                                        return ZEND_HASH_APPLY_REMOVE;
+                                }
+                        }
+                }
+        }
+        ea_debug_printf(EA_DEBUG, "[%d] store_static_member_access_check: result: keep\n",getpid());
+        return ZEND_HASH_APPLY_KEEP;
+}
+
-#endif
-
@@ -751 +820 @@
-        EAG(mem) += sizeof(eaccelerator_class_entry);
-        to->type = from->type;
+        ea_debug_printf(EA_DEBUG, "[%d] store_class_entry: class type=%d\n", getpid(), from->type);
-        to->name = NULL;
-        to->name_length = from->name_length;
@@ -758 +828 @@
-        to->static_members = NULL;
-        to->num_interfaces = from->num_interfaces;
-
-
-
-
-
-
-
-
-
-
-
-
-
+
+
+
-#endif
-
@@ -786 +846 @@
-                to->parent = store_string(from->parent->name, from->parent->name_length + 1 TSRMLS_CC);
-
-/*
-  if (!from->constants_updated) {
-    zend_hash_apply_with_argument(&from->default_properties, (apply_func_arg_t) zval_update_constant, (void *) 1 TSRMLS_CC);
-    to->constants_updated = 1;
-  }
-*/
-#ifdef ZEND_ENGINE_2
-        to->line_start = from->line_start;
@@ -808 +862 @@
-        store_zval_hash(&to->constants_table, &from->constants_table);
-        store_zval_hash(&to->default_properties, &from->default_properties);
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-        if (from->static_members != NULL) {
-                EACCELERATOR_ALIGN(EAG(mem));
@@ -815 +886 @@
-                store_zval_hash(to->static_members, from->static_members);
-        }
+#  endif
-#else
-        store_zval_hash(&to->default_properties, &from->default_properties);
-#endif
-
-
-
-
-
+
-
-#ifdef DEBUG

eaccelerator/branches/PHP_5_1/eaccelerator.c

@@ -884 +884 @@
-    return 0;
-  }
+  ea_debug_printf (EA_DEBUG, "[%d] eaccelerator_store: calc_size returned %d, mm=%x", getpid(), size, eaccelerator_mm_instance->mm);
-  EACCELERATOR_UNPROTECT();
-  EAG(mem) = eaccelerator_malloc(size);
@@ -1474 +1475 @@
-              (nreloads == 1) ? "cached" : "re-cached", file_handle->opened_path);
-      } else {
-n
+
-      }
-    } else {

eaccelerator/branches/PHP_5_1/opcodes.c

@@ -210 +210 @@
-  OPDEF("CLONE",                     EXT_UNUSED | OP1_STD    | OP2_UNUSED | RES_VAR), /* 110 */
-  OPDEF("INIT_CTOR_CALL",            EXT_UNUSED | OP1_STD    | OP2_UNUSED | RES_UNUSED), /* 111 */
-UNUSED
+VAR
-  OPDEF("INIT_STATIC_METHOD_CALL",   EXT_UNUSED | OP1_UCLASS | OP2_STD    | RES_UNUSED), /* 113 */
-  OPDEF("ISSET_ISEMPTY_VAR",         EXT_ISSET  | OP1_STD    | OP2_FETCH  | RES_TMP), /* 114 */

eaccelerator/branches/PHP_5_1/optimize.c

@@ -311 +311 @@
-             case ZEND_FE_FETCH:
-#ifdef ZEND_ENGINE_2
+             case ZEND_INIT_METHOD_CALL:
+             case ZEND_INIT_STATIC_METHOD_CALL:
-             case ZEND_ASSIGN_DIM:
-             case ZEND_DECLARE_CLASS: