Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

68HC05: Fix #7064 - incorrect handling of X-indexed JMP and JSR #7065

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

philpem
Copy link
Contributor

@philpem philpem commented Oct 16, 2024

Fix for #7064 (incorrect handling of X-indexed JMP and JSR)

Fix for NationalSecurityAgency#7064 (incorrect handling of X-indexed JMP and JSR)
@GhidorahRex
Copy link
Collaborator

This doesn't fix the issue for the HC05/HC08/HCS08. The issue is that the subconstructor is exporting an address, so the goto [ADDRI]; tries to de-reference the address, whereas goto ADDRI tries to go to the unique varnode U500:1. The solution is to change the subconstructor to export the value, rather than the address. But the subconstructors oprx8_8_X, oprx16_8_X, and comma_X are also used in OP1 which should export the address.

So this is what I came up with, and it seems to work well, leaving the brackets in ADDRI. I imagine the same changes would be needed for the 6805:

@if defined(HC05)
oprx8_8_X:		imm8,X   is imm8 & X   { address:2 = zext(X) + imm8;   export  address; }
oprx16_8_X:		imm16,X  is imm16 & X  { address:2 = zext(X) + imm16;  export  address; }
comma_X:		","X     is X		   { address:2 = zext(X);          export  address; }
@endif

@if defined(HCS08) || defined(HC08)
oprx8_8_X:		imm8,X   is imm8 & X   { address:2 = HIX + imm8;       export address; }
oprx16_8_X:		imm16,X  is imm16 & X  { address:2 = HIX + imm16;      export address; }
comma_X:		","X     is X		   { address:2 = HIX;              export address; }
@endif

and

OP1: iopr8i		is op4_6=2; iopr8i       { export iopr8i; }
OP1: opr8a_8	is op4_6=3; opr8a_8      { export opr8a_8; }
OP1: opr16a_8	is op4_6=4; opr16a_8     { export opr16a_8; }
OP1: oprx16_8_X	is op4_6=5; oprx16_8_X   { export *:1 oprx16_8_X; }
OP1: oprx8_8_X	is op4_6=6; oprx8_8_X    { export *:1 oprx8_8_X; }
OP1: comma_X	is op4_6=7 & comma_X     { export *:1 comma_X; }

@GhidorahRex GhidorahRex self-assigned this Oct 17, 2024
Copy link

@byt3n33dl3 byt3n33dl3 left a comment

Choose a reason for hiding this comment

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

JMP and JSR Code Review

Copy link

@byt3n33dl3 byt3n33dl3 left a comment

Choose a reason for hiding this comment

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

Format / Master

maybe back to "[ADDRI]" ?

@philpem
Copy link
Contributor Author

philpem commented Oct 29, 2024

This doesn't fix the issue for the HC05/HC08/HCS08. The issue is that the subconstructor is exporting an address, so the goto [ADDRI]; tries to de-reference the address, whereas goto ADDRI tries to go to the unique varnode U500:1. The solution is to change the subconstructor to export the value, rather than the address. But the subconstructors oprx8_8_X, oprx16_8_X, and comma_X are also used in OP1 which should export the address.

So this is what I came up with, and it seems to work well, leaving the brackets in ADDRI. I imagine the same changes would be needed for the 6805:

Yep. I agree that this is the correct fix - it seems to work for me too, so I've pulled it into my branch.

I've applied the same fix to the 6805 SLASPEC too -- this also had a bug where the non-offset indexed syntax would be incorrect -- it would show as LDA X instead of LDA ,X.

I'll have to read the SLEIGH spec more closely in regard to what the *:1 does. Mea culpa for pushing this without properly testing it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants