Fixed bug #75218

I've introduced a new CompileError type, from which ParseError inherits. These errors are not parse errors in the narrow sense of the term, even though they happen to be generated during parsing in our implementation. Additionally reusing the ParseError class for this purpose would change existing error messages (if the exception is not caught) from a "Fatal error:" to a "Parse error:" prefix, and also the error kind from E_COMPILE_ERROR to E_PARSE.
php · Jun 16, 2018 · d04917c · d04917c
1 parent 0bd3fec
commit d04917c
Show file tree

Hide file tree

Showing 8 changed files with 79 additions and 21 deletions.
diff --git a/NEWS b/NEWS
@@ -26,6 +26,10 @@ PHP                                                                        NEWS
   . Fixed bug #76446 (zend_variables.c:73: zend_string_destroy: Assertion
     `!(zval_gc_flags((str)->gc)). (Nikita, Laruence)
 
+- Tokenizer:
+  . Fixed bug #75218 (Change remaining uncatchable fatal errors for parsing
+    into ParseError). (Nikita)
+
 - ZIP:
   . Fixed bug #76461 (OPSYS_Z_CPM defined instead of OPSYS_CPM).
     (Dennis Birkholz, Remi)

diff --git a/UPGRADING b/UPGRADING
@@ -102,6 +102,11 @@ Core:
     (RFC: https://wiki.php.net/rfc/list_reference_assignment)
   . instanceof now allows literals as the first operand,
     in which case the result is always FALSE.
+  . A new CompileError exception has been added, from which ParseError inherits.
+    A small number of compilation errors will now throw a CompileError instead
+    of generating a fatal error. Currently this only affects compilation errors
+    that may be thrown by token_get_all() in TOKEN_PARSE mode, but more errors
+    may be converted in the future.
 
 BCMath:
   . bcscale() can now also be used as getter to retrieve the current scale in use.

diff --git a/Zend/tests/bug75218.phpt b/Zend/tests/bug75218.phpt
@@ -0,0 +1,24 @@
+--TEST--
+Bug #75218: Change remaining uncatchable fatal errors for parsing into ParseError
+--FILE--
+<?php
+
+function try_eval($code) {
+    try {
+        eval($code);
+    } catch (CompileError $e) {
+        echo $e->getMessage(), "\n";
+    }
+}
+
+try_eval('if (false) {class C { final final function foo($fff) {}}}');
+try_eval('if (false) {class C { private protected $x; }}');
+try_eval('if (true) { __HALT_COMPILER(); }');
+try_eval('declare(encoding=[]);');
+
+?>
+--EXPECT--
+Multiple final modifiers are not allowed
+Multiple access type modifiers are not allowed
+__HALT_COMPILER() can only be used from the outermost scope
+Encoding must be a literal
diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
@@ -769,13 +769,18 @@ uint32_t zend_add_class_modifier(uint32_t flags, uint32_t new_flag) /* {{{ */
 {
 	uint32_t new_flags = flags | new_flag;
 	if ((flags & ZEND_ACC_EXPLICIT_ABSTRACT_CLASS) && (new_flag & ZEND_ACC_EXPLICIT_ABSTRACT_CLASS)) {
-		zend_error_noreturn(E_COMPILE_ERROR, "Multiple abstract modifiers are not allowed");
+		zend_throw_exception(zend_ce_compile_error,
+			"Multiple abstract modifiers are not allowed", 0);
+		return 0;
 	}
 	if ((flags & ZEND_ACC_FINAL) && (new_flag & ZEND_ACC_FINAL)) {
-		zend_error_noreturn(E_COMPILE_ERROR, "Multiple final modifiers are not allowed");
+		zend_throw_exception(zend_ce_compile_error, "Multiple final modifiers are not allowed", 0);
+		return 0;
 	}
 	if ((new_flags & ZEND_ACC_EXPLICIT_ABSTRACT_CLASS) && (new_flags & ZEND_ACC_FINAL)) {
-		zend_error_noreturn(E_COMPILE_ERROR, "Cannot use the final modifier on an abstract class");
+		zend_throw_exception(zend_ce_compile_error,
+			"Cannot use the final modifier on an abstract class", 0);
+		return 0;
 	}
 	return new_flags;
 }
@@ -785,19 +790,26 @@ uint32_t zend_add_member_modifier(uint32_t flags, uint32_t new_flag) /* {{{ */
 {
 	uint32_t new_flags = flags | new_flag;
 	if ((flags & ZEND_ACC_PPP_MASK) && (new_flag & ZEND_ACC_PPP_MASK)) {
-		zend_error_noreturn(E_COMPILE_ERROR, "Multiple access type modifiers are not allowed");
+		zend_throw_exception(zend_ce_compile_error,
+			"Multiple access type modifiers are not allowed", 0);
+		return 0;
 	}
 	if ((flags & ZEND_ACC_ABSTRACT) && (new_flag & ZEND_ACC_ABSTRACT)) {
-		zend_error_noreturn(E_COMPILE_ERROR, "Multiple abstract modifiers are not allowed");
+		zend_throw_exception(zend_ce_compile_error, "Multiple abstract modifiers are not allowed", 0);
+		return 0;
 	}
 	if ((flags & ZEND_ACC_STATIC) && (new_flag & ZEND_ACC_STATIC)) {
-		zend_error_noreturn(E_COMPILE_ERROR, "Multiple static modifiers are not allowed");
+		zend_throw_exception(zend_ce_compile_error, "Multiple static modifiers are not allowed", 0);
+		return 0;
 	}
 	if ((flags & ZEND_ACC_FINAL) && (new_flag & ZEND_ACC_FINAL)) {
-		zend_error_noreturn(E_COMPILE_ERROR, "Multiple final modifiers are not allowed");
+		zend_throw_exception(zend_ce_compile_error, "Multiple final modifiers are not allowed", 0);
+		return 0;
 	}
 	if ((new_flags & ZEND_ACC_ABSTRACT) && (new_flags & ZEND_ACC_FINAL)) {
-		zend_error_noreturn(E_COMPILE_ERROR, "Cannot use the final modifier on an abstract class member");
+		zend_throw_exception(zend_ce_compile_error,
+			"Cannot use the final modifier on an abstract class member", 0);
+		return 0;
 	}
 	return new_flags;
 }
@@ -5241,7 +5253,7 @@ void zend_compile_try(zend_ast *ast) /* {{{ */
 /* }}} */
 
 /* Encoding declarations must already be handled during parsing */
-void zend_handle_encoding_declaration(zend_ast *ast) /* {{{ */
+zend_bool zend_handle_encoding_declaration(zend_ast *ast) /* {{{ */
 {
 	zend_ast_list *declares = zend_ast_get_list(ast);
 	uint32_t i;
@@ -5253,7 +5265,8 @@ void zend_handle_encoding_declaration(zend_ast *ast) /* {{{ */
 
 		if (zend_string_equals_literal_ci(name, "encoding")) {
 			if (value_ast->kind != ZEND_AST_ZVAL) {
-				zend_error_noreturn(E_COMPILE_ERROR, "Encoding must be a literal");
+				zend_throw_exception(zend_ce_compile_error, "Encoding must be a literal", 0);
+				return 0;
 			}
 
 			if (CG(multibyte)) {
@@ -5286,6 +5299,8 @@ void zend_handle_encoding_declaration(zend_ast *ast) /* {{{ */
 			}
 		}
 	}
+
+	return 1;
 }
 /* }}} */
 

diff --git a/Zend/zend_compile.h b/Zend/zend_compile.h
@@ -723,7 +723,7 @@ zend_ast *zend_ast_append_str(zend_ast *left, zend_ast *right);
 zend_ast *zend_negate_num_string(zend_ast *ast);
 uint32_t zend_add_class_modifier(uint32_t flags, uint32_t new_flag);
 uint32_t zend_add_member_modifier(uint32_t flags, uint32_t new_flag);
-void zend_handle_encoding_declaration(zend_ast *ast);
+zend_bool zend_handle_encoding_declaration(zend_ast *ast);
 
 /* parser-driven code generators */
 void zend_do_free(znode *op1);

diff --git a/Zend/zend_exceptions.c b/Zend/zend_exceptions.c
@@ -34,6 +34,7 @@ ZEND_API zend_class_entry *zend_ce_throwable;
 ZEND_API zend_class_entry *zend_ce_exception;
 ZEND_API zend_class_entry *zend_ce_error_exception;
 ZEND_API zend_class_entry *zend_ce_error;
+ZEND_API zend_class_entry *zend_ce_compile_error;
 ZEND_API zend_class_entry *zend_ce_parse_error;
 ZEND_API zend_class_entry *zend_ce_type_error;
 ZEND_API zend_class_entry *zend_ce_argument_count_error;
@@ -154,7 +155,7 @@ ZEND_API ZEND_COLD void zend_throw_exception_internal(zval *exception) /* {{{ */
 		}
 	}
 	if (!EG(current_execute_data)) {
-		if (exception && Z_OBJCE_P(exception) == zend_ce_parse_error) {
+		if (exception && (Z_OBJCE_P(exception) == zend_ce_parse_error || Z_OBJCE_P(exception) == zend_ce_compile_error)) {
 			return;
 		}
 		if(EG(exception)) {
@@ -221,7 +222,8 @@ static zend_object *zend_default_exception_new_ex(zend_class_entry *class_type,
 
 	base_ce = i_get_exception_base(&obj);
 
-	if (EXPECTED(class_type != zend_ce_parse_error || !(filename = zend_get_compiled_filename()))) {
+	if (EXPECTED((class_type != zend_ce_parse_error && class_type != zend_ce_compile_error)
+			|| !(filename = zend_get_compiled_filename()))) {
 		ZVAL_STRING(&tmp, zend_get_executed_filename());
 		zend_update_property_ex(base_ce, &obj, ZSTR_KNOWN(ZEND_STR_FILE), &tmp);
 		zval_ptr_dtor(&tmp);
@@ -842,8 +844,12 @@ void zend_register_default_exception(void) /* {{{ */
 	zend_declare_property_null(zend_ce_error, "trace", sizeof("trace")-1, ZEND_ACC_PRIVATE);
 	zend_declare_property_null(zend_ce_error, "previous", sizeof("previous")-1, ZEND_ACC_PRIVATE);
 
+	INIT_CLASS_ENTRY(ce, "CompileError", NULL);
+	zend_ce_compile_error = zend_register_internal_class_ex(&ce, zend_ce_error);
+	zend_ce_compile_error->create_object = zend_default_exception_new;
+
 	INIT_CLASS_ENTRY(ce, "ParseError", NULL);
-	zend_ce_parse_error = zend_register_internal_class_ex(&ce, zend_ce_error);
+	zend_ce_parse_error = zend_register_internal_class_ex(&ce, zend_ce_compile_error);
 	zend_ce_parse_error->create_object = zend_default_exception_new;
 
 	INIT_CLASS_ENTRY(ce, "TypeError", NULL);
@@ -963,12 +969,13 @@ ZEND_API ZEND_COLD void zend_exception_error(zend_object *ex, int severity) /* {
 	ZVAL_OBJ(&exception, ex);
 	ce_exception = Z_OBJCE(exception);
 	EG(exception) = NULL;
-	if (ce_exception == zend_ce_parse_error) {
+	if (ce_exception == zend_ce_parse_error || ce_exception == zend_ce_compile_error) {
 		zend_string *message = zval_get_string(GET_PROPERTY(&exception, ZEND_STR_MESSAGE));
 		zend_string *file = zval_get_string(GET_PROPERTY_SILENT(&exception, ZEND_STR_FILE));
 		zend_long line = zval_get_long(GET_PROPERTY_SILENT(&exception, ZEND_STR_LINE));
 
-		zend_error_helper(E_PARSE, ZSTR_VAL(file), line, "%s", ZSTR_VAL(message));
+		zend_error_helper(ce_exception == zend_ce_parse_error ? E_PARSE : E_COMPILE_ERROR,
+			ZSTR_VAL(file), line, "%s", ZSTR_VAL(message));
 
 		zend_string_release_ex(file, 0);
 		zend_string_release_ex(message, 0);

diff --git a/Zend/zend_exceptions.h b/Zend/zend_exceptions.h
@@ -30,6 +30,7 @@ extern ZEND_API zend_class_entry *zend_ce_throwable;
 extern ZEND_API zend_class_entry *zend_ce_exception;
 extern ZEND_API zend_class_entry *zend_ce_error_exception;
 extern ZEND_API zend_class_entry *zend_ce_error;
+extern ZEND_API zend_class_entry *zend_ce_compile_error;
 extern ZEND_API zend_class_entry *zend_ce_parse_error;
 extern ZEND_API zend_class_entry *zend_ce_type_error;
 extern ZEND_API zend_class_entry *zend_ce_argument_count_error;

diff --git a/Zend/zend_language_parser.y b/Zend/zend_language_parser.y
@@ -28,6 +28,7 @@
 #include "zend_API.h"
 #include "zend_constants.h"
 #include "zend_language_scanner.h"
+#include "zend_exceptions.h"
 
 #define YYSIZE_T size_t
 #define yytnamerr zend_yytnamerr
@@ -414,8 +415,8 @@ inner_statement:
 	|	trait_declaration_statement			{ $$ = $1; }
 	|	interface_declaration_statement		{ $$ = $1; }
 	|	T_HALT_COMPILER '(' ')' ';'
-			{ $$ = NULL; zend_error_noreturn(E_COMPILE_ERROR,
-			      "__HALT_COMPILER() can only be used from the outermost scope"); }
+			{ $$ = NULL; zend_throw_exception(zend_ce_compile_error,
+			      "__HALT_COMPILER() can only be used from the outermost scope", 0); YYERROR; }
 ;
 
 
@@ -446,7 +447,7 @@ statement:
 		foreach_statement
 			{ $$ = zend_ast_create(ZEND_AST_FOREACH, $3, $7, $5, $9); }
 	|	T_DECLARE '(' const_list ')'
-			{ zend_handle_encoding_declaration($3); }
+			{ if (!zend_handle_encoding_declaration($3)) { YYERROR; } }
 		declare_statement
 			{ $$ = zend_ast_create(ZEND_AST_DECLARE, $3, $6); }
 	|	';'	/* empty statement */ { $$ = NULL; }
@@ -511,7 +512,8 @@ class_declaration_statement:
 
 class_modifiers:
 		class_modifier 					{ $$ = $1; }
-	|	class_modifiers class_modifier 	{ $$ = zend_add_class_modifier($1, $2); }
+	|	class_modifiers class_modifier
+			{ $$ = zend_add_class_modifier($1, $2); if (!$$) { YYERROR; } }
 ;
 
 class_modifier:
@@ -797,7 +799,7 @@ method_modifiers:
 non_empty_member_modifiers:
 		member_modifier			{ $$ = $1; }
 	|	non_empty_member_modifiers member_modifier
-			{ $$ = zend_add_member_modifier($1, $2); }
+			{ $$ = zend_add_member_modifier($1, $2); if (!$$) { YYERROR; } }
 ;
 
 member_modifier: