Skip to content

Conversation

@jiegec
Copy link
Collaborator

@jiegec jiegec commented May 14, 2025

Matching by type name can make incorrect assumption of memory operands, when the destnation register and base register share the same type of GPR. Drop this logic for now. This commit fixes the destination register info of load instructions of LoongArch ISA.

Addresses capstone-engine/capstone#2700.

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.

Thanks!

@Rot127 Rot127 changed the base branch from auto-sync to auto-sync-18 May 14, 2025 16:10
@Rot127
Copy link
Collaborator

Rot127 commented May 14, 2025

Can you please quickly rebase onto the upstream auto-sync-18 branch? I just updated it and Github doesn't seem to recognize it.

If you have the time it would be awesome if you could regenerate the tables in Capstone again and open a PR there. So we can close the issue.
ASUpdate -a LoongArch -w -s IncGen + some one or two tests it should already do the trick.

Matching by type name can make incorrect assumption of memory operands,
when the destnation register and base register share the same type of
GPR. Drop this logic for now. This commit fixes the destination register
info of load instructions of LoongArch ISA.

Fixes capstone-engine/capstone#2700.

Co-authored-by: Rot127 <[email protected]>
jiegec added a commit to jiegec/capstone that referenced this pull request May 15, 2025
According to capstone-engine#2700, the register info of LoongArch ld/st instructions is
wrong. A fix for the issue is proposed for llvm-capstone at
capstone-engine/llvm-capstone#79. This commit integrates the fix and
regenerates the tables via `ASUpdater -a LoongArch -w -s IncGen`. The
case change in LoongArchGenCSOpGroup.inc is also handled.

Fixes: capstone-engine#2700
@jiegec
Copy link
Collaborator Author

jiegec commented May 15, 2025

Can you please quickly rebase onto the upstream auto-sync-18 branch? I just updated it and Github doesn't seem to recognize it.

If you have the time it would be awesome if you could regenerate the tables in Capstone again and open a PR there. So we can close the issue. ASUpdate -a LoongArch -w -s IncGen + some one or two tests it should already do the trick.

Done! See capstone-engine/capstone#2701

@Rot127 Rot127 merged commit 566ab9c into capstone-engine:auto-sync-18 May 16, 2025
3 checks passed
jiegec added a commit to jiegec/capstone that referenced this pull request May 20, 2025
According to capstone-engine#2700, the register info of LoongArch ld/st instructions is
wrong. A fix for the issue is proposed for llvm-capstone at
capstone-engine/llvm-capstone#79. This commit integrates the fix and
regenerates the tables via `ASUpdater -a LoongArch -w -s IncGen`. The
case change in LoongArchGenCSOpGroup.inc is also handled.

Fixes: capstone-engine#2700
kabeor pushed a commit to capstone-engine/capstone that referenced this pull request May 20, 2025
According to #2700, the register info of LoongArch ld/st instructions is
wrong. A fix for the issue is proposed for llvm-capstone at
capstone-engine/llvm-capstone#79. This commit integrates the fix and
regenerates the tables via `ASUpdater -a LoongArch -w -s IncGen`. The
case change in LoongArchGenCSOpGroup.inc is also handled.

Fixes: #2700
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants