Segmentation fault - possibly another GC issue?

[redbullmarky]

Hey!
I think we've had something very similar, but this code will trigger a segfault - Ubuntu 22 & their PHP8.1, plus V8 10.9 against the latest V8Js version. To summarise, it creates an object with a function in it, wraps it inside of an anonymous class so that it can be treated more like a regular object, then passes it to a function that will validate what it needs and execute the 'main' function that JS returned.

Included near the bottom are two lines - one commented out - one that works and one that doesn't:

<?php

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

$o = new class($r, $v) {
	private $result;
	private $v8js;

	public function __construct($result, $v8js) {
		$this->result = $result;
		$this->v8js = $v8js;
	}

	public function __call($method, $args) {
		return $this->result[$method](...$args);
	}

	public function __get($item) {
		return $this->result[$item] ?? null;
	}
};

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

function dothings($result) {
	// if (is_object($result->main) && ($result->main instanceof V8Function)) { // no error
	if (($foo = $result->main) && is_object($foo) && ($foo instanceof V8Function)) { // error
		return $result->main();
	}
}

The problem exists in the dothings function - I've included two lines, one that works and one that doesn't. My suspicion is that (with the one that breaks) that $foo is being discarded after that line (before the return) but also discarding the underlaying V8Function (if you change the return line to return $foo(); it also segfaults, perhaps because $result is then discarded because of not having any usage?

A few pointers:

1. The code above is a rough replication :-p
2. We use FLAG_FORCE_ARRAY for legacy reasons (plenty of other use cases), but for a few cases we somewhat loosely 'emulate' using an anonymous class whilst otherwise keeping everything compatible.
3. The passing in of $v8js argument to the anonymous class was for something related, to make sure we were keeping a reference to the main V8Js instance alive. Added it to my replication as it meant the outcome of the code was identical to that of the main system.

Any help would be much appreciated!

[redbullmarky]

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?

[chrisbckr]

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.

[chrisbckr]

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;

[redbullmarky]

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)

[redbullmarky]

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?

[redbullmarky]

@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

[chrisbckr]

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 !!