Skip to content

Test precision cap in zend_gcvt() for GH-24563 - #24688

Open
aasul wants to merge 1 commit into
php:PHP-8.4from
aasul:investigate/gh24563-gcvt-precision-cap
Open

aasul wants to merge 1 commit into
php:PHP-8.4from
aasul:investigate/gh24563-gcvt-precision-cap

Conversation

@aasul

@aasul aasul commented Oct 25, 2026

Copy link
Copy Markdown

this fixes the excessive temporary allocations caused by unusually large serialize_precision values during float to string conversion

currently zend_gcvt() passes ndigit directly to zend_dtoa(), which can cause multi MB Bigint allocations even when the actual formatted output is much shorter

the patch caps positive precision values at 770 before calling zend_dtoa(). this is based on the maximum 767 significant decimal digits needed to represent a binary64 value exactly. negative precision and the existing default behavior are left unchanged

the original problem can be reproduced with:

ini_set('serialize_precision', 4194304);
echo var_export(0.1, true);

under tight RLIMIT_AS limits this can trigger Balloc() failed to allocate memory on the unpatched build

i tested the change on Linux x86_64 using NTS and ZTS debug builds. with the patch, the largest conversion malloc dropped from 4194332 to 1052 bytes and peak tracked live allocation dropped from 4194492 to 1212 bytes. peak virtual memory also decreased by roughly 4 MB

i compared the output across 387 test value entries including subnormals, large doubles, negative zero, INF, NAN and different precision settings. no byte for byte differences were found between the original and patched builds

FPM testing covered 648 requests under controlled memory limits. the baseline produced 20 Balloc() fatals per worker while the patched version completed those conversions under the same conditions. subsequent requests continued working correctly and the worker PIDs remained unchanged

the relevant NTS and ZTS tests passed with 412 passes, 43 skips and no failures each, including the new PHPT

@srayuth089 also independently tested the patch on macOS arm64 and confirmed that the original failure no longer occurs within the tested memory limits. their 302 conversion battery produced identical output between the original and patched builds, including with serialize_precision=4194304

this change only addresses the excessive precision allocation path through zend_gcvt(). it doesn't change the general OOM handling or other direct zend_dtoa() callers

thanks to @srayuth089 for suggesting the common zend_gcvt() cap and independently verifying the patch

related to #24563

the investigation was assisted by AI tools and reviewed by me

@aasul
aasul requested a review from bukka as a code owner October 25, 2026 13:53
Copilot AI balanced review requested due to automatic review settings October 25, 2026 13:53

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

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.

2 participants