Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[WIP] Disallow non-static calls to the constructor #5405

Closed
wants to merge 1 commit into from
Closed

[WIP] Disallow non-static calls to the constructor #5405

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion Zend/tests/bug31177-2.phpt
View file
Open in desktop
Expand Up @@ -6,10 +6,13 @@ class foo {
function __construct($n=0) {
if($n) throw new Exception("new");
}
function another() {
return self::__construct(1);
}
}
$x = new foo();
try {
$y=$x->__construct(1);
$y=$x->another();
} catch (Exception $e) {
var_dump($x);
}
Expand Down
3 changes: 0 additions & 3 deletions Zend/tests/bug35634.phpt
View file
Open in desktop
Expand Up @@ -14,9 +14,6 @@ if (defined("pass3")) {
class TestClass {
function __construct() {
}
function TestClass() {
$this->__construct();
}
}

} else {
Expand Down
6 changes: 5 additions & 1 deletion Zend/tests/bug60536_001.phpt
View file
Open in desktop
Expand Up @@ -16,9 +16,13 @@ class Z extends Y {
function __construct() {
return ++$this->x;
}

function another() {
self::__construct();
}
}
$a = new Z();
$a->__construct();
$a->another();
echo "DONE";
?>
--EXPECTF--
Expand Down
9 changes: 9 additions & 0 deletions Zend/tests/constructor_instance_001.phpt
View file
Open in desktop
@@ -0,0 +1,9 @@
--TEST--
Not allowed to call the constructor on an object instance
--FILE--
<?php
class C {}
$c = new C();
$c->__construct();
--EXPECTF--
Fatal error: Cannot call the constructor on an object instance %s
15 changes: 15 additions & 0 deletions Zend/tests/constructor_instance_002.phpt
View file
Open in desktop
@@ -0,0 +1,15 @@
--TEST--
Not allowed to call the constructor on an object instance
--FILE--
<?php
class C {
function __construct() {}
}
$c = new C();
$constr = '__construct';
$c->$constr();
--EXPECTF--
Fatal error: Uncaught Error: Cannot call the constructor on an object instance %s
Stack trace:
#0 {main}
thrown %s
16 changes: 10 additions & 6 deletions Zend/zend_compile.c
View file
Open in desktop
Expand Up @@ -4101,6 +4101,12 @@ void zend_compile_call(znode *result, zend_ast *ast, uint32_t type) /* {{{ */
}
/* }}} */

static zend_bool zend_is_constructor(zend_string *name) /* {{{ */
{
return zend_string_equals_literal_ci(name, ZEND_CONSTRUCTOR_FUNC_NAME);
}
/* }}} */

void zend_compile_method_call(znode *result, zend_ast *ast, uint32_t type) /* {{{ */
{
zend_ast *obj_ast = ast->child[0];
Expand Down Expand Up @@ -4130,6 +4136,10 @@ void zend_compile_method_call(znode *result, zend_ast *ast, uint32_t type) /* {{
zend_error_noreturn(E_COMPILE_ERROR, "Method name must be a string");
}

if (zend_is_constructor(Z_STR_P(&method_node.u.constant))) {
zend_error_noreturn(E_COMPILE_ERROR, "Cannot call the constructor on an object instance");
}

opline->op2_type = IS_CONST;
opline->op2.constant = zend_add_func_name_literal(
Z_STR(method_node.u.constant));
Expand All @@ -4155,12 +4165,6 @@ void zend_compile_method_call(znode *result, zend_ast *ast, uint32_t type) /* {{
}
/* }}} */

static zend_bool zend_is_constructor(zend_string *name) /* {{{ */
{
return zend_string_equals_literal_ci(name, ZEND_CONSTRUCTOR_FUNC_NAME);
}
/* }}} */

static zend_function *zend_get_compatible_func_or_null(zend_class_entry *ce, zend_string *lcname) /* {{{ */
{
zend_function *fbc = zend_hash_find_ptr(&ce->function_table, lcname);
Expand Down
7 changes: 7 additions & 0 deletions Zend/zend_vm_def.h
View file
Open in desktop
Expand Up @@ -3470,6 +3470,13 @@ ZEND_VM_HOT_OBJ_HANDLER(112, ZEND_INIT_METHOD_CALL, CONST|TMPVAR|UNUSED|THIS|CV,
}
}

if (UNEXPECTED((fbc->common.fn_flags & ZEND_ACC_CTOR) != 0)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be part of the else above, to reduce cost.

It might make sense to actually move this into get_method, which will prevent acquiring __construct as an ordinary method completely. But maybe that will interfere with parent::__construct?

Right now this is probably missing constructor calls via call_user_func.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason why I left it outside of the else was that even though the CONST case should be caught by the compile time check, I wasn't completely sure if it would cover all the cases. I'll try to check those other cases/options.

zend_throw_error(NULL, "Cannot call the constructor on an object instance");
FREE_OP2();
FREE_OP1();
HANDLE_EXCEPTION();
}

if (OP2_TYPE != IS_CONST) {
FREE_OP2();
}
Expand Down
84 changes: 84 additions & 0 deletions Zend/zend_vm_execute.h
View file
Open in desktop
Expand Up @@ -5678,6 +5678,13 @@ static ZEND_VM_COLD ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_INIT_METHOD_CALL_
}
}

if (UNEXPECTED((fbc->common.fn_flags & ZEND_ACC_CTOR) != 0)) {
zend_throw_error(NULL, "Cannot call the constructor on an object instance");


HANDLE_EXCEPTION();
}

if (IS_CONST != IS_CONST) {

}
Expand Down Expand Up @@ -7853,6 +7860,13 @@ static ZEND_VM_COLD ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_INIT_METHOD_CALL_
}
}

if (UNEXPECTED((fbc->common.fn_flags & ZEND_ACC_CTOR) != 0)) {
zend_throw_error(NULL, "Cannot call the constructor on an object instance");
zval_ptr_dtor_nogc(EX_VAR(opline->op2.var));

HANDLE_EXCEPTION();
}

if ((IS_TMP_VAR|IS_VAR) != IS_CONST) {
zval_ptr_dtor_nogc(EX_VAR(opline->op2.var));
}
Expand Down Expand Up @@ -10109,6 +10123,13 @@ static ZEND_VM_COLD ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_INIT_METHOD_CALL_
}
}

if (UNEXPECTED((fbc->common.fn_flags & ZEND_ACC_CTOR) != 0)) {
zend_throw_error(NULL, "Cannot call the constructor on an object instance");


HANDLE_EXCEPTION();
}

if (IS_CV != IS_CONST) {

}
Expand Down Expand Up @@ -14400,6 +14421,13 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_INIT_METHOD_CALL_SPEC_TMPVAR_C
}
}

if (UNEXPECTED((fbc->common.fn_flags & ZEND_ACC_CTOR) != 0)) {
zend_throw_error(NULL, "Cannot call the constructor on an object instance");

zval_ptr_dtor_nogc(EX_VAR(opline->op1.var));
HANDLE_EXCEPTION();
}

if (IS_CONST != IS_CONST) {

}
Expand Down Expand Up @@ -15781,6 +15809,13 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_INIT_METHOD_CALL_SPEC_TMPVAR_T
}
}

if (UNEXPECTED((fbc->common.fn_flags & ZEND_ACC_CTOR) != 0)) {
zend_throw_error(NULL, "Cannot call the constructor on an object instance");
zval_ptr_dtor_nogc(EX_VAR(opline->op2.var));
zval_ptr_dtor_nogc(EX_VAR(opline->op1.var));
HANDLE_EXCEPTION();
}

if ((IS_TMP_VAR|IS_VAR) != IS_CONST) {
zval_ptr_dtor_nogc(EX_VAR(opline->op2.var));
}
Expand Down Expand Up @@ -17056,6 +17091,13 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_INIT_METHOD_CALL_SPEC_TMPVAR_C
}
}

if (UNEXPECTED((fbc->common.fn_flags & ZEND_ACC_CTOR) != 0)) {
zend_throw_error(NULL, "Cannot call the constructor on an object instance");

zval_ptr_dtor_nogc(EX_VAR(opline->op1.var));
HANDLE_EXCEPTION();
}

