Skip to content

Commit f2e363a

Browse files
GirgiasGirgias
committed
ext/pdo: Convert def_stmt_ctor_args field from zval to Hashtable pointer
1 parent 479edcc commit f2e363a

9 files changed

+298
-33
lines changed

‎ext/pdo/pdo_dbh.c

Lines changed: 26 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -434,11 +434,9 @@ PHP_METHOD(PDO, __construct)
434434
}
435435
/* }}} */
436436

437-
static zval *pdo_stmt_instantiate(pdo_dbh_t *dbh, zval *object, zend_class_entry *dbstmt_ce, zval *ctor_args) /* {{{ */
437+
static zval *pdo_stmt_instantiate(pdo_dbh_t *dbh, zval *object, zend_class_entry *dbstmt_ce, const HashTable *ctor_args) /* {{{ */
438438
{
439-
if (!Z_ISUNDEF_P(ctor_args)) {
440-
/* This implies an error within PDO if this does not hold */
441-
ZEND_ASSERT(Z_TYPE_P(ctor_args) == IS_ARRAY);
439+
if (ctor_args) {
442440
if (!dbstmt_ce->constructor) {
443441
zend_throw_error(NULL, "User-supplied statement does not accept constructor arguments");
444442
return NULL;
@@ -475,8 +473,9 @@ PHP_METHOD(PDO, prepare)
475473
{
476474
pdo_stmt_t *stmt;
477475
zend_string *statement;
478-
zval *options = NULL, *value, *item, ctor_args;
476+
zval *options = NULL, *value, *item;
479477
zend_class_entry *dbstmt_ce, *pce;
478+
/* const */ HashTable *ctor_args = NULL;
480479
pdo_dbh_object_t *dbh_obj = Z_PDO_OBJECT_P(ZEND_THIS);
481480
pdo_dbh_t *dbh = dbh_obj->inner;
482481

@@ -525,16 +524,14 @@ PHP_METHOD(PDO, prepare)
525524
zend_zval_value_name(value));
526525
RETURN_THROWS();
527526
}
528-
ZVAL_COPY_VALUE(&ctor_args, item);
529-
} else {
530-
ZVAL_UNDEF(&ctor_args);
527+
ctor_args = Z_ARRVAL_P(item);
531528
}
532529
} else {
533530
dbstmt_ce = dbh->def_stmt_ce;
534-
ZVAL_COPY_VALUE(&ctor_args, &dbh->def_stmt_ctor_args);
531+
ctor_args = dbh->def_stmt_ctor_args;
535532
}
536533

537-
if (!pdo_stmt_instantiate(dbh, return_value, dbstmt_ce, &ctor_args)) {
534+
if (!pdo_stmt_instantiate(dbh, return_value, dbstmt_ce, ctor_args)) {
538535
RETURN_THROWS();
539536
}
540537
stmt = Z_PDO_STMT_P(return_value);
@@ -549,11 +546,7 @@ PHP_METHOD(PDO, prepare)
549546
ZVAL_UNDEF(&stmt->lazy_object_ref);
550547

551548
if (dbh->methods->preparer(dbh, statement, stmt, options)) {
552-
if (Z_TYPE(ctor_args) == IS_ARRAY) {
553-
pdo_stmt_construct(stmt, return_value, dbstmt_ce, Z_ARRVAL(ctor_args));
554-
} else {
555-
pdo_stmt_construct(stmt, return_value, dbstmt_ce, /* ctor_args */ NULL);
556-
}
549+
pdo_stmt_construct(stmt, return_value, dbstmt_ce, ctor_args);
557550
return;
558551
}
559552

@@ -817,17 +810,19 @@ static bool pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /
817810
return false;
818811
}
819812
dbh->def_stmt_ce = pce;
820-
if (!Z_ISUNDEF(dbh->def_stmt_ctor_args)) {
821-
zval_ptr_dtor(&dbh->def_stmt_ctor_args);
822-
ZVAL_UNDEF(&dbh->def_stmt_ctor_args);
813+
if (dbh->def_stmt_ctor_args) {
814+
zend_array_release(dbh->def_stmt_ctor_args);
815+
dbh->def_stmt_ctor_args = NULL;
823816
}
824817
if ((item = zend_hash_index_find(Z_ARRVAL_P(value), 1)) != NULL) {
825818
if (Z_TYPE_P(item) != IS_ARRAY) {
826819
zend_type_error("PDO::ATTR_STATEMENT_CLASS constructor_args must be of type ?array, %s given",
827820
zend_zval_value_name(value));
828821
return false;
829822
}
830-
ZVAL_COPY(&dbh->def_stmt_ctor_args, item);
823+
dbh->def_stmt_ctor_args = Z_ARRVAL_P(item);
824+
/* Increase refcount */
825+
GC_TRY_ADDREF(dbh->def_stmt_ctor_args);
831826
}
832827
return true;
833828
}
@@ -906,9 +901,10 @@ PHP_METHOD(PDO, getAttribute)
906901
case PDO_ATTR_STATEMENT_CLASS:
907902
array_init(return_value);
908903
add_next_index_str(return_value, zend_string_copy(dbh->def_stmt_ce->name));
909-
if (!Z_ISUNDEF(dbh->def_stmt_ctor_args)) {
910-
Z_TRY_ADDREF(dbh->def_stmt_ctor_args);
911-
add_next_index_zval(return_value, &dbh->def_stmt_ctor_args);
904+
if (dbh->def_stmt_ctor_args) {
905+
/* Increment refcount of constructor arguments */
906+
GC_TRY_ADDREF(dbh->def_stmt_ctor_args);
907+
add_next_index_array(return_value, dbh->def_stmt_ctor_args);
912908
}
913909
return;
914910
case PDO_ATTR_DEFAULT_FETCH_MODE:
@@ -1093,7 +1089,7 @@ PHP_METHOD(PDO, query)
10931089

10941090
PDO_DBH_CLEAR_ERR();
10951091

1096-
if (!pdo_stmt_instantiate(dbh, return_value, dbh->def_stmt_ce, &dbh->def_stmt_ctor_args)) {
1092+
if (!pdo_stmt_instantiate(dbh, return_value, dbh->def_stmt_ce, dbh->def_stmt_ctor_args)) {
10971093
RETURN_THROWS();
10981094
}
10991095
stmt = Z_PDO_STMT_P(return_value);
@@ -1122,11 +1118,7 @@ PHP_METHOD(PDO, query)
11221118
stmt->executed = 1;
11231119
}
11241120
if (ret) {
1125-
if (Z_TYPE(dbh->def_stmt_ctor_args) == IS_ARRAY) {
1126-
pdo_stmt_construct(stmt, return_value, dbh->def_stmt_ce, Z_ARRVAL(dbh->def_stmt_ctor_args));
1127-
} else {
1128-
pdo_stmt_construct(stmt, return_value, dbh->def_stmt_ce, /* ctor_args */ NULL);
1129-
}
1121+
pdo_stmt_construct(stmt, return_value, dbh->def_stmt_ce, dbh->def_stmt_ctor_args);
11301122
return;
11311123
}
11321124
}
@@ -1320,7 +1312,9 @@ static HashTable *dbh_get_gc(zend_object *object, zval **gc_data, int *gc_count)
13201312
{
13211313
pdo_dbh_t *dbh = php_pdo_dbh_fetch_inner(object);
13221314
zend_get_gc_buffer *gc_buffer = zend_get_gc_buffer_create();
1323-
zend_get_gc_buffer_add_zval(gc_buffer, &dbh->def_stmt_ctor_args);
1315+
if (dbh->def_stmt_ctor_args) {
1316+
zend_get_gc_buffer_add_ht(gc_buffer, dbh->def_stmt_ctor_args);
1317+
}
13241318
if (dbh->methods && dbh->methods->get_gc) {
13251319
dbh->methods->get_gc(dbh, gc_buffer);
13261320
}
@@ -1382,8 +1376,8 @@ static void dbh_free(pdo_dbh_t *dbh, bool free_persistent)
13821376
pefree((char *)dbh->persistent_id, dbh->is_persistent);
13831377
}
13841378

1385-
if (!Z_ISUNDEF(dbh->def_stmt_ctor_args)) {
1386-
zval_ptr_dtor(&dbh->def_stmt_ctor_args);
1379+
if (dbh->def_stmt_ctor_args) {
1380+
GC_DTOR_NOGC(dbh->def_stmt_ctor_args);
13871381
}
13881382

13891383
for (i = 0; i < PDO_DBH_DRIVER_METHOD_KIND__MAX; i++) {
@@ -1427,6 +1421,7 @@ zend_object *pdo_dbh_new(zend_class_entry *ce)
14271421
rebuild_object_properties(&dbh->std);
14281422
dbh->inner = ecalloc(1, sizeof(pdo_dbh_t));
14291423
dbh->inner->def_stmt_ce = pdo_dbstmt_ce;
1424+
dbh->inner->def_stmt_ctor_args = NULL;
14301425

14311426
return &dbh->std;
14321427
}

‎ext/pdo/php_pdo_driver.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,7 @@ struct _pdo_dbh_t {
487487

488488
zend_class_entry *def_stmt_ce;
489489

490-
zval def_stmt_ctor_args;
490+
HashTable *def_stmt_ctor_args;
491491

492492
/* when calling PDO::query(), we need to keep the error
493493
* context from the statement around until we next clear it.

‎ext/pdo/tests/pdo_ATTR_STATEMENT_CLASS_ctor_arg_gc.phpt

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
--TEST--
2+
PDO: Set PDOStatement class with ctor_args that are freed with GC intervention
3+
--EXTENSIONS--
4+
pdo
5+
--SKIPIF--
6+
<?php
7+
$dir = getenv('REDIR_TEST_DIR');
8+
if (false == $dir) die('skip no driver');
9+
require_once $dir . 'pdo_test.inc';
10+
PDOTest::skip();
11+
?>
12+
--FILE--
13+
<?php
14+
if (getenv('REDIR_TEST_DIR') === false) putenv('REDIR_TEST_DIR='.__DIR__ . '/../../pdo/tests/');
15+
require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
16+
17+
class Foo extends PDOStatement {
18+
private function __construct($v) {
19+
var_dump($v);
20+
}
21+
}
22+
23+
class Bar extends PDO {
24+
public $statementClass = 'Foo';
25+
function __construct($dsn, $username, $password, $driver_options = []) {
26+
$driver_options[PDO::ATTR_ERRMODE] = PDO::ERRMODE_EXCEPTION;
27+
parent::__construct($dsn, $username, $password, $driver_options);
28+
29+
$this->setAttribute(PDO::ATTR_STATEMENT_CLASS, [$this->statementClass, [$this]]);
30+
}
31+
}
32+
33+
$db = PDOTest::factory(Bar::class);
34+
35+
$dummy_query = get_dummy_sql_request();
36+
37+
$stmt = $db->query($dummy_query);
38+
var_dump($stmt instanceof Foo);
39+
var_dump($stmt->queryString === $dummy_query);
40+
41+
?>
42+
--EXPECT--
43+
object(Bar)#1 (1) {
44+
["statementClass"]=>
45+
string(3) "Foo"
46+
}
47+
bool(true)
48+
bool(true)

‎ext/pdo/tests/pdo_ATTR_STATEMENT_CLASS_ctor_arg_no-gc.phpt

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
--TEST--
2+
PDO: Set PDOStatement class with ctor_args that are freed without GC intervention
3+
--EXTENSIONS--
4+
pdo
5+
--SKIPIF--
6+
<?php
7+
$dir = getenv('REDIR_TEST_DIR');
8+
if (false == $dir) die('skip no driver');
9+
require_once $dir . 'pdo_test.inc';
10+
PDOTest::skip();
11+
?>
12+
--FILE--
13+
<?php
14+
if (getenv('REDIR_TEST_DIR') === false) putenv('REDIR_TEST_DIR='.__DIR__ . '/../../pdo/tests/');
15+
require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
16+
17+
class Foo extends PDOStatement {
18+
private function __construct($v) {
19+
var_dump($v);
20+
}
21+
}
22+
23+
$db = PDOTest::factory();
24+
25+
$db->setAttribute(PDO::ATTR_STATEMENT_CLASS, array('Foo', ['param1']));
26+
27+
$dummy_query = get_dummy_sql_request();
28+
29+
$stmt = $db->query($dummy_query);
30+
var_dump($stmt instanceof Foo);
31+
var_dump($stmt->queryString === $dummy_query);
32+
33+
?>
34+
--EXPECT--
35+
string(6) "param1"
36+
bool(true)
37+
bool(true)

‎ext/pdo/tests/pdo_ATTR_STATEMENT_CLASS_ctor_arg_no-gc_var_modified.phpt

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
--TEST--
2+
PDO: Set PDOStatement class with ctor_args that are freed without GC intervention as a variable that is modified
3+
--EXTENSIONS--
4+
pdo
5+
--SKIPIF--
6+
<?php
7+
$dir = getenv('REDIR_TEST_DIR');
8+
if (false == $dir) die('skip no driver');
9+
require_once $dir . 'pdo_test.inc';
10+
PDOTest::skip();
11+
?>
12+
--FILE--
13+
<?php
14+
if (getenv('REDIR_TEST_DIR') === false) putenv('REDIR_TEST_DIR='.__DIR__ . '/../../pdo/tests/');
15+
require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
16+
17+
class Foo extends PDOStatement {
18+
private function __construct($v) {
19+
var_dump($v);
20+
}
21+
}
22+
23+
$db = PDOTest::factory();
24+
25+
$a = ['Foo', ['param1']];
26+
$db->setAttribute(PDO::ATTR_STATEMENT_CLASS, $a);
27+
$a[0] = 'Bar';
28+
29+
$dummy_query = get_dummy_sql_request();
30+
31+
$stmt = $db->query($dummy_query);
32+
var_dump($stmt instanceof Foo);
33+
var_dump($stmt->queryString === $dummy_query);
34+
35+
?>
36+
--EXPECT--
37+
string(6) "param1"
38+
bool(true)
39+
bool(true)

‎ext/pdo/tests/pdo_ATTR_STATEMENT_CLASS_ctor_arg_no-gc_var_unset.phpt

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
--TEST--
2+
PDO: Set PDOStatement class with ctor_args that are freed without GC intervention as a variable that is unset
3+
--EXTENSIONS--
4+
pdo
5+
--SKIPIF--
6+
<?php
7+
$dir = getenv('REDIR_TEST_DIR');
8+
if (false == $dir) die('skip no driver');
9+
require_once $dir . 'pdo_test.inc';
10+
PDOTest::skip();
11+
?>
12+
--FILE--
13+
<?php
14+
if (getenv('REDIR_TEST_DIR') === false) putenv('REDIR_TEST_DIR='.__DIR__ . '/../../pdo/tests/');
15+
require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
16+
17+
class Foo extends PDOStatement {
18+
private function __construct($v) {
19+
var_dump($v);
20+
}
21+
}
22+
23+
$db = PDOTest::factory();
24+
25+
$a = ['Foo', ['param1']];
26+
$db->setAttribute(PDO::ATTR_STATEMENT_CLASS, $a);
27+
unset($a);
28+
29+
$dummy_query = get_dummy_sql_request();
30+
31+
$stmt = $db->query($dummy_query);
32+
var_dump($stmt instanceof Foo);
33+
var_dump($stmt->queryString === $dummy_query);
34+
35+
?>
36+
--EXPECT--
37+
string(6) "param1"
38+
bool(true)
39+
bool(true)

‎ext/pdo/tests/pdo_prepare_ATTR_STATEMENT_CLASS_ctor_arg_gc.phpt

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
--TEST--
2+
PDO: Set PDOStatement class with ctor_args that are freed with GC intervention in PDO::prepare()
3+
--EXTENSIONS--
4+
pdo
5+
--SKIPIF--
6+
<?php
7+
$dir = getenv('REDIR_TEST_DIR');
8+
if (false == $dir) die('skip no driver');
9+
require_once $dir . 'pdo_test.inc';
10+
PDOTest::skip();
11+
?>
12+
--FILE--
13+
<?php
14+
if (getenv('REDIR_TEST_DIR') === false) putenv('REDIR_TEST_DIR='.__DIR__ . '/../../pdo/tests/');
15+
require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
16+
17+
class Foo extends PDOStatement {
18+
private function __construct($v) {
19+
var_dump($v);
20+
}
21+
}
22+
23+
class Bar extends PDO {
24+
public $statementClass = 'Foo';
25+
function __construct($dsn, $username, $password, $driver_options = []) {
26+
$driver_options[PDO::ATTR_ERRMODE] = PDO::ERRMODE_EXCEPTION;
27+
parent::__construct($dsn, $username, $password, $driver_options);
28+
29+
$this->setAttribute(PDO::ATTR_STATEMENT_CLASS, [$this->statementClass, [$this]]);
30+
}
31+
}
32+
33+
$db = PDOTest::factory(Bar::class);
34+
35+
36+
$dummy_query = get_dummy_sql_request();
37+
38+
$stmt = $db->prepare($dummy_query);
39+
var_dump($stmt instanceof Foo);
40+
var_dump($stmt->queryString === $dummy_query);
41+
$r = $stmt->execute();
42+
var_dump($r);
43+
44+
?>
45+
--EXPECT--
46+
object(Bar)#1 (1) {
47+
["statementClass"]=>
48+
string(3) "Foo"
49+
}
50+
bool(true)
51+
bool(true)
52+
bool(true)

0 commit comments

Comments
 (0)