ext/pdo: Convert FETCH_INTO zval to a zend_object pointer

[Girgias]

No description provided.

[nielsdos]

Why try?

[Girgias]

I don't know why I did this, I think I didn't want to get bit by SHM stuff, forgetting that objects can't live in it. (although can enums live in SHM?)

[nielsdos]

Objects indeed can't live inside shm, and so enums can't either. If you use a default value with an enum you're actually storing an IS_CONSTANT_AST and deferring the evaluation into an object to the runtime.

[nielsdos]

A bitwise AND check seems wrong, PDO_FETCH_INTO is part of an enum with sequential integer indices so that means that there will be fetch mode with overlapping bits.
That said, I also think there's a pre-existing bug here: because into is part of a union, we should indeed only add the object/zval to the gc buffer if it is in this mode...

[Girgias]

Because of how the current struct is organized, this "into" zval also handles the class fetch ctor arguments.

Which means that I do have a bug indeed, thanks for making me realize this, I guess time to write even more tests to handle these cases.

[nielsdos]

Right, but the zval also overlap with an int, which is the bigger problem.

[nielsdos]

Using a bitwise AND here seems wrong, as they're not bitmasks, so you're going to hit the condition for more than just PDO_FETCH_INTO or PDO_FETCH_CLASS.
See

php-src/ext/pdo/php_pdo_driver.h

Lines 66 to 81 in 8ed0d7f

	enum pdo_fetch_type {
	PDO_FETCH_USE_DEFAULT,
	PDO_FETCH_LAZY,
	PDO_FETCH_ASSOC,
	PDO_FETCH_NUM,
	PDO_FETCH_BOTH,
	PDO_FETCH_OBJ,
	PDO_FETCH_BOUND, /* return true/false only; rely on bound columns */
	PDO_FETCH_COLUMN, /* fetch a numbered column only */
	PDO_FETCH_CLASS, /* create an instance of named class, call ctor and set properties */
	PDO_FETCH_INTO, /* fetch row into an existing object */
	PDO_FETCH_FUNC, /* fetch into function and return its result */
	PDO_FETCH_NAMED, /* like PDO_FETCH_ASSOC, but can handle duplicate names */
	PDO_FETCH_KEY_PAIR, /* fetch into an array where the 1st column is a key and all subsequent columns are values */
	PDO_FETCH__MAX /* must be last */
	};

[Girgias]

The problem is that the default_fetch_type is a union of the fetch mode, with the fetch flags. I could bitwise not it with PDO_FETCH_FLAGS to separate out the flags from the mode as a bunch of other places do it.

[nielsdos]

Okay I remember now, but it's still wrong: you should AND with ~PDO_FETCH_FLAGS otherwise the following can happen:

Known: PDO_FETCH_CLASS equals 8 (0b1000), PDO_FETCH_INTO equals 9 (0b1001).
Problem example: let's say default_fetch_type is PDO_FETCH_INTO, and we look at the condition (stmt->default_fetch_type & PDO_FETCH_CLASS) == PDO_FETCH_CLASS. This condition expands to: (0b1001 & 0b1000) == 0b1000. We know that 0b1001 & 0b1000 is 0b1000, so it will be equal to PDO_FETCH_CLASS even though that's not what we want. In this particular case you don't run into it because of the ordering of the branches but that's extremely fragile and I'm sure you can come up with another combination of modes to trigger this kind of behaviour.

[nielsdos]

Looks right now, let's wait for CI though.