Fixing TriCore disasm instructions #2088

bkoppelmann · 2023-07-16T11:20:07Z

Hi,
while trying to integrate Capstone into QEMU, I came across some bugs.
Here are the fixes :)

Cheers,
Bastian

XVilka · 2023-07-17T02:15:50Z

@imbillow take a look please when you have time

imbillow · 2023-07-20T02:55:03Z

arch/TriCore/TriCoreInstPrinter.c

+static void off8_fixup(MCInst *MI, uint64_t *off8)
+{
+	switch (MCInst_getOpcode(MI)) {
+	case TRICORE_LD_A_sc:
+	case TRICORE_ST_A_sc:
+	case TRICORE_ST_W_sc:
+	case TRICORE_LD_W_sc: {
+		*off8 *= 4;


Suggested change

static void off8_fixup(MCInst *MI, uint64_t *off8)

{

switch (MCInst_getOpcode(MI)) {

case TRICORE_LD_A_sc:

case TRICORE_ST_A_sc:

case TRICORE_ST_W_sc:

case TRICORE_LD_W_sc: {

*off8 *= 4;

static void const8_fixup(MCInst *MI, uint64_t *const8)

{

switch (MCInst_getOpcode(MI)) {

case TRICORE_LD_A_sc:

case TRICORE_ST_A_sc:

case TRICORE_ST_W_sc:

case TRICORE_LD_W_sc: {

*const8 *= 4;

imbillow · 2023-07-20T02:55:19Z

arch/TriCore/TriCoreInstPrinter.c

@@ -235,6 +248,9 @@ static void print_zero_ext(MCInst *MI, int OpNum, SStream *O, unsigned n)
 		if (n == 4) {
 			off4_fixup(MI, &imm);
 		}
+		if (n == 8) {
+			off8_fixup(MI, &imm);


Suggested change

off8_fixup(MI, &imm);

const8_fixup(MI, &imm);

imbillow · 2023-07-20T03:17:54Z

arch/TriCore/TriCoreDisassembler.c

+			case TRICORE_ABSS_H_rr:
+			case TRICORE_ABS_H_rr:
+			case TRICORE_ABS_B_rr:
+			case TRICORE_ABS_rr: {


I checked and found that you still haven't fully fixed it here.
Can you please double-check the following instructions?

d, s2[, s1[, n]]

ADDSC.A
ADDSC.AT
MOV.A
MOV.AA
MOV.D
CRC32.B
CRC32B.W
CRC32L.W

a

UPDFL

They have been correct before. Not sure why. These are the insns I sent to capstone as well as GNU objdump and they agree.

CODE = b'\x01\x23\x01\x16' # addsc.a %a1,%a2,%d3,1 CODE += b'\x01\x34\x20\x26' # addsc.at %a2,%a3,%d4 CODE += b'\x01\x20\x30\x16' # mov.a %a1,%d2 CODE += b'\x01\x30\x00\x20' # mov.aa %a2,%a3 CODE += b'\x01\xb0\xc0\xa4' # mov.d %d10,%a11 CODE += b'\x4b\xbc\x60\xa0' # crc32.b %d10,%d11,%d12 CODE += b'\x4b\xbc\x30\xa0' # crc32 %d10,%d11,%d12 CODE += b'\x4b\xbc\x70\xa0' # crc32l.w %d10,%d11,%d12 CODE += b'\x4b\x0a\xc1\xa0' # updfl %d10

I remembered that we can also change the order of operands in the td file.

XVilka · 2023-07-20T07:29:33Z

@bkoppelmann @imbillow do we have tests for instructions in question? If not - we should add in a testsuite.

bkoppelmann · 2023-07-23T10:16:35Z

I can create tests. However, I'm not quite sure where to put them. Is suite/MC/TriCore/ the right place?

XVilka · 2023-07-23T10:29:58Z

Is suite/MC/TriCore/ the right place?

Yes

when we got 2 operands we were expecting them to be one of the CACHE insns. However, 2 operands are also used by ld/st insns with bitreverse +r addressing mode. In that case we would print the registers in the wrong order.

abs instruction use 's2' as the first source operand and not 's1'.

extr and extr.u in the RRRR format use 's3' as the second source operand instead of 's2'.

For these insns destination and source are swapped compared to their ld counterparts.

we need to multiply const8 by 4 for the SC variants of ld/sw.

bkoppelmann · 2023-07-23T15:09:31Z

@bkoppelmann @imbillow do we have tests for instructions in question? If not - we should add in a testsuite.

Done

XVilka · 2023-07-25T02:28:36Z

@imbillow take a look again please.

XVilka · 2023-07-25T03:57:06Z

@kabeor this one is ready to merge. Also worth importing into v5 branch.

kabeor · 2023-07-26T06:46:58Z

Great! Thanks for your contribution!

* Add Python bindings for WASM * Update Python bindings for m68k * Update Python bindings for mos65xx * Update Python bindings for x86 * Add Python bindings for SH * Update CS_* constants in Python bindings * Update constants from ARM auto-sync patch * Fixing TriCore disasm instructions (#2088) * allow absolute CMAKE_INSTALL_*DIR (#2134) This patch fixes Capstone 5 build on NixOS. NixOS's build infrastructure sets CMAKE_INSTALL_{LIB,INCLUDE}DIR to absolute paths. If you append it to ${prefix}, you get the wrong path. NixOS automatically detects it and links this issue: NixOS/nixpkgs#144170 --------- Co-authored-by: Peace-Maker <peace-maker@wcfan.de> Co-authored-by: Bastian Koppelmann <bkoppelmann@users.noreply.github.com> Co-authored-by: chayleaf <chayleaf@protonmail.com>

* Add Python bindings for WASM * Update Python bindings for m68k * Update Python bindings for mos65xx * Update Python bindings for x86 * Add Python bindings for SH * Update CS_* constants in Python bindings * Update constants from ARM auto-sync patch * Fixing TriCore disasm instructions (#2088) * allow absolute CMAKE_INSTALL_*DIR (#2134) This patch fixes Capstone 5 build on NixOS. NixOS's build infrastructure sets CMAKE_INSTALL_{LIB,INCLUDE}DIR to absolute paths. If you append it to ${prefix}, you get the wrong path. NixOS automatically detects it and links this issue: NixOS/nixpkgs#144170 * Disable swift binding const generate * update bindings const * update capstone version * update ChangeLog --------- Co-authored-by: Peace-Maker <peace-maker@wcfan.de> Co-authored-by: Bastian Koppelmann <bkoppelmann@users.noreply.github.com> Co-authored-by: chayleaf <chayleaf@protonmail.com>

bkoppelmann force-pushed the tricore-fixes branch from 2cdeaae to 8c383ac Compare July 16, 2023 13:31

XVilka mentioned this pull request Jul 17, 2023

5.0.x release suggestions #2081

Closed

24 tasks

imbillow suggested changes Jul 20, 2023

View reviewed changes

bkoppelmann force-pushed the tricore-fixes branch from 8c383ac to 16ec69d Compare July 23, 2023 10:13

Bastian Koppelmann added 10 commits July 23, 2023 17:07

Fix TriCore ld/st instructions using +r registers

d0ac226

when we got 2 operands we were expecting them to be one of the CACHE insns. However, 2 operands are also used by ld/st insns with bitreverse +r addressing mode. In that case we would print the registers in the wrong order.

Fix TriCore abs insns using wrong source operand

11254b6

abs instruction use 's2' as the first source operand and not 's1'.

Fix TriCore extr/_u insns using wrong source operand

54f2f14

extr and extr.u in the RRRR format use 's3' as the second source operand instead of 's2'.

Fix TriCore mtcr insns using wrong source register

b6d6904

Fix TriCore store insns using +r or +c operands

13f9567

For these insns destination and source are swapped compared to their ld counterparts.

TriCore: Add const8_fixup()

8228bd3

we need to multiply const8 by 4 for the SC variants of ld/sw.

suite/TriCore: Add test for rr insns

23455ce

suite/TriCore: Add tests for ld/st with +r and +c operands

098a8ca

suite/TriCore: Add tests for extr and extr.u insns

5944315

suite/TriCore: Add tests for mtcr mfcr insns

62b99ad

bkoppelmann force-pushed the tricore-fixes branch from 16ec69d to 62b99ad Compare July 23, 2023 15:09

XVilka approved these changes Jul 24, 2023

View reviewed changes

imbillow approved these changes Jul 25, 2023

View reviewed changes

kabeor merged commit 9099567 into capstone-engine:next Jul 26, 2023

kabeor pushed a commit to kabeor/capstone that referenced this pull request Aug 21, 2023

Fixing TriCore disasm instructions (capstone-engine#2088)

a3ad50e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixing TriCore disasm instructions #2088

Fixing TriCore disasm instructions #2088

bkoppelmann commented Jul 16, 2023

XVilka commented Jul 17, 2023

imbillow Jul 20, 2023 •

edited

Loading

imbillow Jul 20, 2023

bkoppelmann Jul 23, 2023

imbillow Jul 20, 2023

bkoppelmann Jul 23, 2023

imbillow Jul 24, 2023

XVilka commented Jul 20, 2023 •

edited

Loading

bkoppelmann commented Jul 23, 2023

XVilka commented Jul 23, 2023

bkoppelmann commented Jul 23, 2023

XVilka commented Jul 25, 2023

XVilka commented Jul 25, 2023

kabeor commented Jul 26, 2023

Fixing TriCore disasm instructions #2088

Fixing TriCore disasm instructions #2088

Conversation

bkoppelmann commented Jul 16, 2023

XVilka commented Jul 17, 2023

imbillow Jul 20, 2023 • edited Loading

Choose a reason for hiding this comment

imbillow Jul 20, 2023

Choose a reason for hiding this comment

bkoppelmann Jul 23, 2023

Choose a reason for hiding this comment

imbillow Jul 20, 2023

Choose a reason for hiding this comment

d, s2[, s1[, n]]

a

bkoppelmann Jul 23, 2023

Choose a reason for hiding this comment

imbillow Jul 24, 2023

Choose a reason for hiding this comment

XVilka commented Jul 20, 2023 • edited Loading

bkoppelmann commented Jul 23, 2023

XVilka commented Jul 23, 2023

bkoppelmann commented Jul 23, 2023

XVilka commented Jul 25, 2023

XVilka commented Jul 25, 2023

kabeor commented Jul 26, 2023

imbillow Jul 20, 2023 •

edited

Loading

`d, s2[, s1[, n]]`

`a`

XVilka commented Jul 20, 2023 •

edited

Loading