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

Fix LoongArch aliases and CS_OPT_SYNTAX_NO_DOLLAR support #2594

Merged
merged 2 commits into from
Jan 6, 2025

Conversation

wargio
Copy link
Contributor

@wargio wargio commented Jan 1, 2025

Your checklist for this pull request

  • I've documented or updated the documentation of every API function and struct this PR changes.
  • I've added tests that prove my fix is effective or that my feature works (if possible)

Detailed description

For some reasons the CS_OPT_DETAIL_REAL and CS_OPT_SYNTAX_NO_DOLLAR were not supported in LoongArch, so it was impossible to get real info also from aliases.

@XVilka
Copy link
Contributor

XVilka commented Jan 1, 2025

cc @jiegec @FurryAcetylCoA

arch/LoongArch/LoongArchInstPrinter.c Outdated Show resolved Hide resolved
@wargio
Copy link
Contributor Author

wargio commented Jan 2, 2025

Looks like also the alias mapping was missing. now it should be correct.

Before the patch suggested

 0  20 00 00 4c  ret
        ID: 322 (jirl)
        op_count: 3
                operands[0].type: REG = zero
                operands[0].access: WRITE
                operands[1].type: REG = ra
                operands[1].access: READ
                operands[2].type: IMM = 0x0
                operands[2].access: READ
        Registers read: ra
        Registers modified: zero

after applying the patch from @Rot127

 0  20 00 00 4c  ret
        ID: 322 (jirl)
        Is alias: 2058 ((null)) with REAL operand set
        op_count: 3
                operands[0].type: REG = zero
                operands[0].access: WRITE
                operands[1].type: REG = ra
                operands[1].access: READ
                operands[2].type: IMM = 0x0
                operands[2].access: READ
        Registers read: ra
        Registers modified: zero

After adding mapping of the aliases.

 0  20 00 00 4c  ret
        ID: 322 (jirl)
        Is alias: 2058 (ret) with REAL operand set
        op_count: 3
                operands[0].type: REG = zero
                operands[0].access: WRITE
                operands[1].type: REG = ra
                operands[1].access: READ
                operands[2].type: IMM = 0x0
                operands[2].access: READ
        Registers read: ra
        Registers modified: zero

@wargio wargio changed the title Fix CS_OPT_DETAIL_REAL & CS_OPT_SYNTAX_NO_DOLLAR support in LoongArch Fix LoongArch Aliases and CS_OPT_SYNTAX_NO_DOLLAR support Jan 2, 2025
@wargio wargio changed the title Fix LoongArch Aliases and CS_OPT_SYNTAX_NO_DOLLAR support Fix LoongArch aliases and CS_OPT_SYNTAX_NO_DOLLAR support Jan 2, 2025
@wargio wargio requested a review from Rot127 January 2, 2025 02:07
Copy link
Collaborator

@Rot127 Rot127 left a comment

Choose a reason for hiding this comment

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

Only the suggestions, ten we are good to go.

tests/issues/issues.yaml Show resolved Hide resolved
tests/issues/issues.yaml Show resolved Hide resolved
tests/issues/issues.yaml Show resolved Hide resolved
tests/issues/issues.yaml Show resolved Hide resolved
Copy link
Collaborator

@Rot127 Rot127 left a comment

Choose a reason for hiding this comment

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

Copy link
Member

@kabeor kabeor left a comment

Choose a reason for hiding this comment

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

LGTM, thank you.

@kabeor kabeor merged commit 8629313 into capstone-engine:next Jan 6, 2025
20 checks passed
@wargio wargio deleted the fix-loongarch branch January 6, 2025 11:02
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.

5 participants