summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNikita Popov <nikic@php.net>2016-10-06 23:09:41 +0200
committerNikita Popov <nikic@php.net>2016-10-07 00:12:55 +0200
commit8754b191f77efdbb3d5dd6440e0546589fb65473 (patch)
treea99fcfdd1c7f8ab20a6f4e7f87850e771affab4b
parent940731df04f95d1ced5c336da39248f7229a8ddc (diff)
downloadphp-git-8754b191f77efdbb3d5dd6440e0546589fb65473.tar.gz
Fix "already in use" check inconsistencies/bugs
This fixes the following issues: * "use function" and "use const" inside namespaced code were checking for conflicts against class imports. Now they always check against the correct symbol type. * Symbol conflicts are now always checked within a single file only. Previously class uses inside namespaced code were checked globally. This behavior is illegal because symbols from other files are not visible if opcache is used, resulting in behavioral discrepancies. Additionally this made the presence/absence of symbol errors dependent on autoloading order, which is volatile. * The "single file" restriction is now enforced by collecting defined symbols inside a separate hash table. Previously it was enforced (for the non-namespaced case) by comparing the filename of the symbol declaration. However this is inaccurate if the same filename is used multiple times, such as may happen if eval() is used. * Additionally the previous approach relies on symbols being registered at compile-time, which is not the case for late-bound classes, which makes the behavior dependent on class declaration order, as well as opcache (which may cause delayed early-binding). * Lastly, conflicts are now consistently checked for conditionally defined symbols. Previously only declaration-after-use conflicts were checked in this case. Now use-after-declaration conflicts are detected as well.
-rw-r--r--Zend/tests/use_function/conditional_function_declaration.phpt6
-rw-r--r--Zend/tests/use_function/no_conflict_with_classes.phpt15
-rw-r--r--Zend/tests/use_late_binding_conflict.phpt13
-rw-r--r--Zend/tests/use_no_eval_conflict.phpt13
-rw-r--r--Zend/tests/use_no_file_conflict.phpt12
-rw-r--r--Zend/tests/use_no_file_conflict_1.inc4
-rw-r--r--Zend/tests/use_no_file_conflict_2.inc4
-rw-r--r--Zend/zend_compile.c75
-rw-r--r--Zend/zend_compile.h7
-rw-r--r--Zend/zend_globals.h2
-rw-r--r--Zend/zend_language_parser.y8
11 files changed, 108 insertions, 51 deletions
diff --git a/Zend/tests/use_function/conditional_function_declaration.phpt b/Zend/tests/use_function/conditional_function_declaration.phpt
index ccfb96103a..02ac0803f0 100644
--- a/Zend/tests/use_function/conditional_function_declaration.phpt
+++ b/Zend/tests/use_function/conditional_function_declaration.phpt
@@ -1,5 +1,5 @@
--TEST--
-function that is conditionally defined at runtime should not cause compiler error
+function that is conditionally defined is subject to symbol use checks
--FILE--
<?php
@@ -13,5 +13,5 @@ use function bar\foo;
echo "Done";
?>
---EXPECT--
-Done
+--EXPECTF--
+Fatal error: Cannot use function bar\foo as foo because the name is already in use in %s on line %d
diff --git a/Zend/tests/use_function/no_conflict_with_classes.phpt b/Zend/tests/use_function/no_conflict_with_classes.phpt
new file mode 100644
index 0000000000..bde94afb03
--- /dev/null
+++ b/Zend/tests/use_function/no_conflict_with_classes.phpt
@@ -0,0 +1,15 @@
+--TEST--
+"use function" should not conflict with class names
+--FILE--
+<?php
+
+namespace Foo;
+
+class Bar {}
+
+use function bar;
+
+?>
+===DONE===
+--EXPECT--
+===DONE===
diff --git a/Zend/tests/use_late_binding_conflict.phpt b/Zend/tests/use_late_binding_conflict.phpt
new file mode 100644
index 0000000000..c8514d0b1a
--- /dev/null
+++ b/Zend/tests/use_late_binding_conflict.phpt
@@ -0,0 +1,13 @@
+--TEST--
+Use conflicts are detected for late-bound classes
+--FILE--
+<?php
+
+/* Reverse declaration order disables early-binding */
+class B extends A {}
+class A {}
+use Foo\B;
+
+?>
+--EXPECTF--
+Fatal error: Cannot use Foo\B as B because the name is already in use in %s on line %d
diff --git a/Zend/tests/use_no_eval_conflict.phpt b/Zend/tests/use_no_eval_conflict.phpt
new file mode 100644
index 0000000000..cf9014b77d
--- /dev/null
+++ b/Zend/tests/use_no_eval_conflict.phpt
@@ -0,0 +1,13 @@
+--TEST--
+Use conflicts should not occur across eval()s
+--FILE--
+<?php
+
+/* It is important that these two eval()s occur on the same line,
+ * as this forces them to have the same filename. */
+eval("class A {}"); eval("use Foo\A;");
+
+?>
+===DONE===
+--EXPECT--
+===DONE===
diff --git a/Zend/tests/use_no_file_conflict.phpt b/Zend/tests/use_no_file_conflict.phpt
new file mode 100644
index 0000000000..9423995af3
--- /dev/null
+++ b/Zend/tests/use_no_file_conflict.phpt
@@ -0,0 +1,12 @@
+--TEST--
+Use conflicts should not occur across files
+--FILE--
+<?php
+
+require __DIR__ . '/use_no_file_conflict_1.inc';
+require __DIR__ . '/use_no_file_conflict_2.inc';
+
+?>
+===DONE===
+--EXPECT--
+===DONE===
diff --git a/Zend/tests/use_no_file_conflict_1.inc b/Zend/tests/use_no_file_conflict_1.inc
new file mode 100644
index 0000000000..c2739ff64d
--- /dev/null
+++ b/Zend/tests/use_no_file_conflict_1.inc
@@ -0,0 +1,4 @@
+<?php
+
+namespace Foo;
+class A {}
diff --git a/Zend/tests/use_no_file_conflict_2.inc b/Zend/tests/use_no_file_conflict_2.inc
new file mode 100644
index 0000000000..badcc85bea
--- /dev/null
+++ b/Zend/tests/use_no_file_conflict_2.inc
@@ -0,0 +1,4 @@
+<?php
+
+namespace Foo;
+use A;
diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
index 04fd3adffa..d56e670d9b 100644
--- a/Zend/zend_compile.c
+++ b/Zend/zend_compile.c
@@ -301,12 +301,14 @@ void zend_file_context_begin(zend_file_context *prev_context) /* {{{ */
FC(in_namespace) = 0;
FC(has_bracketed_namespaces) = 0;
FC(declarables).ticks = 0;
+ zend_hash_init(&FC(seen_symbols), 8, NULL, NULL, 0);
}
/* }}} */
void zend_file_context_end(zend_file_context *prev_context) /* {{{ */
{
zend_end_namespace();
+ zend_hash_destroy(&FC(seen_symbols));
CG(file_context) = *prev_context;
}
/* }}} */
@@ -318,12 +320,27 @@ void zend_init_compiler_data_structures(void) /* {{{ */
CG(active_class_entry) = NULL;
CG(in_compilation) = 0;
CG(start_lineno) = 0;
- zend_hash_init(&CG(const_filenames), 8, NULL, NULL, 0);
CG(encoding_declared) = 0;
}
/* }}} */
+static void zend_register_seen_symbol(zend_string *name, uint32_t kind) {
+ zval *zv = zend_hash_find(&FC(seen_symbols), name);
+ if (zv) {
+ Z_LVAL_P(zv) |= kind;
+ } else {
+ zval tmp;
+ ZVAL_LONG(&tmp, kind);
+ zend_hash_add_new(&FC(seen_symbols), name, &tmp);
+ }
+}
+
+static zend_bool zend_have_seen_symbol(zend_string *name, uint32_t kind) {
+ zval *zv = zend_hash_find(&FC(seen_symbols), name);
+ return zv && (Z_LVAL_P(zv) & kind) != 0;
+}
+
ZEND_API void file_handle_dtor(zend_file_handle *fh) /* {{{ */
{
@@ -349,7 +366,6 @@ void shutdown_compiler(void) /* {{{ */
zend_stack_destroy(&CG(loop_var_stack));
zend_stack_destroy(&CG(delayed_oplines_stack));
zend_hash_destroy(&CG(filenames_table));
- zend_hash_destroy(&CG(const_filenames));
zend_arena_destroy(CG(arena));
}
/* }}} */
@@ -5515,6 +5531,7 @@ static void zend_begin_func_decl(znode *result, zend_op_array *op_array, zend_as
key = zend_build_runtime_definition_key(lcname, decl->lex_pos);
zend_hash_update_ptr(CG(function_table), key, op_array);
+ zend_register_seen_symbol(lcname, ZEND_SYMBOL_FUNCTION);
if (op_array->fn_flags & ZEND_ACC_CLOSURE) {
opline = zend_emit_op_tmp(result, ZEND_DECLARE_LAMBDA_FUNCTION, NULL, NULL);
@@ -5916,6 +5933,8 @@ void zend_compile_class_decl(zend_ast *ast) /* {{{ */
"because the name is already in use", ZSTR_VAL(name));
}
}
+
+ zend_register_seen_symbol(lcname, ZEND_SYMBOL_CLASS);
} else {
name = zend_generate_anon_class_name(decl->lex_pos);
lcname = zend_string_tolower(name);
@@ -6083,19 +6102,19 @@ void zend_compile_class_decl(zend_ast *ast) /* {{{ */
static HashTable *zend_get_import_ht(uint32_t type) /* {{{ */
{
switch (type) {
- case T_CLASS:
+ case ZEND_SYMBOL_CLASS:
if (!FC(imports)) {
FC(imports) = emalloc(sizeof(HashTable));
zend_hash_init(FC(imports), 8, NULL, str_dtor, 0);
}
return FC(imports);
- case T_FUNCTION:
+ case ZEND_SYMBOL_FUNCTION:
if (!FC(imports_function)) {
FC(imports_function) = emalloc(sizeof(HashTable));
zend_hash_init(FC(imports_function), 8, NULL, str_dtor, 0);
}
return FC(imports_function);
- case T_CONST:
+ case ZEND_SYMBOL_CONST:
if (!FC(imports_const)) {
FC(imports_const) = emalloc(sizeof(HashTable));
zend_hash_init(FC(imports_const), 8, NULL, str_dtor, 0);
@@ -6111,11 +6130,11 @@ static HashTable *zend_get_import_ht(uint32_t type) /* {{{ */
static char *zend_get_use_type_str(uint32_t type) /* {{{ */
{
switch (type) {
- case T_CLASS:
+ case ZEND_SYMBOL_CLASS:
return "";
- case T_FUNCTION:
+ case ZEND_SYMBOL_FUNCTION:
return " function";
- case T_CONST:
+ case ZEND_SYMBOL_CONST:
return " const";
EMPTY_SWITCH_DEFAULT_CASE()
}
@@ -6142,7 +6161,7 @@ void zend_compile_use(zend_ast *ast) /* {{{ */
zend_string *current_ns = FC(current_namespace);
uint32_t type = ast->attr;
HashTable *current_import = zend_get_import_ht(type);
- zend_bool case_sensitive = type == T_CONST;
+ zend_bool case_sensitive = type == ZEND_SYMBOL_CONST;
for (i = 0; i < list->children; ++i) {
zend_ast *use_ast = list->child[i];
@@ -6180,7 +6199,7 @@ void zend_compile_use(zend_ast *ast) /* {{{ */
lookup_name = zend_string_tolower(new_name);
}
- if (type == T_CLASS && zend_is_reserved_class_name(new_name)) {
+ if (type == ZEND_SYMBOL_CLASS && zend_is_reserved_class_name(new_name)) {
zend_error_noreturn(E_COMPILE_ERROR, "Cannot use %s as %s because '%s' "
"is a special class name", ZSTR_VAL(old_name), ZSTR_VAL(new_name), ZSTR_VAL(new_name));
}
@@ -6191,42 +6210,14 @@ void zend_compile_use(zend_ast *ast) /* {{{ */
ZSTR_VAL(ns_name)[ZSTR_LEN(current_ns)] = '\\';
memcpy(ZSTR_VAL(ns_name) + ZSTR_LEN(current_ns) + 1, ZSTR_VAL(lookup_name), ZSTR_LEN(lookup_name));
- if (zend_hash_exists(CG(class_table), ns_name)) {
+ if (zend_have_seen_symbol(ns_name, type)) {
zend_check_already_in_use(type, old_name, new_name, ns_name);
}
zend_string_free(ns_name);
} else {
- switch (type) {
- case T_CLASS:
- {
- zend_class_entry *ce = zend_hash_find_ptr(CG(class_table), lookup_name);
- if (ce && ce->type == ZEND_USER_CLASS
- && ce->info.user.filename == CG(compiled_filename)
- ) {
- zend_check_already_in_use(type, old_name, new_name, lookup_name);
- }
- break;
- }
- case T_FUNCTION:
- {
- zend_function *fn = zend_hash_find_ptr(CG(function_table), lookup_name);
- if (fn && fn->type == ZEND_USER_FUNCTION
- && fn->op_array.filename == CG(compiled_filename)
- ) {
- zend_check_already_in_use(type, old_name, new_name, lookup_name);
- }
- break;
- }
- case T_CONST:
- {
- zend_string *filename = zend_hash_find_ptr(&CG(const_filenames), lookup_name);
- if (filename && filename == CG(compiled_filename)) {
- zend_check_already_in_use(type, old_name, new_name, lookup_name);
- }
- break;
- }
- EMPTY_SWITCH_DEFAULT_CASE()
+ if (zend_have_seen_symbol(lookup_name, type)) {
+ zend_check_already_in_use(type, old_name, new_name, lookup_name);
}
}
@@ -6300,7 +6291,7 @@ void zend_compile_const_decl(zend_ast *ast) /* {{{ */
zend_emit_op(NULL, ZEND_DECLARE_CONST, &name_node, &value_node);
- zend_hash_add_ptr(&CG(const_filenames), name, CG(compiled_filename));
+ zend_register_seen_symbol(name, ZEND_SYMBOL_CONST);
}
}
/* }}}*/
diff --git a/Zend/zend_compile.h b/Zend/zend_compile.h
index 506b414583..a736ecc668 100644
--- a/Zend/zend_compile.h
+++ b/Zend/zend_compile.h
@@ -121,6 +121,8 @@ typedef struct _zend_file_context {
HashTable *imports;
HashTable *imports_function;
HashTable *imports_const;
+
+ HashTable seen_symbols;
} zend_file_context;
typedef union _zend_parser_stack_elem {
@@ -964,6 +966,11 @@ static zend_always_inline int zend_check_arg_send_type(const zend_function *zf,
#define ZEND_ARRAY_NOT_PACKED (1<<1)
#define ZEND_ARRAY_SIZE_SHIFT 2
+/* For "use" AST nodes and the seen symbol table */
+#define ZEND_SYMBOL_CLASS (1<<0)
+#define ZEND_SYMBOL_FUNCTION (1<<1)
+#define ZEND_SYMBOL_CONST (1<<2)
+
/* Pseudo-opcodes that are used only temporarily during compilation */
#define ZEND_GOTO 253
#define ZEND_BRK 254
diff --git a/Zend/zend_globals.h b/Zend/zend_globals.h
index fbbf503c41..7662e263b1 100644
--- a/Zend/zend_globals.h
+++ b/Zend/zend_globals.h
@@ -105,8 +105,6 @@ struct _zend_compiler_globals {
uint32_t compiler_options; /* set of ZEND_COMPILE_* constants */
- HashTable const_filenames;
-
zend_oparray_context context;
zend_file_context file_context;
diff --git a/Zend/zend_language_parser.y b/Zend/zend_language_parser.y
index 957d657909..1880a5fc64 100644
--- a/Zend/zend_language_parser.y
+++ b/Zend/zend_language_parser.y
@@ -327,14 +327,14 @@ top_statement:
{ $$ = zend_ast_create(ZEND_AST_NAMESPACE, NULL, $4); }
| T_USE mixed_group_use_declaration ';' { $$ = $2; }
| T_USE use_type group_use_declaration ';' { $$ = $3; $$->attr = $2; }
- | T_USE use_declarations ';' { $$ = $2; $$->attr = T_CLASS; }
+ | T_USE use_declarations ';' { $$ = $2; $$->attr = ZEND_SYMBOL_CLASS; }
| T_USE use_type use_declarations ';' { $$ = $3; $$->attr = $2; }
| T_CONST const_list ';' { $$ = $2; }
;
use_type:
- T_FUNCTION { $$ = T_FUNCTION; }
- | T_CONST { $$ = T_CONST; }
+ T_FUNCTION { $$ = ZEND_SYMBOL_FUNCTION; }
+ | T_CONST { $$ = ZEND_SYMBOL_CONST; }
;
group_use_declaration:
@@ -373,7 +373,7 @@ use_declarations:
;
inline_use_declaration:
- unprefixed_use_declaration { $$ = $1; $$->attr = T_CLASS; }
+ unprefixed_use_declaration { $$ = $1; $$->attr = ZEND_SYMBOL_CLASS; }
| use_type unprefixed_use_declaration { $$ = $2; $$->attr = $1; }
;