Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:11936 Return-Path: Mailing-List: contact internals-help@lists.php.net; run by ezmlm Delivered-To: mailing list internals@lists.php.net Received: (qmail 90751 invoked by uid 1010); 5 Aug 2004 14:21:36 -0000 Delivered-To: ezmlm-scan-internals@lists.php.net Delivered-To: ezmlm-internals@lists.php.net Received: (qmail 90723 invoked by uid 1007); 5 Aug 2004 14:21:35 -0000 Message-ID: <20040805142135.90691.qmail@pb1.pair.com> To: internals@lists.php.net Date: Thu, 05 Aug 2004 15:21:57 +0100 User-Agent: Mozilla Thunderbird 0.7.2 (Windows/20040707) X-Accept-Language: en-us, en MIME-Version: 1.0 References: In-Reply-To: Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-Posted-By: 80.5.118.146 Subject: Re: emalloc and friends From: scottmacvicar@ntlworld.com (Scott MacVicar) Hi, I patched a fresh 4.3.8 and I end up with the following bt after applying the patch. #0 0x08182d91 in execute (op_array=0x823d540) at /usr/local/src/php-4.3.8/Zend/zend_execute.c:1293 #1 0x081744af in zend_execute_scripts (type=8, retval=0x0, file_count=3) at /usr/local/src/php-4.3.8/Zend/zend.c:891 #2 0x0813feab in php_execute_script (primary_file=0xbfffe9a0) at /usr/local/src/php-4.3.8/main/main.c:1734 #3 0x0818afe1 in main (argc=2, argv=0xbfffea34) at /usr/local/src/php-4.3.8/sapi/cli/php_cli.c:822 #4 0x42015704 in __libc_start_main () from /lib/tls/libc.so.6 regards, Scott Derick Rethans wrote: > Hei, > > I've been debugging some memory related problems lately and had the need > to disable the emalloc and friend stuff so that valgrind gave meaningful > errors. The attached patch adds some replacement defines to the > zend_alloc.h stuff and is attached. (You can swap between the two > ways by changing the USE_ZEND_ALLOC define). With this out of the way > I've been able to get some weird bugs out of Xdebug. > > While having this patch enabled I did some more testing, and in a large > script (eZ publish) valgrind was showing a lot of strange errors, such > as: > > ==8347== Invalid read of size 4 > ==8347== at 0x80CAEFF: execute (zend_execute_locks.h:7) > ==8347== by 0x80C8DE5: execute (zend_execute.c:1707) > ==8347== by 0x80C8DE5: execute (zend_execute.c:1707) > ==8347== by 0x80C8DE5: execute (zend_execute.c:1707) > ==8347== by 0x80C8DE5: execute (zend_execute.c:1707) > ==8347== by 0x80C8DE5: execute (zend_execute.c:1707) > ==8347== by 0x80C8DE5: execute (zend_execute.c:1707) > ==8347== by 0x80CA1D0: execute (zend_execute.c:2229) > ==8347== by 0x80C8DE5: execute (zend_execute.c:1707) > ==8347== by 0x80C8DE5: execute (zend_execute.c:1707) > ==8347== by 0x80C8DE5: execute (zend_execute.c:1707) > ==8347== by 0x80BC8D8: zend_execute_scripts (zend.c:891) > ==8347== Address 0x3D390DF8 is 12 bytes inside a block of size 40 free'd > ==8347== at 0x3C01F918: free (vg_replace_malloc.c:127) > ==8347== by 0x80C0729: zend_hash_del_key_or_index (zend_hash.c:529) > ==8347== by 0x80CA5E3: execute (zend_execute.c:2282) > ==8347== by 0x80C8DE5: execute (zend_execute.c:1707) > ==8347== by 0x80C8DE5: execute (zend_execute.c:1707) > ==8347== by 0x80C8DE5: execute (zend_execute.c:1707) > ==8347== by 0x80C8DE5: execute (zend_execute.c:1707) > ==8347== by 0x80C8DE5: execute (zend_execute.c:1707) > ==8347== by 0x80C8DE5: execute (zend_execute.c:1707) > ==8347== by 0x80CA1D0: execute (zend_execute.c:2229) > ==8347== by 0x80C8DE5: execute (zend_execute.c:1707) > ==8347== by 0x80C8DE5: execute (zend_execute.c:1707) > ==8347== by 0x80C8DE5: execute (zend_execute.c:1707) > ==8347== by 0x80BC8D8: zend_execute_scripts (zend.c:891) > (Full trace attached, this is created with valgrind /path/to/apache -X) > > This looks like something is used after it's freed but i've no idea > where to look for it. I believe this also causes the crashes that I've > seen with the first part of Sterling's and Thies' fhs patch (also > attached). Can somebody else test this with complex applications too or > perhaps somebody has a clue on what this might cause? > (Spoiler, the patches are for 4.3.x, not for 5.x) > > > regards, > Derick > > > ------------------------------------------------------------------------ > > Index: Zend/zend_alloc.h > =================================================================== > RCS file: /repository/Zend/Attic/zend_alloc.h,v > retrieving revision 1.39.4.6 > diff -u -p -r1.39.4.6 zend_alloc.h > --- Zend/zend_alloc.h 28 Aug 2003 14:46:51 -0000 1.39.4.6 > +++ Zend/zend_alloc.h 5 Aug 2004 11:55:54 -0000 > @@ -80,6 +80,10 @@ ZEND_API void *_erealloc(void *ptr, size > ZEND_API char *_estrdup(const char *s ZEND_FILE_LINE_DC ZEND_FILE_LINE_ORIG_DC) ZEND_ATTRIBUTE_MALLOC; > ZEND_API char *_estrndup(const char *s, unsigned int length ZEND_FILE_LINE_DC ZEND_FILE_LINE_ORIG_DC) ZEND_ATTRIBUTE_MALLOC; > > +#define USE_ZEND_ALLOC 1 > + > +#if USE_ZEND_ALLOC > + > /* Standard wrapper macros */ > #define emalloc(size) _emalloc((size) ZEND_FILE_LINE_CC ZEND_FILE_LINE_EMPTY_CC) > #define safe_emalloc(nmemb, size, offset) _safe_emalloc((nmemb), (size), (offset) ZEND_FILE_LINE_CC ZEND_FILE_LINE_EMPTY_CC) > @@ -111,6 +115,45 @@ ZEND_API char *_estrndup(const char *s, > #define safe_estrdup(ptr) ((ptr)?(estrdup(ptr)):(empty_string)) > #define safe_estrndup(ptr, len) ((ptr)?(estrndup((ptr), (len))):(empty_string)) > > +#else > + > +#define _GNU_SOURCE > +#include > +#undef _GNU_SOURCE > + > +/* Standard wrapper macros */ > +#define emalloc(size) malloc(size) > +#define safe_emalloc(nmemb, size, offset) malloc((nmemb) * (size) + (offset)) > +#define efree(ptr) free(ptr) > +#define ecalloc(nmemb, size) calloc((nmemb), (size)) > +#define erealloc(ptr, size) realloc((ptr), (size)) > +#define erealloc_recoverable(ptr, size) realloc((ptr), (size)) > +#define estrdup(s) strdup(s) > +#define estrndup(s, length) strndup((s), (length)) > + > +/* Relay wrapper macros */ > +#define emalloc_rel(size) malloc(size) > +#define safe_emalloc_rel(nmemb, size, offset) malloc((nmemb) * (size) + (offset)) > +#define efree_rel(ptr) free(ptr) > +#define ecalloc_rel(nmemb, size) calloc((nmemb), (size)) > +#define erealloc_rel(ptr, size) realloc((ptr), (size)) > +#define erealloc_recoverable_rel(ptr, size) realloc((ptr), (size)) > +#define estrdup_rel(s) strdup(s) > +#define estrndup_rel(s, length) strndup((s), (length)) > + > +/* Selective persistent/non persistent allocation macros */ > +#define pemalloc(size, persistent) malloc(size) > +#define pefree(ptr, persistent) free(ptr) > +#define pecalloc(nmemb, size, persistent) calloc((nmemb), (size)) > +#define perealloc(ptr, size, persistent) realloc((ptr), (size)) > +#define perealloc_recoverable(ptr, size, persistent) realloc((ptr), (size)) > +#define pestrdup(s, persistent) strdup(s) > + > +#define safe_estrdup(ptr) ((ptr)?(strdup(ptr)):(empty_string)) > +#define safe_estrndup(ptr, len) ((ptr)?(strndup((ptr), (len))):(empty_string)) > + > +#endif > + > ZEND_API int zend_set_memory_limit(unsigned int memory_limit); > > ZEND_API void start_memory_manager(TSRMLS_D); > > > ------------------------------------------------------------------------ > > Index: Zend/zend_execute.c > =================================================================== > RCS file: /repository/Zend/Attic/zend_execute.c,v > retrieving revision 1.316.2.37 > diff -u -p -r1.316.2.37 zend_execute.c > --- Zend/zend_execute.c 24 Jun 2004 16:39:47 -0000 1.316.2.37 > +++ Zend/zend_execute.c 5 Aug 2004 12:03:02 -0000 > @@ -18,6 +18,7 @@ > */ > > #define ZEND_INTENSIVE_DEBUGGING 0 > +#define CACHE_VAR_LOOKUP 1 > > #include > #include > @@ -607,6 +608,12 @@ static void zend_fetch_var_address(zend_ > break; > EMPTY_SWITCH_DEFAULT_CASE() > } > + } else { > + if (opline->op1.op_type == IS_CONST) { > + Ts[opline->result.u.var].var.store = retval; > + } else { > + Ts[opline->result.u.var].var.store = NULL; > + } > } > if (opline->op2.u.fetch_type == ZEND_FETCH_LOCAL) { > FREE_OP(Ts, &opline->op1, free_op1); > @@ -1029,6 +1036,26 @@ static int zend_check_symbol(zval **pz T > EX(opline)++; \ > continue; > > +#if CACHE_VAR_LOOKUP > + > +#define GET_VAR_ADDRESS(type) \ > + if (EX(Ts)[EX(opline)->result.u.var].var.store) { \ > + zval **retval; \ > +\ > + retval = EX(Ts)[EX(opline)->result.u.var].var.store; \ > + EX(Ts)[EX(opline)->result.u.var].var.ptr_ptr = retval; \ > + SELECTIVE_PZVAL_LOCK(*retval, &EX(opline)->result); \ > + } else { \ > + zend_fetch_var_address(EX(opline), EX(Ts), type TSRMLS_CC); \ > + } > + > +#else > + > +#define GET_VAR_ADDRESS(type) \ > + zend_fetch_var_address(EX(opline), EX(Ts), type TSRMLS_CC); > + > +#endif > + > ZEND_API void execute(zend_op_array *op_array TSRMLS_DC) > { > zend_execute_data execute_data; > @@ -1039,6 +1066,7 @@ ZEND_API void execute(zend_op_array *op_ > EX(object).ptr = NULL; > EX(op_array) = op_array; > EX(Ts) = (temp_variable *) do_alloca(sizeof(temp_variable)*op_array->T); > + memset(EX(Ts), 0, sizeof(temp_variable)*op_array->T); > EX(prev_execute_data) = EG(current_execute_data); > EX(original_in_execution)=EG(in_execution); > > @@ -1259,14 +1287,14 @@ binary_assign_op_addr: { > FREE_OP(EX(Ts), &EX(opline)->op1, EG(free_op1)); > NEXT_OPCODE(); > case ZEND_FETCH_R: > - zend_fetch_var_address(EX(opline), EX(Ts), BP_VAR_R TSRMLS_CC); > + GET_VAR_ADDRESS(BP_VAR_R); > AI_USE_PTR(EX(Ts)[EX(opline)->result.u.var].var); > NEXT_OPCODE(); > case ZEND_FETCH_W: > - zend_fetch_var_address(EX(opline), EX(Ts), BP_VAR_W TSRMLS_CC); > + GET_VAR_ADDRESS(BP_VAR_W); > NEXT_OPCODE(); > case ZEND_FETCH_RW: > - zend_fetch_var_address(EX(opline), EX(Ts), BP_VAR_RW TSRMLS_CC); > + GET_VAR_ADDRESS(BP_VAR_RW); > NEXT_OPCODE(); > case ZEND_FETCH_FUNC_ARG: > if (ARG_SHOULD_BE_SENT_BY_REF(EX(opline)->extended_value, EX(fbc), EX(fbc)->common.arg_types)) { > @@ -1279,7 +1307,7 @@ binary_assign_op_addr: { > } > NEXT_OPCODE(); > case ZEND_FETCH_UNSET: > - zend_fetch_var_address(EX(opline), EX(Ts), BP_VAR_R TSRMLS_CC); > + GET_VAR_ADDRESS(BP_VAR_R); > PZVAL_UNLOCK(*EX(Ts)[EX(opline)->result.u.var].var.ptr_ptr); > if (EX(Ts)[EX(opline)->result.u.var].var.ptr_ptr != &EG(uninitialized_zval_ptr)) { > SEPARATE_ZVAL_IF_NOT_REF(EX(Ts)[EX(opline)->result.u.var].var.ptr_ptr); > @@ -1287,7 +1315,7 @@ binary_assign_op_addr: { > PZVAL_LOCK(*EX(Ts)[EX(opline)->result.u.var].var.ptr_ptr); > NEXT_OPCODE(); > case ZEND_FETCH_IS: > - zend_fetch_var_address(EX(opline), EX(Ts), BP_VAR_IS TSRMLS_CC); > + GET_VAR_ADDRESS(BP_VAR_IS); > AI_USE_PTR(EX(Ts)[EX(opline)->result.u.var].var); > NEXT_OPCODE(); > case ZEND_FETCH_DIM_R: > @@ -2233,6 +2261,7 @@ send_by_ref: > } > NEXT_OPCODE(); > case ZEND_UNSET_VAR: { > + zval **value; > zval tmp, *variable = get_zval_ptr(&EX(opline)->op1, EX(Ts), &EG(free_op1), BP_VAR_R); > > if (variable->type != IS_STRING) { > @@ -2242,8 +2271,44 @@ send_by_ref: > variable = &tmp; > } > > - zend_hash_del(EG(active_symbol_table), variable->value.str.val, variable->value.str.len+1); > + if (zend_hash_find(EG(active_symbol_table), > + variable->value.str.val, variable->value.str.len+1, > + (void **) &value) == SUCCESS) { > + zend_op *opl, *eop; > + zend_execute_data *exd; > + int i; > + int org_refcount = (*value)->refcount; > + > + zend_hash_del(EG(active_symbol_table), variable->value.str.val, variable->value.str.len+1); > + > + /* this loop will invalidate all caches that we might have of value > + * ONLY if the value was actually destructed */ > + if (org_refcount == 1) { > + exd = EG(current_execute_data); > + while (exd) { > + if (exd->op_array) { > + opl = exd->op_array->opcodes; > + eop = opl + exd->op_array->last; > + while (opl < eop) { > + switch (opl->opcode) { > + case ZEND_FETCH_R: > + case ZEND_FETCH_W: > + case ZEND_FETCH_RW: > + case ZEND_FETCH_UNSET: > + case ZEND_FETCH_IS: > + if (exd->Ts[opl->result.u.var].var.store == value) { > + exd->Ts[opl->result.u.var].var.store = NULL; > + } > + break; > + } > + opl++; > + } > + } > > + exd = exd->prev_execute_data; > + } > + } > + } > if (variable == &tmp) { > zval_dtor(&tmp); > } > Index: Zend/zend_execute_globals.h > =================================================================== > RCS file: /repository/Zend/Attic/zend_execute_globals.h,v > retrieving revision 1.2.2.1 > diff -u -p -r1.2.2.1 zend_execute_globals.h > --- Zend/zend_execute_globals.h 31 Dec 2002 16:23:00 -0000 1.2.2.1 > +++ Zend/zend_execute_globals.h 5 Aug 2004 12:03:02 -0000 > @@ -31,6 +31,7 @@ typedef union _temp_variable { > struct { > zval **ptr_ptr; > zval *ptr; > + void *store; > } var; > struct { > zval tmp_var; /* a dummy */