segfault in ReflectionNamedType::getName()

[ju1ius]

Description

Hi,

Given an internal function named Acme\\example with the following return type (a resource):

zend_type return_type = {
  .ptr = NULL,
  .type_mask = MAY_BE_RESOURCE,
};

The following code:

<?php
$fn = new ReflectionFunction('Acme\\example');
$ty = $fn->getReturnType()?->getName();
var_dump($ty);

Resulted in this output:

Segmentation fault (core dumped).

GDB core dump:

Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x000055d3178b7c25 in zim_ReflectionNamedType_getName (execute_data=0x7f23f7c1a4c0, return_value=0x7f23f7c1a2d0)
    at /php-src/ext/reflection/php_reflection.c:3065

3065			RETURN_STR(zend_type_to_string_without_null(param->type));
(gdb) bt
#0  0x000055d3178b7c25 in zim_ReflectionNamedType_getName (execute_data=0x7f23f7c1a4c0, return_value=0x7f23f7c1a2d0)
    at /php-src/ext/reflection/php_reflection.c:3065
#1  0x000055d317b6166a in ZEND_DO_FCALL_SPEC_RETVAL_USED_HANDLER ()
    at /php-src/Zend/zend_vm_execute.h:1976
#2  0x000055d317be151c in execute_ex (ex=0x7f23f7c18020)
    at /php-src/Zend/zend_vm_execute.h:57246
#3  0x000055d317be5d57 in zend_execute (op_array=0x7f23f7c8c000, return_value=0x0)
    at /php-src/Zend/zend_vm_execute.h:61598
#4  0x000055d317b1ad40 in zend_execute_scripts (type=8, retval=0x0, file_count=3)
    at /php-src/Zend/zend.c:1881
#5  0x000055d317a562aa in php_execute_script (primary_file=0x7ffdf97060f0)
    at /php-src/main/main.c:2501
#6  0x000055d317ca8772 in do_cli (argc=6, argv=0x55d31a302b80)
    at /php-src/sapi/cli/php_cli.c:966
#7  0x000055d317ca953f in main (argc=6, argv=0x55d31a302b80)
    at /php-src/sapi/cli/php_cli.c:1340

But I expected this output instead:

string(8) "resource"

There are two problems here.

The root is that zend_type_to_string does not handle the MAY_BE_RESOURCE flag, thus returning a NULL pointer in this case.

Then, looking at the core dump, we can see that zim_ReflectionNamedType_getName does not handle the NULL pointer case, hence the segfault.

A possible fix would be to make zend_type_to_string return "resource" when the type is a resource. Eventhough this type is not nameable in userland PHP, it is a real type in the engine, and this information can be valuable to expose to the reflection API. This would be a breaking change though (i.e. a codegen tool using reflection type info, would now generate an invalid type).

Or maybe a ReflectionType::isResource() method ?

Or just fix the null pointer dereference and call it a day...

WDYT?

PHP Version

PHP 8.3-dev

Operating System

irrelevant

[nikic]

We should detect attempts to use the resource type when reading in function entries and throw an error in that case. The type is explicitly not supported, and extensions should not be able to work around that.

[ju1ius]

The type is explicitly not supported

Out of curiosity, could you point me to where it is explicitly stated that this is not supported in zend_type / internal arginfos?

[iluuu1994]

@ju1ius resource is not supported in userland. Nowadays the types supported in core/userland should mostly be congruent.

[iluuu1994]

diff --git a/build/gen_stub.php b/build/gen_stub.php
index 5220d1de1a..ae11d4c162 100755
--- a/build/gen_stub.php
+++ b/build/gen_stub.php
@@ -567,6 +567,10 @@ public static function fromNode(Node $node): Type {
             );
         }
 
+        if ($node instanceof Name\FullyQualified && $node->toLowerString() === 'resource') {
+            throw new Exception("resource is not a supported builtin type");
+        }
+
         if ($node instanceof Node\Identifier && $node->toLowerString() === "iterable") {
             return new Type(
                 [

Something like this might suffice.

[ju1ius]

@ju1ius resource is not supported in userland.

I've been following PHP for 20+ years now, so I'm well aware. 😉

My question was about internal functions.
I've not been able to find anything explicitely stating that MAY_BE_RESOURCE is not allowed as a zend_type in zend_internal_arginfos, and using the type works everywhere except reflection.
Since the resource type is explicitely mentioned in the docs as one of the fundamental atoms of PHP's type system, albeit not nameable in userland, it is not too far fetch to interpret this as: «the resource type is only available to internal functions».
So I'm genuinely curious about where to find these kind of informations.

Something like this might suffice.

I'm afraid not. gen_stubs.php is an optional tool that only helps when writing C, so disallowing resources in zend_internal_arginfo needs to be done at the Zend API level (in zend_declare_typed_constant, zend_declare_typed_property, zend_register_functions, et al), which would be a breaking change.

All in all since the resource type is de facto already supported for internal functions, maybe the most backward-compatible solution would be to just skip it in the ReflectionType API?

[iluuu1994]

My question was about internal functions.

Well, then you skipped the second half of my message.

Nowadays the types supported in core/userland should mostly be congruent.

If you want to declare function args yourself, you run the risk of making mistakes. There are many ways you can mess up, some of them being impossible to detect in C (e.g. arity mismatch/buffer overflow, invalid pointers, etc).

So I'm genuinely curious about where to find these kind of informations.

I think you know the answer ;) Internal documentation is spotty. Just like the language itself, core is specified by its implementation. The only reliable way to find out how something works is to check the code.

No, I'm afraid not. Disallowing resources in zend_internal_arginfo needs to be done at the Zend API level.

Sure, we may also add checks to zend_declare_typed_constant, zend_declare_typed_property, zend_register_functions and so forth. But we do not consider misuse of internal API bugs.

[ju1ius]

@nikic Thanks for your feedback, I've submitted a PR implementing your suggestions. 👍🏻