Nothing Special   »   [go: up one dir, main page]

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect result with reflection in low-trigger JIT #16572

Closed
YuanchengJiang opened this issue Oct 24, 2024 · 2 comments
Closed

Incorrect result with reflection in low-trigger JIT #16572

YuanchengJiang opened this issue Oct 24, 2024 · 2 comments

Comments

@YuanchengJiang
Copy link
YuanchengJiang commented Oct 24, 2024

Description

The following code:

<?php
$fusion=[1, 2];
function dumpType(ReflectionType $rt, int $indent = 0) {
        $str_indent = str_repeat(' ', 2 * $indent);var_dump($str_indent);
    echo $str_indent . "Type $rt is " . $rt::class . ":\n";
    echo $str_indent . "Allows Null: " . json_encode($rt->allowsNull()) . "\n";var_dump($str_indent);
    foreach ($rt->getTypes() as $type) {
        if ($type instanceof ReflectionNamedType) {
            echo $str_indent . "  Name: " . $type->getName() . "\n";var_dump($str_indent);
            echo $str_indent . "  String: " . (string) $type . "\n";var_dump($str_indent);
        } else {
            dumpType($fusion, $indent+1);var_dump($fusion);
        }
    }
}
function test1(): (X&Y)|(Z&Traversable)|null { }
dumpType((new ReflectionFunction('test1'))->getReturnType());

Resulted in this output (with JIT 1215):

Fatal error: Uncaught TypeError: Cannot use "::class" on null

But I expected this output instead:

Fatal error: Uncaught TypeError: dumpType(): Argument #1 ($rt) must be of type ReflectionType, null given

This could potentially lead to segfault.

PHP Version

nightly

Operating System

ubuntu 22.04

@nielsdos
Copy link
Member

Reduce to:

<?php
function dumpType(ReflectionType $rt) {
    var_dump($rt::class);
    dumpType(null);
}
function test1(): int { }
dumpType((new ReflectionFunction('test1'))->getReturnType());

@nielsdos
Copy link
Member

It appears the bug is related to recursive_call_through_jmp when we generate the recursive call to dumpType. It wants to jump to an entry basic block by doing begin = jit->bb_start_ref[call_num_args];. However, If not all arguments are guaranteed to be valid, this can skip the argument validation. I wonder if this was meant to be begin = jit->bb_start_ref[num_args]; instead...
Let me think a bit...

nielsdos added a commit to nielsdos/php-src that referenced this issue Oct 24, 2024
When a recursive call happens with invalid arguments, the maximum valid
arguments are computed and stored in `num_args`, but the RECV entry
block we jump to is `call_num_args` instead. This can skip argument
validation checks. Fix this by using `num_args` instead.
nielsdos added a commit to nielsdos/php-src that referenced this issue Oct 24, 2024
When a recursive call happens with invalid arguments, the maximum valid
arguments are computed and stored in `num_args`, but the RECV entry
block we jump to is `call_num_args` instead. This can skip argument
validation checks. Fix this by using `num_args` instead.
nielsdos added a commit that referenced this issue Oct 28, 2024
* PHP-8.4:
  Fix GH-16594: Assertion failure in DOM -> before
  Fix GH-16572: Incorrect result with reflection in low-trigger JIT
  Fix GH-16577: EG(strtod_state).freelist leaks with opcache.preload
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants