Skip to content

Fix GH-24988: Incorrect opcode range check causes bugs with exceptions in function JIT - #25070

Closed
ndossche wants to merge 2 commits into
php:PHP-8.4from
ndossche:fix-24988
Closed

ndossche wants to merge 2 commits into
php:PHP-8.4from
ndossche:fix-24988

Conversation

@ndossche

Copy link
Copy Markdown
Member

range->var is a byte offset, but var_num is an index, so we have to convert the index to the byte offset using EX_NUM_TO_VAR(). This matches cleanup_live_vars() code.

…ions in function JIT

`range->var` is a byte offset, but `var_num` is an index, so we have to
convert the index to the byte offset using `EX_NUM_TO_VAR()`.
This matches `cleanup_live_vars()` code.

@jorgsowa jorgsowa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix looks good to me

Comment thread ext/opcache/tests/jit/gh24988.phpt Outdated
Comment on lines +12 to +34
<?php

function deep(int $a, int $b, int $c) {
$a &= 255;
return ($a + 1) - ($b + $b + (($a + 1) % ($b % $c)));
}

$out = [deep(15, 3, 7)];
try {
deep(15, 3, 0);
} catch (DivisionByZeroError $e) {
$out[] = $e->getMessage();
}
var_dump($out);

?>
--EXPECT--
array(2) {
[0]=>
int(9)
[1]=>
string(14) "Modulo by zero"
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<?php

class D {
    function self() { return $this; }
    function __destruct() { echo "dtor\n"; }
}

// Leaves a stale object in the frame slot deep() uses for its first temporary
function pollute($a, $b, $c) {
    return $a->self()->self();
}

function deep(int $a, int $b, int $c) {
    $a &= 255;
    return ($a + 1) - ($b + $b + (($a + 1) % ($b % $c)));
}

$o = new D;
var_dump(deep(15, 3, 7));
pollute($o, 1, 2);
try {
    deep(15, 3, 0);
} catch (DivisionByZeroError $e) {
    echo $e->getMessage(), "\n";
}
echo "end\n";

?>
--EXPECT--
int(9)
Modulo by zero
end
dtor

Such test fails without ASan enabled.

@iliaal iliaal left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tracing JIT's version of this check compares opline->result.var, already a byte offset, so only this site needed the conversion. +1 to jorgsowa's test: the current one passes without the fix on a non-ASan debug build, while the proposed one fails there and passes with it.

Co-authored-by: Jorg Sowa <jorg.sowa@gmail.com>
@ndossche ndossche closed this in bc3a830 Oct 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JIT: crash in cleanup_live_vars when % by zero throws mid-expression (function JIT, optimization_level=0)

3 participants