Skip to content

Fix segfault in do_operation handler (ext-decimal 2.0.x)#96

Merged
rtheunissen merged 1 commit intophp-decimal:masterfrom
andypost:master
Apr 15, 2026
Merged

Fix segfault in do_operation handler (ext-decimal 2.0.x)#96
rtheunissen merged 1 commit intophp-decimal:masterfrom
andypost:master

Conversation

@andypost
Copy link
Copy Markdown
Contributor

@andypost andypost commented Apr 14, 2026

Summary

  • The do_operation handler segfaults when encountering unsupported opcodes (ZEND_CONCAT, ZEND_BW_OR, etc.)
  • The handler throws ArithmeticError and returns SUCCESS without initializing the result zval, causing the VM's exception handler to crash in zval_delref_p
  • Fix by adding ZVAL_UNDEF(result) before returning SUCCESS for unsupported operators

Root Cause

This bug was introduced in commit b8b4420 (Jan 2019, "Alpha implementation of Number and Rational") when the unsupported-operator handling was changed from the 1.x behavior:

  • 1.x: return FAILURE; — engine falls through to type juggling (casting object to string/int)
  • 2.0.x: throws ArithmeticError + return SUCCESS + result left uninitialized — engine skips type juggling but crashes cleaning up the garbage result zval

This affects all PHP versions (8.2+), not only PHP 8.5. The concat_functiondo_operation(ZEND_CONCAT) call path has existed since PHP 5.6 (commit 089f4967997, Oct 2014). The ZEND_TRY_BINARY_OBJECT_OPERATION macro is identical between PHP 8.4 and 8.5.

Test Plan

  • All 131 tests pass on PHP 8.2, 8.3, 8.4, 8.5 (previously 3 tests failed with SIGSEGV)
  • Decimal/operators.phpt, Number/operators.phpt, Rational/operators.phpt now pass
  • Verified no regression on supported operators (+, -, *, /, %, **, <<, >>)

@andypost
Copy link
Copy Markdown
Contributor Author

@andypost
Copy link
Copy Markdown
Contributor Author

andypost commented Apr 14, 2026

This is not a PHP 8.5-only issue

After further investigation, the concat_functiondo_operation(ZEND_CONCAT) call path has existed since PHP 5.6 (commit 089f4967997, Oct 2014 — "Moved proxy object support in ASSIGN_ADD (and family) from VM to slow paths of corresponding operators"). The ZEND_TRY_BINARY_OBJECT_OPERATION macro and how concat_function uses it are identical between PHP 8.4 and 8.5.

The bug is in ext-decimal 2.0.x, introduced in commit b8b4420 (Jan 2019, "Alpha implementation of Number and Rational") when the unsupported-operator handling was changed from the 1.x behavior:

1.x (correct) 2.0.x (buggy)
Unsupported opcode return FAILURE; throws ArithmeticError + return SUCCESS + result left uninitialized
Engine behavior Falls through to type juggling (cast object to string/int) Skips type juggling, but crashes in zval_delref_p cleaning up garbage result zval

The ZVAL_UNDEF(result) fix is correct and needed on all PHP versions (5.6+) when using ext-decimal 2.0.x.

@andypost
Copy link
Copy Markdown
Contributor Author

Tested other branches and since PHP 8.2 all extension needs this patch

@andypost andypost changed the title Fix segfault in do_operation handler on PHP 8.5 Fix segfault in do_operation handler (ext-decimal 2.0.x) Apr 14, 2026
Bug was introduced in commit b8b4420 ("Alpha implementation of Number
and Rational", Jan 2019) which changed unsupported-operator handling
from return FAILURE (1.x) to throw ArithmeticError + return SUCCESS
with result zval left uninitialized.

The ZEND_TRY_BINARY_OBJECT_OPERATION macro in Zend engine has called
do_operation for all operator types (including CONCAT, BW_OR) since
PHP 5.6 (commit 089f4967997, Oct 2014). When do_operation throws an
exception but returns SUCCESS without initializing result, the VM's
exception handler crashes in zval_delref_p.

This affects all PHP versions (8.2+), not only PHP 8.5.

Fix by setting result to UNDEF before returning SUCCESS for unsupported
operators, so the exception handler safely skips cleanup.

Co-Authored-By: GLM-5.1 <noreply@z.ai>
@rtheunissen rtheunissen merged commit f8812de into php-decimal:master Apr 15, 2026
@rtheunissen
Copy link
Copy Markdown
Contributor

Thanks @andypost!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants