Segmentation fault - possibly another GC issue? about v8js HOT 7 CLOSED

@stesie got it! I think...

// v8js_v8object_class.cc
v8js_v8object_handlers.cast_object = NULL

I looked at the PHP code for zend_object_is_true and it has this:

ref: https://github.com/php/php-src/blob/master/Zend/zend_operators.c#L2653

	if (zobj->handlers->cast_object(zobj, &tmp, _IS_BOOL) == SUCCESS) {

but V8Js sets this handler to NULL.

I tried implementing a basic handler (that returns SUCCESS) and it worked without segfault, exiting normally. It also worked if i just commented out the explicit assignment of NULL to that handler - so perhaps there's a sensible default of sorts.

Is it just a case of implementing or removing this cast_object? Looking at PHP7.4 (the previous version we were using), the zend_object_is_true function started with:

if (Z_OBJ_HT_P(op)->cast_object) {

ref: https://github.com/php/php-src/blob/PHP-7.4.3/Zend/zend_operators.c#L2599

so i guess it had some protection for when this handler was not set.

anyway, a bit more digging, turns out making this a required handler was intentional:

php/php-src@8fd7f02

from v8js.

The bit in the above example that appears to consistently break things is the assignment ($foo = $result->main)

This gives me my line number:

if ((print("ln:" . __LINE__ . "\n")) &&  ($foo = $result->main) && is_object($foo) && ($foo instanceof V8Function)) {

Whereas this does not:

if (($foo = $result->main) && (print("ln:" . __LINE__ . "\n")) && is_object($foo) && ($foo instanceof V8Function)) {

Also - for the purpose of checking whether my anonymous class was at fault somehow, I tried removing it (along with the V8Js::FLAG_FORCE_ARRAY flag):

<?php

$v = new \V8Js();
$r = $v->executeString('
	a = {
		main: function() {
			print("foo\n");
		}
	};
', null, V8Js::FLAG_PROPAGATE_PHP_EXCEPTIONS);

print_r(dothings($r));
echo 'all done!' . PHP_EOL;

function dothings($result) {
	if (($foo = $result->main) && (print("ln:" . __LINE__ . "\n")) && is_object($foo) && ($foo instanceof V8Function)) { // error
		return $result->main();
	}
}

This gave me the same outcome, including the same print outputs mentioned above - i.e. the issue is still due to the assignment.

@stesie Does this reveal enough clues to give any pointers as to where I could start debugging the C++ code?

from v8js.

Yes, definitely has something with de assignment that is treated differently when inside if statement.
Inside the __get method, if you wrap the return on a closure, then everything works fine.
But!!! Just if I check with isset()!

    public function __get($item) {
	//return $this->result[$item]        ? (fn(...$args) => $this->result[$item](...$args)) : null; // error
	return isset($this->result[$item]) ? (fn(...$args) => $this->result[$item](...$args)) : null; // no error
    }

It's weird.

from v8js.

And in fact, this code can reproduce the same behavior:

<?php

$v = new \V8Js();
$r = $v->executeString('
    a = {
	main: function() {
	    print("foo\n");
	}
    };
', null, V8Js::FLAG_FORCE_ARRAY | V8Js::FLAG_PROPAGATE_PHP_EXCEPTIONS);

if (is_object($r['main']) && ($r['main'] instanceof V8Function)) { // no error
//if (($foo = $r['main']) && print("0") && is_object($foo) && print("1") && ($foo instanceof V8Function) && print("2")) { // error
    print_r($r['main']());
}
echo 'all done!' . PHP_EOL;

from v8js.

Thanks for replicating, what specific version of PHP / V8 you on? Would be good if we can at least somewhat rule a couple of things out.

My C++ debugging has gotten a little rusty, but I’m either suspecting something GC or refcount related or something about changes around accessing non-existent properties in PHP8 (though I would have imagined the __get would take care of that)

from v8js.

perhaps some extra clues. Tried the script with GC disabled to rule that out. I don't know too much about the PHP internals but my guess is there's something an object has to do to report back when an 'empty' check is done on it. I suppose like the __isset() magic method or the valid() iterator method (albeit low-level). The GDB backtrace seems to agree, if i'm reading it right:

#0  0x0000000000000000 in ?? ()
#1  0x00005633e2196231 in zend_object_is_true ()

// this is ok
isset($result->main);

// this will segfault
!empty($result->main);

// I guess this probably uses the same `empty` logic under the hood, hence why it also crashes:
if ($foo = $result->main) {}

So i guess now all i need is a steer towards where in the V8Js code I might need to look?

from v8js.

I've teste your solution here and confirm that works.
Makes a lot of sense. Tested with this code (I have removed FLAG_FORCE_ARRAY here, same results):

var_dump(empty($foo = $r->main));

And got exactly same behavior with the if statement. But now with your patch, everything works!
Tested with v8 10.2.154.8 and 3 php versions:
PHP 8.0.27 (cli) (built: Jan 13 2023 10:38:04) ( NTS )
PHP 8.1.14 (cli) (built: Jan 13 2023 10:38:02) (NTS)
PHP 8.2.1 (cli) (built: Jan 13 2023 10:38:46) (NTS)

Thanks @redbullmarky !!

from v8js.