tests/x86: add regression test for TCG temps overflow on dense x87 blocks - #2372
Open
ioma8 wants to merge 1 commit into
Open
tests/x86: add regression test for TCG temps overflow on dense x87 blocks#2372ioma8 wants to merge 1 commit into
ioma8 wants to merge 1 commit into
Conversation
Member
|
Hi, could you please send your patch to dev branch instead? |
Contributor
Dense x87 basic blocks (real-world matrix math from a 32-bit Windows game) drive nb_temps past TCG_MAX_TEMPS during translation; temps[] overflows into the ops queue head and tcg_emit_op crashes with a NULL deref. Reproduced on arm64 macOS hosts, see also unicorn-engine#2240. The test documents the overflow: it SIGSEGVs without the temps overflow error handling from unicorn-engine#2307 and passes with it, with TCG_MAX_TEMPS left at 512. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ioma8
force-pushed
the
fix/tcg-max-temps-64bit
branch
from
July 24, 2026 06:36
bdceb21 to
3222ae7
Compare
ioma8
changed the title
Author
Yes changed the target branch. |
Author
Yes I see. My workaround fixed it for the game to run well, but isnt root cause solution. #2307 seems to fix it at underlying level and I validated it works. Still I think my PR contains good real-data x86/x87 reproducer test. Do you want me to keep the PR open / do you wanna cherrypick it? |
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
3 participants
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.
Summary
Reworked per review feedback (originally this raised
TCG_MAX_TEMPSto 1024 on 64-bit hosts): this PR is now test-only, rebased ontodev, and adds a real-world x86 regression test for the TCG temps overflow.@PhilippTakacs was right that raising the limit only moves the threshold. I verified against #2307:
test_x86_tcg_temp_limit_x87_blockdev(TCG_MAX_TEMPS512)tcg_emit_opdev+ #2307, limit unchangedUC_ERR_OKSo #2307's overflow error handling fixes the underlying problem; the limit raise is unnecessary and has been dropped.
The bug this test covers
During translation of long basic blocks,
nb_tempscan exceedTCG_MAX_TEMPS.tcg_temp_alloc()only guards withtcg_debug_assert, a no-op in release builds, sotemps[]silently overflows into the adjacentopsqueue head inTCGContext, and the nexttcg_emit_op()crashes on a NULLtqh_last. Same failure class as #2240 (MIPS), reproduced here on x86.The test embeds a dense x87 basic block (matrix math from a 32-bit Windows game, found via OpenSWE1R on Apple Silicon) that reliably drives
nb_tempspast 512 (nb_temps == 513,nb_ops == 3830at crash).Merge order
This complements #2307 with an x86 reproducer (it currently only has a MIPS test) and must land after it — on current
devthe test SIGSEGVs, so CI will fail until #2307 is merged. Happy to have the test cherry-picked into #2307 directly instead, if that is preferred.Test plan
./test_x86 test_x86_tcg_temp_limit_x87_blockon arm64 macOS: SIGSEGV on stockdev, passes ondev+ Add error handling for tcg temps overflow #2307