[broken, don't merge] correct register access attributes of Arm's LDM instruction

[covanam]

Arm's LDM first register is only written when writeback is true. Otherwise, it is read-only.

[covanam]

Please don't merge, this PR introduces a bug when MI->csh->detail is NULL

I will update this PR soon.

[Rot127]

We refactor the complete ARM module to make it easier to update with new LLVM releases.
This is the PR: #1949

The access information of operands are now generated (see ARMGenCSMappingInsnOp.inc).
The detail information gets set in several functions in ARMModule.c. This is also the place where READ/WRITE attributes get added. The constraints of an instruction (which determine the writeback information) are checked in ARMDisassembler.c::ARM_setWriteback().

Please be so kind and fix this directly on the branch of #1949. Otherwise this fix will be gone directly after #1949.

Let me know if you wish any more details how this could be implemented.

[Rot127]

I recommend to checkout the comment of enum MCOI_OperandConstraint to understand how the LLVM constraints work. Then parse and store them in a new member of MCInst. In the detail filling functions you can then check those constraints.

[covanam]

@Rot127 Thank you so much for the information. Unfortunately I do not have the time to look into those at the moment. I am using Capstone for my thesis project, and noticed and fixed this bug. I can spare a few minutes to open this PR, but I cannot commit more than that.

If anyone would like to fix the bug being addressed by this PR, you are very welcome to do so. Otherwise, I will come back in a few months once my thesis is completed.

Thanks!

[covanam]

The access information of operands are now generated (see ARMGenCSMappingInsnOp.inc). The detail information gets set in several functions in ARMModule.c. This is also the place where READ/WRITE attributes get added. The constraints of an instruction (which determine the writeback information) are checked in ARMDisassembler.c::ARM_setWriteback().

This bug exists because Capstone does not use ARMGenCSMappingInsnOp.inc to get READ/WRITE attributes for LDM instruction, but instead hard-code the value in C code. The values in ARMGenCSMappingInsnOp.inc are already correct. If READ/WRITE attributes are no longer hard-coded in C code after the mentioned PR, then this PR should not be needed. Feel free to close this if that's true.

[Rot127]

That is really nice to hear then. Could you please provide me with two LDM instructions and their encoding so I can add a test for it?

[covanam]

That is really nice to hear then. Could you please provide me with two LDM instructions and their encoding so I can add a test for it?

Sure, can you try these?
ldm r0, {r1, r2, r3}; encoding: 90 e8 0e 00; expect READ/WRITE attributes: R, W, W, W
ldm r0!, {r1, r2, r3}; encoding: 0e c8; expect READ/WRITE attributes: RW, W, W, W

Encoding is Thumb, little endian.

Thanks!

[Rot127]

Unfortunately the correct write attributes can't be fixed currently. You need to do it by hand (after #1949 is merged).
It is an inherent problem of LLVM. See the issue here: llvm/llvm-project#62455

I'll provide a patch file after it is done and track it in an issue.

[Rot127]

@covanam It is now part of auto-sync. You can close this PR and wait for the merge of #1949 or fix it so these changes will be part of v5.

[Rot127]

@kabeor Can be closed. Fix has been merged with #2115