Fix valgrind errors in phpdbg

Revert "We cannot safely assume that all op array will be refcount 0 after execution"
This reverts commit b6936adb58.

This change turns out to not have been a clever idea and was causing more weirdness than it helped...

Zend/zend_compile.h

@@ -726,7 +726,7 @@ ZEND_API zend_op_array *compile_filename(int type, zval *filename);
 ZEND_API int zend_execute_scripts(int type, zval *retval, int file_count, ...);
 ZEND_API int open_file_for_scanning(zend_file_handle *file_handle);
 ZEND_API void init_op_array(zend_op_array *op_array, zend_uchar type, int initial_ops_size);
-ZEND_API zend_bool destroy_op_array(zend_op_array *op_array);
+ZEND_API void destroy_op_array(zend_op_array *op_array);
 ZEND_API void zend_destroy_file_handle(zend_file_handle *file_handle);
 ZEND_API void zend_cleanup_user_class_data(zend_class_entry *ce);
 ZEND_API void zend_cleanup_internal_class_data(zend_class_entry *ce);

Zend/zend_opcode.c

@@ -335,7 +335,7 @@ void zend_class_add_ref(zval *zv)
 	ce->refcount++;
 }
 
-ZEND_API zend_bool destroy_op_array(zend_op_array *op_array)
+ZEND_API void destroy_op_array(zend_op_array *op_array)
 {
 	zval *literal = op_array->literals;
 	zval *end;
@@ -353,12 +353,8 @@ ZEND_API zend_bool destroy_op_array(zend_op_array *op_array)
 		op_array->run_time_cache = NULL;
 	}
 
-	if (!op_array->refcount) {
-		return 1;
-	}
-
-	if (--(*op_array->refcount) > 0) {
-		return 0;
+	if (!op_array->refcount || --(*op_array->refcount) > 0) {
+		return;
 	}
 
 	efree_size(op_array->refcount, sizeof(*(op_array->refcount)));
@@ -419,8 +415,6 @@ ZEND_API zend_bool destroy_op_array(zend_op_array *op_array)
 		}
 		efree(arg_info);
 	}
-
-	return 1;
 }
 
 void init_op(zend_op *op)

Zend/zend_vm_def.h

@@ -2364,9 +2364,8 @@ ZEND_VM_HELPER(zend_leave_helper, ANY, ANY)
 		ZEND_VM_LEAVE();
 	} else if (ZEND_CALL_KIND_EX(call_info) == ZEND_CALL_NESTED_CODE) {
 		zend_detach_symbol_table(execute_data);
-		if (EXPECTED(destroy_op_array(&EX(func)->op_array) != 0)) {
-			efree_size(EX(func), sizeof(zend_op_array));
-		}
+		destroy_op_array(&EX(func)->op_array);
+		efree_size(EX(func), sizeof(zend_op_array));
 		old_execute_data = execute_data;
 		execute_data = EG(current_execute_data) = EX(prev_execute_data);
 		zend_vm_stack_free_call_frame_ex(call_info, old_execute_data);
@@ -5455,7 +5454,7 @@ ZEND_VM_HANDLER(73, ZEND_INCLUDE_OR_EVAL, CONST|TMPVAR|CV, ANY)
 		}
 
 		call->prev_execute_data = execute_data;
-		i_init_code_execute_data(call, new_op_array, return_value);
+	    i_init_code_execute_data(call, new_op_array, return_value);
 		if (EXPECTED(zend_execute_ex == execute_ex)) {
 			ZEND_VM_ENTER();
 		} else {
@@ -5464,9 +5463,8 @@ ZEND_VM_HANDLER(73, ZEND_INCLUDE_OR_EVAL, CONST|TMPVAR|CV, ANY)
 			zend_vm_stack_free_call_frame(call);
 		}
 
