Repository navigation
Claude/upgrade ape c library mm z gd - #731
Merged
Merged
Conversation
44 of 45 cases pass on the VM, including PHP_INT_SIZE 8, PHP_INT_MAX,
strlen, max, str_repeat at 100k, preg, json, and /net/tcp/stats read
as 404 bytes. The interpreter evaluates correctly, which is the
thing `php -v' could not say.
Section 5 reproduces display_text_table's padding loop over the exact
table php had printed before dying, and it passes: $m is 15, not
2**32. So the Phoronix diagnosis was wrong and the test is what said
so. The smoke test dies on its own line 194, which is `exit($fails);'
-- after printing its total -- and Phoronix's line 854 is `exit;'.
Two unrelated programs, the same line kind, the same byte count.
The size decomposes: 4294971392 is 1048577 pages of 4096, a request
of just over 2**32 rounded up one page, i.e. a length of exactly
2**32 plus a header. 2**32 is (uint32_t)-1 + 1, so the shape is a
32-bit -1 zero-extended into a 64-bit size and incremented -- not
sign-extended, which would have given 0. An identical size in two
programs of different sizes is not data-dependent.
exit compiles to ZEND_EXIT, which calls zend_bailout, which is
LONGJMP into EG(bailout) -- and zend_portability.h:427 makes that
siglongjmp into a sigjmp_buf unconditionally on non-Windows, with no
HAVE_SIGSETJMP to answer. APExp's sigsetjmp is assembly and its
siglongjmp is C, and on inspection the two agree about the layout, so
nothing is ruled in or out yet; what is established is that the fault
is on the bailout-and-shutdown path and not in the engine.
Also fixed an instrument fault. The PHP_INT_MIN case was
eq("PHP_INT_MIN", PHP_INT_MIN, -9223372036854775807 - 1);
and on Plan 9 it printed `got -9223372036854775807-1' against `want
-9.223372036854776E+18': the got side is correct and the EXPECTED
side carried the bug, because the subtraction overflowed into a
float. An assertion whose expected value is computed by the thing
under test reports a fault as a wrong expectation -- the same shape
as the xcheck that read both sides out of the field it was testing.
It is four direct cases now, and the overflow is a finding in its own
right: -9223372036854775807 - 1 must stay an int.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WGAwvvTwDg2yknFkmZ3qzs
…Gd' into claude/upgrade-ape-c-library-mmZGd
The merge of main left the corrected document and the original concatenated -- 500 lines where the rewrite is 297. Same content as before, with the superseded copy removed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WGAwvvTwDg2yknFkmZ3qzs
Three one-liners settled where it was, with no rebuild: a script that just ends is clean, `exit(0)' dies, and the memory limit is not involved. So request shutdown and module shutdown are both fine and it is ZEND_EXIT alone. exit calls zend_throw_unwind_exit, which is zend_objects_new(&zend_ce_unwind_exit), which is emalloc(sizeof(zend_object) + zend_object_properties_size(ce)) and that function is sizeof(zval) * (ce->default_properties_count - ((ce->ce_flags & ZEND_ACC_USE_GUARDS) ? 0 : 1)) UnwindExit has zero properties and ZEND_ACC_USE_GUARDS is set only under CG(compiler_options) & ZEND_COMPILE_GUARDS, which nothing in php ever sets -- so the subtraction is 0 - 1, deliberately: zend_object ends in `zval properties_table[1]', so a class with no properties wants one zval less than the struct, and sizeof(zval) * -1 is 2**64-16, which the caller's + sizeof(zend_object) wraps back to 40. Upstream is right and the compiler is the deviation. C makes sizeof a size_t, 64 bits here; cc/com.c:894 types it TULONG, 32 bits, on purpose and with its reason in a comment. `6c -J -S' on the expression is SUBL / SALL $4 / MOVLQZX -- a 32-bit multiply and then a ZERO-extend -- so 4294967280 instead of 2**64-16, plus 56 is 4294967336, which the allocator page-rounds to 4294971392: the number from the Phoronix crash and from the smoke test, to the digit. The same source compiled by gcc asks for 40. Both numbers were run. Fixed in the vendored header by computing in ptrdiff_t, which is signed and size_t's width on every target. kencc then emits MOVLQSX / SUBQ / SALQ and agrees with gcc; 515 TUs still compile. The compiler change is recorded and NOT taken. It is one line -- TUVLONG where ewidth[TIND] is 8 -- and it would introduce a new silent failure wherever a sizeof is printed with %d, which is ordinary in Plan 9's own sources. docs/notes/kencc.md has both -S listings, the gcc control, the trade, and the control set to run if it is ever taken; 0.6's inventory carries the entry; and three more instances of the idiom in php's optimizer are named, unmeasured. Two reading errors on the way, both recorded. I blamed display_text_table's padding loop from the line number alone, and the smoke test's reduction of that loop passes -- the real line was `exit' in both programs. And my first reduction declared default_properties_count as uint32_t when it is int, so gcc "confirmed" a 68GB request and briefly made upstream look broken on Linux as well. A reduction that changes a type is not a reduction. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WGAwvvTwDg2yknFkmZ3qzs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.