Fix bug in opcode 0x87 (MOV @Ri, direct) #41
Conversation
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
The mov_mem_indir_rx function was using wrong memory access functions: - Used read_mem_indir() for DIRECT source operand (should be read_mem()) - Used write_mem() for INDIRECT destination operand (should be write_mem_indir()) This caused stack corruption when Ri pointed to Upper RAM (0x80-0xFF) and direct was an SFR address (0x80-0xFF), because write_mem(0x81, val) would write to mSFR[0x01] (SP register) instead of mUpperData[0x01]. Impact: Bosch MA1.7 firmware crashed every ~12k instructions due to SP corruption. After fix: Firmware runs indefinitely without crashes. Bug discovered while emulating SAB 80C517 firmware that uses MOV @r1, B where R1=0x81 (Upper RAM) and B=0xF0 (SFR).
Owner
|
I wouldn't be surprised if there are critical bugs, and thanks for the patch - however, I'm not able to verify it in either way at the moment; please leave it open, so if someone else has a problem they can find the solution. |
Author
Sorry if “critical bug” sounded too strong, the commit message was generated from the LLM description, which tends to be hyperbolic most of the time. I thought I had rewrite the PR title before sending. Thanks for the response, I’ll leave the PR open. |
Fabio3rs
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
I was studying a simulation of real firmware created to a Siemens 80, likely a SAB80C517 (similar to a 8052 in features) and there was a stackoverflow, I used claude code to help debug and found this, I am not sure if this is 100% right, but it seems to fixed the problem, so I am creating this pull request.
Follows the claude description to the problem:
The mov_mem_indir_rx function was using wrong memory access functions:
This caused stack corruption when Ri pointed to Upper RAM (0x80-0xFF) and direct was an SFR address (0x80-0xFF), because write_mem(0x81, val) would write to mSFR[0x01] (SP register) instead of mUpperData[0x01].
Impact: Bosch MA1.7 firmware crashed every ~12k instructions due to SP corruption.
After fix: Firmware runs indefinitely without crashes.
Bug discovered while emulating SAB 80C517 firmware that uses MOV @r1, B where R1=0x81 (Upper RAM) and B=0xF0 (SFR).