-		if (EXPECTED(destroy_op_array(new_op_array) != 0)) {
-			efree_size(new_op_array, sizeof(zend_op_array));
-		}
+		destroy_op_array(new_op_array);
+		efree_size(new_op_array, sizeof(zend_op_array));
 		if (UNEXPECTED(EG(exception) != NULL)) {
 			zend_throw_exception_internal(NULL);
 			HANDLE_EXCEPTION();

Zend/zend_vm_execute.h

@@ -503,9 +503,8 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_leave_helper_SPEC(ZEND_OPCODE_
 		ZEND_VM_LEAVE();
 	} else if (ZEND_CALL_KIND_EX(call_info) == ZEND_CALL_NESTED_CODE) {
 		zend_detach_symbol_table(execute_data);
-		if (EXPECTED(destroy_op_array(&EX(func)->op_array) != 0)) {
-			efree_size(EX(func), sizeof(zend_op_array));
-		}
+		destroy_op_array(&EX(func)->op_array);
+		efree_size(EX(func), sizeof(zend_op_array));
 		old_execute_data = execute_data;
 		execute_data = EG(current_execute_data) = EX(prev_execute_data);
 		zend_vm_stack_free_call_frame_ex(call_info, old_execute_data);
@@ -3670,7 +3669,7 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_INCLUDE_OR_EVAL_SPEC_CONST_HAN
 		}
 
 		call->prev_execute_data = execute_data;
-		i_init_code_execute_data(call, new_op_array, return_value);
+	    i_init_code_execute_data(call, new_op_array, return_value);
 		if (EXPECTED(zend_execute_ex == execute_ex)) {
 			ZEND_VM_ENTER();
 		} else {
@@ -3679,9 +3678,8 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_INCLUDE_OR_EVAL_SPEC_CONST_HAN
 			zend_vm_stack_free_call_frame(call);
 		}
 
-		if (EXPECTED(destroy_op_array(new_op_array) != 0)) {
-			efree_size(new_op_array, sizeof(zend_op_array));
-		}
+		destroy_op_array(new_op_array);
+		efree_size(new_op_array, sizeof(zend_op_array));
 		if (UNEXPECTED(EG(exception) != NULL)) {
 			zend_throw_exception_internal(NULL);
 			HANDLE_EXCEPTION();
@@ -29068,7 +29066,7 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_INCLUDE_OR_EVAL_SPEC_CV_HANDLE
 		}
 
 		call->prev_execute_data = execute_data;
-		i_init_code_execute_data(call, new_op_array, return_value);
+	    i_init_code_execute_data(call, new_op_array, return_value);
 		if (EXPECTED(zend_execute_ex == execute_ex)) {
 			ZEND_VM_ENTER();
 		} else {
@@ -29077,9 +29075,8 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_INCLUDE_OR_EVAL_SPEC_CV_HANDLE
 			zend_vm_stack_free_call_frame(call);
 		}
 
-		if (EXPECTED(destroy_op_array(new_op_array) != 0)) {
-			efree_size(new_op_array, sizeof(zend_op_array));
-		}
+		destroy_op_array(new_op_array);
+		efree_size(new_op_array, sizeof(zend_op_array));
 		if (UNEXPECTED(EG(exception) != NULL)) {
 			zend_throw_exception_internal(NULL);
 			HANDLE_EXCEPTION();
@@ -40480,7 +40477,7 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_INCLUDE_OR_EVAL_SPEC_TMPVAR_HA
 		}
 
 		call->prev_execute_data = execute_data;
-		i_init_code_execute_data(call, new_op_array, return_value);
+	    i_init_code_execute_data(call, new_op_array, return_value);
 		if (EXPECTED(zend_execute_ex == execute_ex)) {
 			ZEND_VM_ENTER();
 		} else {
@@ -40489,9 +40486,8 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_INCLUDE_OR_EVAL_SPEC_TMPVAR_HA
 			zend_vm_stack_free_call_frame(call);
 		}
 
-		if (EXPECTED(destroy_op_array(new_op_array) != 0)) {
-			efree_size(new_op_array, sizeof(zend_op_array));
-		}
+		destroy_op_array(new_op_array);
+		efree_size(new_op_array, sizeof(zend_op_array));
 		if (UNEXPECTED(EG(exception) != NULL)) {
 			zend_throw_exception_internal(NULL);
 			HANDLE_EXCEPTION();

sapi/phpdbg/phpdbg.c

@@ -213,8 +213,8 @@ static PHP_RSHUTDOWN_FUNCTION(phpdbg) /* {{{ */
 	zend_hash_destroy(&PHPDBG_G(bp)[PHPDBG_BREAK_METHOD]);
 	zend_hash_destroy(&PHPDBG_G(bp)[PHPDBG_BREAK_COND]);
 	zend_hash_destroy(&PHPDBG_G(bp)[PHPDBG_BREAK_MAP]);
-	zend_hash_destroy(&PHPDBG_G(seek));
 	zend_hash_destroy(&PHPDBG_G(file_sources));
+	zend_hash_destroy(&PHPDBG_G(seek));
 	zend_hash_destroy(&PHPDBG_G(registered));
 	zend_hash_destroy(&PHPDBG_G(watchpoints));
 	zend_llist_destroy(&PHPDBG_G(watchlist_mem));
@@ -234,12 +234,6 @@ static PHP_RSHUTDOWN_FUNCTION(phpdbg) /* {{{ */
 		PHPDBG_G(oplog) = NULL;
 	}
 
-	if (PHPDBG_G(ops)) {
-		destroy_op_array(PHPDBG_G(ops));
-		efree(PHPDBG_G(ops));
-		PHPDBG_G(ops) = NULL;
-	}
-
 	if (PHPDBG_G(oplog_list)) {
 		phpdbg_oplog_list *cur = PHPDBG_G(oplog_list);
 		do {
@@ -558,8 +552,8 @@ static PHP_FUNCTION(phpdbg_get_executable)
 		phpdbg_file_source *source = zend_hash_find_ptr(&PHPDBG_G(file_sources), name);
 		if (source) {
 			phpdbg_oplog_fill_executable(
-				source->op_array,
-				phpdbg_add_empty_array(Z_ARR_P(return_value), source->op_array->filename),
+				&source->op_array,
+				phpdbg_add_empty_array(Z_ARR_P(return_value), source->op_array.filename),
 				by_opcode);
 		}
 	} ZEND_HASH_FOREACH_END();

sapi/phpdbg/phpdbg_list.c

@@ -294,35 +294,28 @@ zend_op_array *phpdbg_compile_file(zend_file_handle *file, int type) {
 zend_op_array *phpdbg_init_compile_file(zend_file_handle *file, int type) {
 	char *filename = (char *)(file->opened_path ? ZSTR_VAL(file->opened_path) : file->filename);
 	char resolved_path_buf[MAXPATHLEN];
-	zend_op_array *ret;
+	zend_op_array *op_array;
 	phpdbg_file_source *dataptr;
 
 	if (VCWD_REALPATH(filename, resolved_path_buf)) {
 		filename = resolved_path_buf;
 	}
 
-	ret = PHPDBG_G(init_compile_file)(file, type);
+	op_array = PHPDBG_G(init_compile_file)(file, type);
 
-	if (ret == NULL) {
+	if (op_array == NULL) {
 		return NULL;
 	}
 
 	dataptr = zend_hash_str_find_ptr(&PHPDBG_G(file_sources), filename, strlen(filename));
 	ZEND_ASSERT(dataptr != NULL);
 
-	dataptr->op_array = ret;
-	dataptr->destroy_op_array = 1;
-	if (dataptr->op_array) {
-		if (dataptr->op_array->refcount) {
-			++*dataptr->op_array->refcount;
-		} else {
-			dataptr->op_array->refcount = emalloc(sizeof(uint32_t));
-			*dataptr->op_array->refcount = 2;
-			dataptr->destroy_op_array = 0;
-		}
+	dataptr->op_array = *op_array;
+	if (dataptr->op_array.refcount) {
+		efree(op_array);
 	}
 
-	return ret;
+	return &dataptr->op_array;
 }
 
 void phpdbg_free_file_source(zval *zv) {
@@ -332,12 +325,7 @@ void phpdbg_free_file_source(zval *zv) {
 		efree(data->buf);
 	}
 
-	if (!data->destroy_op_array) {
-		efree(data->op_array->refcount);
-	}
-	if (!data->destroy_op_array || destroy_op_array(data->op_array)) {
-		efree(data->op_array);
-	}
+	destroy_op_array(&data->op_array);
 
 	efree(data);
 }

sapi/phpdbg/phpdbg_list.h

@@ -48,7 +48,7 @@ typedef struct {
 #if HAVE_MMAP
 	void *map;
 #endif
-	zend_op_array *op_array;
+	zend_op_array op_array;
 	zend_bool destroy_op_array;
 	uint lines;
 	uint line[1];

sapi/phpdbg/phpdbg_parser.h

@@ -1,21 +1,19 @@
+/* A Bison parser, made by GNU Bison 2.7.12-4996.  */
 
-/* A Bison parser, made by GNU Bison 2.4.1.  */
-
-/* Skeleton interface for Bison's Yacc-like parsers in C
-
-      Copyright (C) 1984, 1989, 1990, 2000, 2001, 2002, 2003, 2004, 2005, 2006
-   Free Software Foundation, Inc.
-
+/* Bison interface for Yacc-like parsers in C
+   
+      Copyright (C) 1984, 1989-1990, 2000-2013 Free Software Foundation, Inc.
+   
    This program is free software: you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
    the Free Software Foundation, either version 3 of the License, or
    (at your option) any later version.
-
+   
    This program is distributed in the hope that it will be useful,
    but WITHOUT ANY WARRANTY; without even the implied warranty of
    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
    GNU General Public License for more details.
-
+   
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
@@ -28,14 +26,22 @@
    special exception, which will cause the skeleton and the resulting
    Bison output files to be licensed under the GNU General Public
    License without this special exception.
-
+   
    This special exception was added by the Free Software Foundation in
    version 2.2 of Bison.  */
 
+#ifndef YY_PHPDBG_SAPI_PHPDBG_PHPDBG_PARSER_H_INCLUDED
+# define YY_PHPDBG_SAPI_PHPDBG_PHPDBG_PARSER_H_INCLUDED
+/* Enabling traces.  */
+#ifndef YYDEBUG
+# define YYDEBUG 0
+#endif
+#if YYDEBUG
+extern int phpdbg_debug;
+#endif
 /* "%code requires" blocks.  */
-
-/* Line 1676 of yacc.c  */
-#line 31 "sapi/phpdbg/phpdbg_parser.y"
+/* Line 2060 of yacc.c  */
+#line 36 "/Users/Bob/php-src-X/sapi/phpdbg/phpdbg_parser.y"
 
 #include "phpdbg.h"
 #ifndef YY_TYPEDEF_YY_SCANNER_T
@@ -44,9 +50,8 @@ typedef void* yyscan_t;
 #endif
 
 
-
-/* Line 1676 of yacc.c  */
-#line 50 "sapi/phpdbg/phpdbg_parser.h"
+/* Line 2060 of yacc.c  */
+#line 55 "sapi/phpdbg/phpdbg_parser.h"
 
 /* Tokens.  */
 #ifndef YYTOKENTYPE
@@ -75,6 +80,26 @@ typedef void* yyscan_t;
      T_REQ_ID = 276
    };
 #endif
+/* Tokens.  */
+#define T_EVAL 258
+#define T_RUN 259
+#define T_SHELL 260
+#define T_IF 261
+#define T_TRUTHY 262
+#define T_FALSY 263
+#define T_STRING 264
+#define T_COLON 265
+#define T_DCOLON 266
+#define T_POUND 267
+#define T_PROTO 268
+#define T_DIGITS 269
+#define T_LITERAL 270
+#define T_ADDR 271
+#define T_OPCODE 272
+#define T_ID 273
+#define T_INPUT 274
+#define T_UNEXPECTED 275
+#define T_REQ_ID 276
 
 
 
@@ -86,5 +111,18 @@ typedef int YYSTYPE;
 #endif
 
 
+#ifdef YYPARSE_PARAM
+#if defined __STDC__ || defined __cplusplus
+int phpdbg_parse (void *YYPARSE_PARAM);
+#else
+int phpdbg_parse ();
+#endif
+#else /* ! YYPARSE_PARAM */
+#if defined __STDC__ || defined __cplusplus
+int phpdbg_parse (void);
+#else
+int phpdbg_parse ();
+#endif
+#endif /* ! YYPARSE_PARAM */
 
-
+#endif /* !YY_PHPDBG_SAPI_PHPDBG_PHPDBG_PARSER_H_INCLUDED  */

sapi/phpdbg/phpdbg_parser.y

@@ -184,6 +184,10 @@ static int yyerror(const char *msg) {
 }
 
 int phpdbg_do_parse(phpdbg_param_t *stack, char *input) {
+	if (!*input) {
+		return 0;
+	}
+
 	phpdbg_init_lexer(stack, input);
 
 	return yyparse();

sapi/phpdbg/phpdbg_prompt.c

@@ -409,7 +409,7 @@ PHPDBG_COMMAND(exec) /* {{{ */
 
 				if (PHPDBG_G(ops)) {
 					phpdbg_notice("exec", "type=\"unsetops\"", "Destroying compiled opcodes");
-					phpdbg_clean(0);
+					zend_hash_clean(&PHPDBG_G(file_sources));
 				}
 
 				PHPDBG_G(exec) = res;
@@ -423,7 +423,7 @@ PHPDBG_COMMAND(exec) /* {{{ */
 				phpdbg_notice("exec", "type=\"set\" context=\"%s\"", "Set execution context: %s", PHPDBG_G(exec));
 
 				if (PHPDBG_G(in_execution)) {
-					phpdbg_clean(1);
+					PHPDBG_G(flags) |= PHPDBG_IS_CLEANING;
 					return SUCCESS;
 				}
 
@@ -651,7 +651,7 @@ PHPDBG_COMMAND(run) /* {{{ */
 		if (PHPDBG_G(in_execution)) {
 			if (phpdbg_ask_user_permission("Do you really want to restart execution?") == SUCCESS) {
 				phpdbg_startup_run++;
-				phpdbg_clean(1);
+				PHPDBG_G(flags) |= PHPDBG_IS_CLEANING;
 			}
 			return SUCCESS;
 		}
@@ -733,8 +733,7 @@ PHPDBG_COMMAND(run) /* {{{ */
 		}
 
 		PHPDBG_G(flags) &= ~PHPDBG_IS_RUNNING;
-
-		phpdbg_clean(1);
+		PHPDBG_G(flags) |= PHPDBG_IS_CLEANING;
 	} else {
 		phpdbg_error("inactive", "type=\"nocontext\"", "Nothing to execute!");
 	}
@@ -1236,10 +1235,9 @@ PHPDBG_COMMAND(clean) /* {{{ */
 	phpdbg_writeln("clean", "constants=\"%d\"", "Constants  %d", zend_hash_num_elements(EG(zend_constants)));
 	phpdbg_writeln("clean", "includes=\"%d\"", "Includes   %d", zend_hash_num_elements(&EG(included_files)));
 
-	phpdbg_clean(1);
-
 	phpdbg_xml("</cleaninfo>");
 
+	PHPDBG_G(flags) |= PHPDBG_IS_CLEANING;
 	return SUCCESS;
 } /* }}} */
 
@@ -1389,21 +1387,6 @@ int phpdbg_interactive(zend_bool allow_async_unsafe) /* {{{ */
 	return ret;
 } /* }}} */
 
-void phpdbg_clean(zend_bool full) /* {{{ */
-{
-	/* this is implicitly required */
-	if (PHPDBG_G(ops)) {
-		if (destroy_op_array(PHPDBG_G(ops))) {
-			efree(PHPDBG_G(ops));
-		}
-		PHPDBG_G(ops) = NULL;
-	}
-
-	if (full) {
-		PHPDBG_G(flags) |= PHPDBG_IS_CLEANING;
-	}
-} /* }}} */
-
 /* code may behave weirdly if EG(exception) is set; thus backup it */
 #define DO_INTERACTIVE(allow_async_unsafe) do { \
 	const zend_op *backup_opline; \

sapi/phpdbg/phpdbg_prompt.h

@@ -27,7 +27,6 @@ void phpdbg_init(char *init_file, size_t init_file_len, zend_bool use_default);
 void phpdbg_try_file_init(char *init_file, size_t init_file_len, zend_bool free_init);
 int phpdbg_interactive(zend_bool allow_async_unsafe);
 int phpdbg_compile(void);
-void phpdbg_clean(zend_bool full);
 void phpdbg_force_interruption(void);
 /* }}} */