if (IS_CV != IS_CONST) {

}
Expand Down Expand Up @@ -30533,6 +30575,13 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_INIT_METHOD_CALL_S
}
}

if (UNEXPECTED((fbc->common.fn_flags & ZEND_ACC_CTOR) != 0)) {
zend_throw_error(NULL, "Cannot call the constructor on an object instance");


HANDLE_EXCEPTION();
}

if (IS_CONST != IS_CONST) {

}
Expand Down Expand Up @@ -32401,6 +32450,13 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_INIT_METHOD_CALL_SPEC_UNUSED_T
}
}

if (UNEXPECTED((fbc->common.fn_flags & ZEND_ACC_CTOR) != 0)) {
zend_throw_error(NULL, "Cannot call the constructor on an object instance");
zval_ptr_dtor_nogc(EX_VAR(opline->op2.var));

HANDLE_EXCEPTION();
}

if ((IS_TMP_VAR|IS_VAR) != IS_CONST) {
zval_ptr_dtor_nogc(EX_VAR(opline->op2.var));
}
Expand Down Expand Up @@ -34795,6 +34851,13 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_INIT_METHOD_CALL_SPEC_UNUSED_C
}
}

if (UNEXPECTED((fbc->common.fn_flags & ZEND_ACC_CTOR) != 0)) {
zend_throw_error(NULL, "Cannot call the constructor on an object instance");


HANDLE_EXCEPTION();
}

if (IS_CV != IS_CONST) {

}
Expand Down Expand Up @@ -39644,6 +39707,13 @@ static ZEND_VM_HOT ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_INIT_METHOD_CALL_S
}
}

if (UNEXPECTED((fbc->common.fn_flags & ZEND_ACC_CTOR) != 0)) {
zend_throw_error(NULL, "Cannot call the constructor on an object instance");


HANDLE_EXCEPTION();
}

if (IS_CONST != IS_CONST) {

}
Expand Down Expand Up @@ -43081,6 +43151,13 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_INIT_METHOD_CALL_SPEC_CV_TMPVA
}
}

if (UNEXPECTED((fbc->common.fn_flags & ZEND_ACC_CTOR) != 0)) {
zend_throw_error(NULL, "Cannot call the constructor on an object instance");
zval_ptr_dtor_nogc(EX_VAR(opline->op2.var));

HANDLE_EXCEPTION();
}

if ((IS_TMP_VAR|IS_VAR) != IS_CONST) {
zval_ptr_dtor_nogc(EX_VAR(opline->op2.var));
}
Expand Down Expand Up @@ -47963,6 +48040,13 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_INIT_METHOD_CALL_SPEC_CV_CV_HA
}
}

if (UNEXPECTED((fbc->common.fn_flags & ZEND_ACC_CTOR) != 0)) {
zend_throw_error(NULL, "Cannot call the constructor on an object instance");


HANDLE_EXCEPTION();
}

if (IS_CV != IS_CONST) {

}
Expand Down
19 changes: 0 additions & 19 deletions ext/dom/tests/DOMComment_construct_basic_001.phpt
View file
Open in desktop

This file was deleted.

16 changes: 0 additions & 16 deletions ext/dom/tests/DOMDocumentFragment_construct_basic_002.phpt
View file
Open in desktop

This file was deleted.

20 changes: 0 additions & 20 deletions ext/dom/tests/bug66502.phpt
View file
Open in desktop

This file was deleted.

15 changes: 0 additions & 15 deletions ext/fileinfo/tests/finfo_open_002.phpt
View file
Open in desktop

This file was deleted.

19 changes: 0 additions & 19 deletions ext/intl/tests/bug62081.phpt
View file
Open in desktop

This file was deleted.

2 changes: 0 additions & 2 deletions ext/phar/tests/phar_oo_001.phpt
View file
Open in desktop
Expand Up @@ -34,7 +34,6 @@ catch (LogicException $e)
}
try {
$phar = new Phar('test.phar');
$phar->__construct('oops');
} catch (LogicException $e)
{
var_dump($e->getMessage());
Expand All @@ -50,4 +49,3 @@ __halt_compiler();
string(5) "1.0.0"
int(5)
string(50) "Cannot call method on an uninitialized Phar object"
string(29) "Cannot call constructor twice"