From 6837a663c55602490ed095e5891e0c4deff4b9db Mon Sep 17 00:00:00 2001 From: Andrew Burgess Date: Thu, 16 Jun 2022 13:46:41 +0100 Subject: [PATCH] opcodes/aarch64: split off creation of comment text in disassembler The function aarch64_print_operand (aarch64-opc.c) is responsible for converting an instruction operand into the textual representation of that operand. In some cases, a comment is included in the operand representation, though this (currently) only happens for the last operand of the instruction. In a future commit I would like to enable the new libopcodes styling for AArch64, this will allow objdump and GDB[1] to syntax highlight the disassembler output, however, having operands and comments combined in a single string like this makes such styling harder. In this commit, I propose to extend aarch64_print_operand to take a second buffer. Any comments for the instruction are written into this extra buffer. The two callers of aarch64_print_operand are then updated to pass an extra buffer, and print any resulting comment. In this commit no styling is added, that will come later. However, I have adjusted the output slightly. Before this commit some comments would be separated from the instruction operands with a tab character, while in other cases the comment was separated with two single spaces. After this commit I use a single tab character in all cases. This means a few test cases needed updated. If people would prefer me to move everyone to use the two spaces, then just let me know. Or maybe there was a good reason why we used a mix of styles, I could probably figure out a way to maintain the old output exactly if that is critical. Other than that, there should be no user visible changes after this commit. [1] GDB patches have not been merged yet, but have been posted to the GDB mailing list: https://sourceware.org/pipermail/gdb-patches/2022-June/190142.html --- gas/config/tc-aarch64.c | 12 +++++- gas/testsuite/gas/aarch64/alias.d | 26 ++++++------ gas/testsuite/gas/aarch64/b_c_1.d | 42 +++++++++---------- gas/testsuite/gas/aarch64/float-fp16.d | 12 +++--- gas/testsuite/gas/aarch64/int-insns.d | 4 +- gas/testsuite/gas/aarch64/no-aliases.d | 26 ++++++------ .../gas/aarch64/programmer-friendly.d | 2 +- include/opcode/aarch64.h | 2 +- opcodes/aarch64-dis.c | 15 ++++++- opcodes/aarch64-opc.c | 22 +++++++--- 10 files changed, 97 insertions(+), 66 deletions(-) diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c index e9f7ee9fc96..779db31828b 100644 --- a/gas/config/tc-aarch64.c +++ b/gas/config/tc-aarch64.c @@ -5358,6 +5358,7 @@ print_operands (char *buf, const aarch64_opcode *opcode, for (i = 0; i < AARCH64_MAX_OPND_NUM; ++i) { char str[128]; + char cmt[128]; /* We regard the opcode operand info more, however we also look into the inst->operands to support the disassembling of the optional @@ -5370,7 +5371,7 @@ print_operands (char *buf, const aarch64_opcode *opcode, /* Generate the operand string in STR. */ aarch64_print_operand (str, sizeof (str), 0, opcode, opnds, i, NULL, NULL, - NULL, cpu_variant); + NULL, cmt, sizeof (cmt), cpu_variant); /* Delimiter. */ if (str[0] != '\0') @@ -5378,6 +5379,15 @@ print_operands (char *buf, const aarch64_opcode *opcode, /* Append the operand string. */ strcat (buf, str); + + /* Append a comment. This works because only the last operand ever + adds a comment. If that ever changes then we'll need to be + smarter here. */ + if (cmt[0] != '\0') + { + strcat (buf, "\t// "); + strcat (buf, cmt); + } } } diff --git a/gas/testsuite/gas/aarch64/alias.d b/gas/testsuite/gas/aarch64/alias.d index 13b943c1b97..b1d1a9a37c2 100644 --- a/gas/testsuite/gas/aarch64/alias.d +++ b/gas/testsuite/gas/aarch64/alias.d @@ -29,19 +29,19 @@ Disassembly of section \.text: 54: 9ba28c20 umsubl x0, w1, w2, x3 58: 9ba2fc20 umnegl x0, w1, w2 5c: 9ba2fc20 umnegl x0, w1, w2 - 60: 1a9f0420 csinc w0, w1, wzr, eq // eq = none - 64: 1a810420 cinc w0, w1, ne // ne = any - 68: 1a810420 cinc w0, w1, ne // ne = any - 6c: 1a9f37e0 cset w0, cs // cs = hs, nlast - 70: 1a9f37e0 cset w0, cs // cs = hs, nlast - 74: da9f2020 csinv x0, x1, xzr, cs // cs = hs, nlast - 78: da812020 cinv x0, x1, cc // cc = lo, ul, last - 7c: da812020 cinv x0, x1, cc // cc = lo, ul, last - 80: da9f43e0 csetm x0, pl // pl = nfrst - 84: da9f43e0 csetm x0, pl // pl = nfrst - 88: da9eb7e0 csneg x0, xzr, x30, lt // lt = tstop - 8c: da9eb7c0 cneg x0, x30, ge // ge = tcont - 90: da9eb7c0 cneg x0, x30, ge // ge = tcont + 60: 1a9f0420 csinc w0, w1, wzr, eq // eq = none + 64: 1a810420 cinc w0, w1, ne // ne = any + 68: 1a810420 cinc w0, w1, ne // ne = any + 6c: 1a9f37e0 cset w0, cs // cs = hs, nlast + 70: 1a9f37e0 cset w0, cs // cs = hs, nlast + 74: da9f2020 csinv x0, x1, xzr, cs // cs = hs, nlast + 78: da812020 cinv x0, x1, cc // cc = lo, ul, last + 7c: da812020 cinv x0, x1, cc // cc = lo, ul, last + 80: da9f43e0 csetm x0, pl // pl = nfrst + 84: da9f43e0 csetm x0, pl // pl = nfrst + 88: da9eb7e0 csneg x0, xzr, x30, lt // lt = tstop + 8c: da9eb7c0 cneg x0, x30, ge // ge = tcont + 90: da9eb7c0 cneg x0, x30, ge // ge = tcont 94: ea020020 ands x0, x1, x2 98: ea02003f tst x1, x2 9c: ea02003f tst x1, x2 diff --git a/gas/testsuite/gas/aarch64/b_c_1.d b/gas/testsuite/gas/aarch64/b_c_1.d index 6427a3a63a5..f9359a4842c 100644 --- a/gas/testsuite/gas/aarch64/b_c_1.d +++ b/gas/testsuite/gas/aarch64/b_c_1.d @@ -31,28 +31,28 @@ Disassembly of section \.text: .*: 54.....b b\.lt 0 <\.text> // b\.tstop .*: 54.....c b\.gt 0 <\.text> .*: 54.....d b\.le 0 <\.text> -.*: 9a830041 csel x1, x2, x3, eq // eq = none -.*: 9a830041 csel x1, x2, x3, eq // eq = none -.*: 9a832041 csel x1, x2, x3, cs // cs = hs, nlast -.*: 9a832041 csel x1, x2, x3, cs // cs = hs, nlast -.*: 9a832041 csel x1, x2, x3, cs // cs = hs, nlast -.*: 9a833041 csel x1, x2, x3, cc // cc = lo, ul, last -.*: 9a833041 csel x1, x2, x3, cc // cc = lo, ul, last -.*: 9a833041 csel x1, x2, x3, cc // cc = lo, ul, last -.*: 9a833041 csel x1, x2, x3, cc // cc = lo, ul, last -.*: 9a834041 csel x1, x2, x3, mi // mi = first -.*: 9a834041 csel x1, x2, x3, mi // mi = first -.*: 9a835041 csel x1, x2, x3, pl // pl = nfrst -.*: 9a835041 csel x1, x2, x3, pl // pl = nfrst +.*: 9a830041 csel x1, x2, x3, eq // eq = none +.*: 9a830041 csel x1, x2, x3, eq // eq = none +.*: 9a832041 csel x1, x2, x3, cs // cs = hs, nlast +.*: 9a832041 csel x1, x2, x3, cs // cs = hs, nlast +.*: 9a832041 csel x1, x2, x3, cs // cs = hs, nlast +.*: 9a833041 csel x1, x2, x3, cc // cc = lo, ul, last +.*: 9a833041 csel x1, x2, x3, cc // cc = lo, ul, last +.*: 9a833041 csel x1, x2, x3, cc // cc = lo, ul, last +.*: 9a833041 csel x1, x2, x3, cc // cc = lo, ul, last +.*: 9a834041 csel x1, x2, x3, mi // mi = first +.*: 9a834041 csel x1, x2, x3, mi // mi = first +.*: 9a835041 csel x1, x2, x3, pl // pl = nfrst +.*: 9a835041 csel x1, x2, x3, pl // pl = nfrst .*: 9a836041 csel x1, x2, x3, vs .*: 9a837041 csel x1, x2, x3, vc -.*: 9a838041 csel x1, x2, x3, hi // hi = pmore -.*: 9a838041 csel x1, x2, x3, hi // hi = pmore -.*: 9a839041 csel x1, x2, x3, ls // ls = plast -.*: 9a839041 csel x1, x2, x3, ls // ls = plast -.*: 9a83a041 csel x1, x2, x3, ge // ge = tcont -.*: 9a83a041 csel x1, x2, x3, ge // ge = tcont -.*: 9a83b041 csel x1, x2, x3, lt // lt = tstop -.*: 9a83b041 csel x1, x2, x3, lt // lt = tstop +.*: 9a838041 csel x1, x2, x3, hi // hi = pmore +.*: 9a838041 csel x1, x2, x3, hi // hi = pmore +.*: 9a839041 csel x1, x2, x3, ls // ls = plast +.*: 9a839041 csel x1, x2, x3, ls // ls = plast +.*: 9a83a041 csel x1, x2, x3, ge // ge = tcont +.*: 9a83a041 csel x1, x2, x3, ge // ge = tcont +.*: 9a83b041 csel x1, x2, x3, lt // lt = tstop +.*: 9a83b041 csel x1, x2, x3, lt // lt = tstop .*: 9a83c041 csel x1, x2, x3, gt .*: 9a83d041 csel x1, x2, x3, le diff --git a/gas/testsuite/gas/aarch64/float-fp16.d b/gas/testsuite/gas/aarch64/float-fp16.d index d1d69d820f0..3d96a45a80c 100644 --- a/gas/testsuite/gas/aarch64/float-fp16.d +++ b/gas/testsuite/gas/aarch64/float-fp16.d @@ -6,12 +6,12 @@ Disassembly of section \.text: 0+ <.*>: - [0-9a-f]+: 1e200400 fccmp s0, s0, #0x0, eq // eq = none - [0-9a-f]+: 1ee00400 fccmp h0, h0, #0x0, eq // eq = none + [0-9a-f]+: 1e200400 fccmp s0, s0, #0x0, eq // eq = none + [0-9a-f]+: 1ee00400 fccmp h0, h0, #0x0, eq // eq = none [0-9a-f]+: 1e22d420 fccmp s1, s2, #0x0, le [0-9a-f]+: 1ee2d420 fccmp h1, h2, #0x0, le - [0-9a-f]+: 1e200410 fccmpe s0, s0, #0x0, eq // eq = none - [0-9a-f]+: 1ee00410 fccmpe h0, h0, #0x0, eq // eq = none + [0-9a-f]+: 1e200410 fccmpe s0, s0, #0x0, eq // eq = none + [0-9a-f]+: 1ee00410 fccmpe h0, h0, #0x0, eq // eq = none [0-9a-f]+: 1e22d430 fccmpe s1, s2, #0x0, le [0-9a-f]+: 1ee2d430 fccmpe h1, h2, #0x0, le [0-9a-f]+: 1e202000 fcmp s0, s0 @@ -26,8 +26,8 @@ Disassembly of section \.text: [0-9a-f]+: 1ee02008 fcmp h0, #0\.0 [0-9a-f]+: 1e202018 fcmpe s0, #0\.0 [0-9a-f]+: 1ee02018 fcmpe h0, #0\.0 - [0-9a-f]+: 1e210c00 fcsel s0, s0, s1, eq // eq = none - [0-9a-f]+: 1ee10c00 fcsel h0, h0, h1, eq // eq = none + [0-9a-f]+: 1e210c00 fcsel s0, s0, s1, eq // eq = none + [0-9a-f]+: 1ee10c00 fcsel h0, h0, h1, eq // eq = none [0-9a-f]+: 9ee60000 fmov x0, h0 [0-9a-f]+: 1ee60000 fmov w0, h0 [0-9a-f]+: 9ee70001 fmov h1, x0 diff --git a/gas/testsuite/gas/aarch64/int-insns.d b/gas/testsuite/gas/aarch64/int-insns.d index 419b8a6e841..76f937c4fbf 100644 --- a/gas/testsuite/gas/aarch64/int-insns.d +++ b/gas/testsuite/gas/aarch64/int-insns.d @@ -13,8 +13,8 @@ Disassembly of section .text: 10: 93c3fc41 extr x1, x2, x3, #63 14: 93c30041 extr x1, x2, x3, #0 18: 13837c41 extr w1, w2, w3, #31 - 1c: 9a9f17e1 cset x1, eq // eq = none - 20: da9f13e1 csetm x1, eq // eq = none + 1c: 9a9f17e1 cset x1, eq // eq = none + 20: da9f13e1 csetm x1, eq // eq = none 24: 71000021 subs w1, w1, #0x0 28: 7100003f cmp w1, #0x0 2c: 4b0203e1 neg w1, w2 diff --git a/gas/testsuite/gas/aarch64/no-aliases.d b/gas/testsuite/gas/aarch64/no-aliases.d index dae0b712dab..3cee28df657 100644 --- a/gas/testsuite/gas/aarch64/no-aliases.d +++ b/gas/testsuite/gas/aarch64/no-aliases.d @@ -30,19 +30,19 @@ Disassembly of section \.text: 54: 9ba28c20 umsubl x0, w1, w2, x3 58: 9ba2fc20 umsubl x0, w1, w2, xzr 5c: 9ba2fc20 umsubl x0, w1, w2, xzr - 60: 1a9f0420 csinc w0, w1, wzr, eq // eq = none - 64: 1a810420 csinc w0, w1, w1, eq // eq = none - 68: 1a810420 csinc w0, w1, w1, eq // eq = none - 6c: 1a9f37e0 csinc w0, wzr, wzr, cc // cc = lo, ul, last - 70: 1a9f37e0 csinc w0, wzr, wzr, cc // cc = lo, ul, last - 74: da9f2020 csinv x0, x1, xzr, cs // cs = hs, nlast - 78: da812020 csinv x0, x1, x1, cs // cs = hs, nlast - 7c: da812020 csinv x0, x1, x1, cs // cs = hs, nlast - 80: da9f43e0 csinv x0, xzr, xzr, mi // mi = first - 84: da9f43e0 csinv x0, xzr, xzr, mi // mi = first - 88: da9eb7e0 csneg x0, xzr, x30, lt // lt = tstop - 8c: da9eb7c0 csneg x0, x30, x30, lt // lt = tstop - 90: da9eb7c0 csneg x0, x30, x30, lt // lt = tstop + 60: 1a9f0420 csinc w0, w1, wzr, eq // eq = none + 64: 1a810420 csinc w0, w1, w1, eq // eq = none + 68: 1a810420 csinc w0, w1, w1, eq // eq = none + 6c: 1a9f37e0 csinc w0, wzr, wzr, cc // cc = lo, ul, last + 70: 1a9f37e0 csinc w0, wzr, wzr, cc // cc = lo, ul, last + 74: da9f2020 csinv x0, x1, xzr, cs // cs = hs, nlast + 78: da812020 csinv x0, x1, x1, cs // cs = hs, nlast + 7c: da812020 csinv x0, x1, x1, cs // cs = hs, nlast + 80: da9f43e0 csinv x0, xzr, xzr, mi // mi = first + 84: da9f43e0 csinv x0, xzr, xzr, mi // mi = first + 88: da9eb7e0 csneg x0, xzr, x30, lt // lt = tstop + 8c: da9eb7c0 csneg x0, x30, x30, lt // lt = tstop + 90: da9eb7c0 csneg x0, x30, x30, lt // lt = tstop 94: ea020020 ands x0, x1, x2 98: ea02003f ands xzr, x1, x2 9c: ea02003f ands xzr, x1, x2 diff --git a/gas/testsuite/gas/aarch64/programmer-friendly.d b/gas/testsuite/gas/aarch64/programmer-friendly.d index 668a13506fb..8fa6aa70dbf 100644 --- a/gas/testsuite/gas/aarch64/programmer-friendly.d +++ b/gas/testsuite/gas/aarch64/programmer-friendly.d @@ -10,7 +10,7 @@ Disassembly of section \.text: 4: 98000241 ldrsw x1, 4c <\.text\+0x4c> 8: 98000007 ldrsw x7, 0 <\.text> 8: R_AARCH64_LD_PREL_LO19 \.data\+0x4 - c: fa42a02a ccmp x1, x2, #0xa, ge // ge = tcont + c: fa42a02a ccmp x1, x2, #0xa, ge // ge = tcont 10: 53001eaf uxtb w15, w21 14: 53003f67 uxth w7, w27 18: 2a1f03e8 mov w8, wzr diff --git a/include/opcode/aarch64.h b/include/opcode/aarch64.h index 8fe27900cba..186ba9a4800 100644 --- a/include/opcode/aarch64.h +++ b/include/opcode/aarch64.h @@ -1371,7 +1371,7 @@ aarch64_get_opcode (enum aarch64_op); extern void aarch64_print_operand (char *, size_t, bfd_vma, const aarch64_opcode *, const aarch64_opnd_info *, int, int *, bfd_vma *, - char **, + char **, char *, size_t, aarch64_feature_set features); /* Miscellaneous interface. */ diff --git a/opcodes/aarch64-dis.c b/opcodes/aarch64-dis.c index bd2068b0f32..acaad28fdff 100644 --- a/opcodes/aarch64-dis.c +++ b/opcodes/aarch64-dis.c @@ -3287,6 +3287,8 @@ print_operands (bfd_vma pc, const aarch64_opcode *opcode, for (i = 0, num_printed = 0; i < AARCH64_MAX_OPND_NUM; ++i) { char str[128]; + char cmt[128]; + /* We regard the opcode operand info more, however we also look into the inst->operands to support the disassembling of the optional operand. @@ -3298,7 +3300,8 @@ print_operands (bfd_vma pc, const aarch64_opcode *opcode, /* Generate the operand string in STR. */ aarch64_print_operand (str, sizeof (str), pc, opcode, opnds, i, &pcrel_p, - &info->target, ¬es, arch_variant); + &info->target, ¬es, cmt, sizeof (cmt), + arch_variant); /* Print the delimiter (taking account of omitted operand(s)). */ if (str[0] != '\0') @@ -3309,7 +3312,15 @@ print_operands (bfd_vma pc, const aarch64_opcode *opcode, if (pcrel_p) (*info->print_address_func) (info->target, info); else - (*info->fprintf_func) (info->stream, "%s", str); + { + (*info->fprintf_func) (info->stream, "%s", str); + + /* Print the comment. This works because only the last operand + ever adds a comment. If that ever changes then we'll need to + be smarter here. */ + if (cmt[0] != '\0') + (*info->fprintf_func) (info->stream, "\t// %s", cmt); + } } if (notes && !no_notes) diff --git a/opcodes/aarch64-opc.c b/opcodes/aarch64-opc.c index 37f6dae07ad..1c93f836020 100644 --- a/opcodes/aarch64-opc.c +++ b/opcodes/aarch64-opc.c @@ -3229,6 +3229,7 @@ aarch64_print_operand (char *buf, size_t size, bfd_vma pc, const aarch64_opcode *opcode, const aarch64_opnd_info *opnds, int idx, int *pcrel_p, bfd_vma *address, char** notes, + char *comment, size_t comment_size, aarch64_feature_set features) { unsigned int i, num_conds; @@ -3237,6 +3238,14 @@ aarch64_print_operand (char *buf, size_t size, bfd_vma pc, enum aarch64_modifier_kind kind; uint64_t addr, enum_value; + if (comment != NULL) + { + assert (comment_size > 0); + comment[0] = '\0'; + } + else + assert (comment_size == 0); + buf[0] = '\0'; if (pcrel_p) *pcrel_p = 0; @@ -3572,12 +3581,13 @@ aarch64_print_operand (char *buf, size_t size, bfd_vma pc, case 4: /* e.g. MOV Wd, #. */ { int imm32 = opnd->imm.value; - snprintf (buf, size, "#0x%-20x\t// #%d", imm32, imm32); + snprintf (buf, size, "#0x%-20x", imm32); + snprintf (comment, comment_size, "#%d", imm32); } break; case 8: /* e.g. MOV Xd, #. */ - snprintf (buf, size, "#0x%-20" PRIx64 "\t// #%" PRIi64, - opnd->imm.value, opnd->imm.value); + snprintf (buf, size, "#0x%-20" PRIx64, opnd->imm.value); + snprintf (comment, comment_size, "#%" PRIi64, opnd->imm.value); break; default: snprintf (buf, size, ""); @@ -3675,12 +3685,12 @@ aarch64_print_operand (char *buf, size_t size, bfd_vma pc, num_conds = ARRAY_SIZE (opnd->cond->names); for (i = 1; i < num_conds && opnd->cond->names[i]; ++i) { - size_t len = strlen (buf); + size_t len = comment != NULL ? strlen (comment) : 0; if (i == 1) - snprintf (buf + len, size - len, " // %s = %s", + snprintf (comment + len, comment_size - len, "%s = %s", opnd->cond->names[0], opnd->cond->names[i]); else - snprintf (buf + len, size - len, ", %s", + snprintf (comment + len, comment_size - len, ", %s", opnd->cond->names[i]); } break; -- 2.30.2