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

AArch64: Some SVE memory operands not set correctly #2055

Closed
accauble opened this issue Jun 21, 2023 · 5 comments · Fixed by #2026
Closed

AArch64: Some SVE memory operands not set correctly #2055

accauble opened this issue Jun 21, 2023 · 5 comments · Fixed by #2026

Comments

@accauble
Copy link

Description

The operands for some SVE memory instructions (at least those that are "vector plus immediate" addressing modes) are not set correctly. Where there should be a memory type, it is labeled as a register type. The immediate should be marked as the displacement, but it is stored as an immediate in the next operand, which is labeled as INVALID.

Examples

Using commit d0ef88a on the next branch:

$ ./cstool -d arm64be "c5 a1 c4 24"
 0  c5 a1 c4 24  ld1d	{z4.d}, p1/z, [z1.d, #8]
	ID: 435 (ld1d)
	op_count: 4
		operands[0].type: REG = z4
			Vector Arrangement Specifier: 0xd
		operands[1].type: REG = p1
		operands[2].type: REG = z1

In the above, the string is correct, but operand 2 should be of type mem and have a displacement of 8.

A similar example, but a different instruction:

$ ./cstool -d arm64be "e5 41 a4 22"
 0  e5 41 a4 22  st1w	{z2.d}, p1, [z1.d, #4]
	ID: 976 (st1w)
	op_count: 4
		operands[0].type: REG = z2
			Vector Arrangement Specifier: 0xd
		operands[1].type: REG = p1
		operands[2].type: REG = z1

Again, the string is correct but the second operand should be a memory operand.

Suggested Solution

In AArch64InstPrinter.c:printSVERegOp, the MI->csh>doing_mem flag could be checked, and operand information could be set accordingly. This would be very similar to the printOperand function in the same file:

if (doing_mem)
    if (base is INVALID)
        mem.base = Reg
    else if (index is INVALID)
        mem.index = Reg
else
    set access
    type = REG
    reg = Reg
    op_count++
accauble added a commit to accauble/capstone that referenced this issue Jun 21, 2023
…tion in

`AArch64InstPrinter.c` that is very similar to the one made by
`printOperand` in the same file. If we are in the middle of
printing a memory operand, the operand type is no longer changed
to type REG. Instead, a base or index register is set.
@Rot127
Copy link
Collaborator

Rot127 commented Jun 21, 2023

Will fix it in #2026
Currently it is still registers and immediate are still printed separately. Would it make sense to make it a separated operand? Like for the SME matrix operations?

typedef struct aarch64_op_sme {
  aarch64_sme_op_type type; ///< AArch64_SME_OP_TILE, AArch64_SME_OP_TILE_VEC, AArch64_SME_OP_ACC_MATRIX
  aarch64_reg tile; ///< Matrix tile register
  aarch64_reg slice_reg; ///< slice index reg
  int8_t slice_offset; ///< slice index offset
  bool is_vertical;	///< Flag if slice is vertical or horizontal
} aarch64_op_sme;

@accauble
Copy link
Author

Wow very excited for the updates in Issue #2026! I don't currently see a need for something different than the original arm64_op_mem/aarch64_op_mem, at least for the SVE instructions. The SVE addressing modes can fit neatly into the struct:

Scalar + immediate: ld1w {z1.d}, p0/z, [x1, #imm]
Scalar + scalar: ld1w {z1.s}, p0/z, [x1, x2, lsl #2]
Scalar + vector: ld1w {z1.s}, p0/z, [x1, z2.s, sxtw #2]
Vector + immediate: ld1w {z1.s}, p0/z, [z2.s, #imm]

The only thing missing is the memory data type (e.g. ld1w loads words before extending).

I did implement a fix in my own repo and reran the examples above. I hope this helps you add to your own tests.

$ ./cstool -d arm64be "c5 a1 c4 24"
 0  c5 a1 c4 24  ld1d	{z4.d}, p1/z, [z1.d, #8]
	ID: 435 (ld1d)
	op_count: 3
		operands[0].type: REG = z4
			Vector Arrangement Specifier: 0xd
		operands[1].type: REG = p1
		operands[2].type: MEM
			operands[2].mem.base: REG = z1
			operands[2].mem.disp: 0x8
	Registers read: z1
$ ./cstool -d arm64be "e5 41 a4 22"
 0  e5 41 a4 22  st1w	{z2.d}, p1, [z1.d, #4]
	ID: 976 (st1w)
	op_count: 3
		operands[0].type: REG = z2
			Vector Arrangement Specifier: 0xd
		operands[1].type: REG = p1
		operands[2].type: MEM
			operands[2].mem.base: REG = z1
			operands[2].mem.disp: 0x4
	Registers read: z1

@Rot127
Copy link
Collaborator

Rot127 commented Jun 21, 2023

Nice! Can you give an example for ld1w {z1.s}, p0/z, [x1, z2.s, sxtw #2] and ld1w {z1.s}, p0/z, [x1, x2, lsl #2].
Just to have some test cases.

@accauble
Copy link
Author

Sure:

$ ./cstool -d arm64be "85 62 40 21" 
 0  85 62 40 21  ld1w	{z1.s}, p0/z, [x1, z2.s, sxtw #2]
	ID: 457 (ld1w)
	op_count: 3
		operands[0].type: REG = z1
			Vector Arrangement Specifier: 0xb
		operands[1].type: REG = p0
		operands[2].type: MEM
			operands[2].mem.base: REG = x1
			operands[2].mem.index: REG = z2
			Shift: type = 1, value = 2
			Ext: 7
	Registers read: x1 z2
$ ~./cstool -d arm64be "a5 42 40 21" 
 0  a5 42 40 21  ld1w	{z1.s}, p0/z, [x1, x2, lsl #2]
	ID: 457 (ld1w)
	op_count: 3
		operands[0].type: REG = z1
			Vector Arrangement Specifier: 0xb
		operands[1].type: REG = p0
		operands[2].type: MEM
			operands[2].mem.base: REG = x1
			operands[2].mem.index: REG = x2
			Shift: type = 1, value = 2
	Registers read: x1 x2

Note that the access is not reported for these operands and the final registers read/written is incomplete. Register z1 should be written, for instance. This is mentioned in other issues (#1911). I don't know if you plan to fix that in #2026.

@Rot127
Copy link
Collaborator

Rot127 commented Jun 22, 2023 via email

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 a pull request may close this issue.

2 participants