Named arguments in CTE functions cause a segfault

[mvorisek]

Description

The following code:

<?php

$h0 = hash("murmur3a", "Two hashes meet in a bar.", options: ["seed" => 2345]);

and hash function marked as CTE (like in https://github.com/php/php-src/blob/f16c3e1265/ext/hash/hash.stub.php#L12)

Resulted in this output:

========DIFF========
     95855f9be0db784a5c37e878c4a4dcee
002+ Segmentation fault (core dumped)
003+ 
004+ Termsig=11
002- 95855f9be0db784a5c37e878c4a4dcee
003- f64c9eb40287fa686575163893e283b2
004- f64c9eb40287fa686575163893e283b2
005- 7f7ec59b
006- 7f7ec59b
========DONE========
FAIL Hash: MurmurHash3 seed [ext/hash/tests/murmurhash3_seed.phpt] 

But I expected this output instead:

discovered in #10771, based on #10771 (comment) hash will be not supported as CTE, but I belive the segfault should be investigated, as it can be used from an extension on compile time for example

PHP Version

master

Operating System

any

[nielsdos]

I can confirm that this crashes. In fact, any compile-time-evaluated function where the programmer uses named arguments will crash because EX(call) is NULL.

For example, even something like:

<?php
echo strstr(needle: 'a', haystack: 'aaa');

will crash after marking that function as CTE (like you did in your recent PR).

And it is even triggerable with functions which are CTE right now already:

<?php
echo count(value: []);

This definitely needs fixing, I'll do that tonight.

[nielsdos]

I edited the title so the root cause is in the title. Others can then also more easily find these issues.
Old title was: "hash as CTE leads to segfault"

[nielsdos]

Interesting bug. I can't always reproduce it with the simple reproducer I gave.
It's actually not during the constant evaluation that it crashes, it's during script execution but only if the constant evaluation code ran.

Furthermore, I found 2 other weird behaviours.

If you use ASAN, you'll find that the following works correctly without opcache:

<?php
print_r(array_keys(array: [1], strict: true, filter_value: 1));

but if you enable opcache: it crashes with a segfault on an invalid pointer. I added a printf to show what types of zvals get destroyed and I see that the crash happens when a zval of type "32" gets destroyed. This is during execution and is not a valid type. It tries to destroy a reference which of course fails and crashes... It appears this is the argument sent to print_r which gets destroyed due to an exception. And the argument probably happens because the zval is invalid... It this destruction were not to happen we get the EX(call) failure from earlier again. Probably also caused by the invalid zval in the first place.

Also worth noting, if I first assign the return value of array_keys to a CV then the crashes do not happen.

If I change the print_r in the above code fragment to var_dump, I get a different error instead (but no ASAN failure):

PHP Fatal error:  Uncaught ArgumentCountError: var_dump() does not accept unknown named parameters in /home/niels/php-src/strlen.php:2
Stack trace:
#0 /home/niels/php-src/strlen.php(2): var_dump(Array, strict: true, filter_value: 1)
#1 {main}
  thrown in /home/niels/php-src/strlen.php on line 2

I'm out of time for today so I'll have to resume some other time.

[nielsdos]

It's an optimizer bug in remove_call in sccp. Looking at the SSA it removes too few SEND_VAL oplines. That's because num_args equals 1 in the reproducer I gave in my previous post. If I don't use named arguments then the num_args variable equals 3 as I (and the optimizer) had expected. So perhaps the num_args variable just simply doesn't count named arguments, which could very well be intended (I don't know yet).
A simple fix I just tried is to change the loop condition to check the opcode nr instead.
I'll try to figure out now if the num_args behaviour is expected...

EDIT: according to https://wiki.php.net/rfc/named_params this is expected. So I'll proceed with my opcode nr fix, unless I find something smart

[nielsdos]

So I fixed the crash and random fatal error bug locally, but I found another bug in the process.
It appears the named arguments are actually ignored for the constant evaluation call. This causes the hash to differ between regular execution vs CTE. I'll check that one as well...

[nielsdos]

Adding support for named parameters in CTE is not trivial, requires tens of lines of code extra instead of the few lines I had to change for the crash fix. So therefore I think that would be master-only.
For now, I'll fix the crash (still useful for variadic functions), and disable the CTE for named parameters on stable branches as it doesn't work properly (results are wrong or it crashes). Note however that named parameters in the function declaration order will still work because internally it'll use the regular argument passing mechanism it seems.