[PATCH] AArch64: Fix disassembly for sys aliases with undefined behaviour when Rt != 31
Alice Carlotti
alice.carlotti@arm.com
Mon Mar 16 15:20:54 GMT 2026
On Mon, Feb 23, 2026 at 03:47:35PM +0000, Muhammad Kamran wrote:
> From: Muhammad Kamran <Muhammad.Kamran@arm.com>
>
> The patch fixes disassembling of certain aliases in the sys encoding space that expect Rt = 31 by disassembling them to sys mnemonic rather than the alias if Rt != 31, with a comment.
>
> The patch also adds a test for aliases with Rt = 31, and Rt != 31 and their corresponding disassemblies.
Apologies for the slow review - I spent too long thinking about the more
complicated part of the change instead of giving earlier feedback on the simple
bug fix part of the patch. See more detailed comments below.
> ---
> gas/testsuite/gas/aarch64/sys-rt-alias.d | 15 +++++
> gas/testsuite/gas/aarch64/sys-rt-alias.s | 11 +++
> opcodes/aarch64-dis.c | 5 +-
> opcodes/aarch64-opc.c | 86 +++++++++++++++++++++---
> 4 files changed, 107 insertions(+), 10 deletions(-)
> create mode 100644 gas/testsuite/gas/aarch64/sys-rt-alias.d
> create mode 100644 gas/testsuite/gas/aarch64/sys-rt-alias.s
>
> diff --git a/gas/testsuite/gas/aarch64/sys-rt-alias.d b/gas/testsuite/gas/aarch64/sys-rt-alias.d
> new file mode 100644
> index 00000000000..70576896c7b
> --- /dev/null
> +++ b/gas/testsuite/gas/aarch64/sys-rt-alias.d
> @@ -0,0 +1,15 @@
> +#objdump: -dr
> +
> +.*: file format .*
> +
> +Disassembly of section \.text:
> +
> +0+ <.*>:
> +.*: d50c879f tlbi alle1
> +.*: d50c8780 sys #4, C8, C7, #4, x0 // unpredictable encoding \(Rt!=31\) for TLBI alle1
> +.*: d50ca79f plbi alle1
> +.*: d50ca780 sys #4, C10, C7, #4, x0 // unpredictable encoding \(Rt!=31\) for PLBI alle1
> +.*: d50c709f mlbi alle1
> +.*: d50c7080 sys #4, C7, C0, #4, x0 // unpredictable encoding \(Rt!=31\) for MLBI alle1
> +.*: d508751f ic iallu
> +.*: d5087500 sys #0, C7, C5, #0, x0 // unpredictable encoding \(Rt!=31\) for IC iallu
> \ No newline at end of file
> diff --git a/gas/testsuite/gas/aarch64/sys-rt-alias.s b/gas/testsuite/gas/aarch64/sys-rt-alias.s
> new file mode 100644
> index 00000000000..e97eaac2e1b
> --- /dev/null
> +++ b/gas/testsuite/gas/aarch64/sys-rt-alias.s
> @@ -0,0 +1,11 @@
> +// sys-rt-alias.s Test file for AArch64 instructions where Rt !=31 is undefined behaviour.
> +
> + .text
> + sys #4, c8, c7, #4 // TLBI ALLE1 with Rt=31
> + sys #4, c8, c7, #4, x0 // TLBI ALLE1 with Rt!=31
> + sys #4, c10, c7, #4 // PLBI ALLE1 with Rt=31
> + sys #4, c10, c7, #4, x0 // PLBI ALLE1 with Rt!=31
> + sys #4, c7, c0, #4 // MLBI ALLE1 with Rt=31
> + sys #4, c7, c0, #4, x0 // MLBI ALLE1 with Rt!=31
> + sys #0, c7, c5, #0 // IC IALLU with Rt=31
> + sys #0, c7, c5, #0, x0 // IC IALLU with Rt!=31
> diff --git a/opcodes/aarch64-dis.c b/opcodes/aarch64-dis.c
> index 8cc2eb3f3c2..f4bd2d933b9 100644
> --- a/opcodes/aarch64-dis.c
> +++ b/opcodes/aarch64-dis.c
> @@ -334,8 +334,11 @@ aarch64_ext_regrt_sysins (const aarch64_operand *self, aarch64_opnd_info *info,
> if (aarch64_sys_ins_reg_tlbid_xt (inst->operands[0].sysins_op)
> && info->reg.regno != 31)
> info->present = true;
> + else if (aarch64_sys_ins_reg_has_xt (inst->operands[0].sysins_op))
> + info->present = true;
> else
> - info->present = aarch64_sys_ins_reg_has_xt (inst->operands[0].sysins_op);
> + /* Force alias rejection when Rt != XZR for ops that do not take Xt. */
> + info->present = info->reg.regno != 31;
>
> return true;
> }
This whole hunk could be reduced to just:
return (info->reg.regno != 31
|| aarch64_sys_ins_reg_has_xt (inst->operands[0].sysins_op));
}
This would be good as a standalone fix, and I'd prefer to have this as a
separate patch (with appropriate tests).
> diff --git a/opcodes/aarch64-opc.c b/opcodes/aarch64-opc.c
> index a50b8fd0fb8..084a0010334 100644
> --- a/opcodes/aarch64-opc.c
> +++ b/opcodes/aarch64-opc.c
> @@ -4141,6 +4141,80 @@ print_sme_za_list (char *buf, size_t size, int mask,
> The function serves both the disassembler and the assembler diagnostics
> issuer, which is the reason why it lives in this file. */
>
> +static bool
> +aarch64_set_sys_alias_rt_comment (const aarch64_opcode *opcode,
> + const aarch64_opnd_info *opnds,
> + const aarch64_opnd_info *opnd,
> + char *comment, size_t comment_size)
> +{
> +
> + if (opnd->reg.regno == get_optional_operand_default_value (opcode))
> + return false;
> +
> + if (opnd->type == AARCH64_OPND_Rt_IN_SYS_ALIASES)
> + {
> + /* Avoid printing an invalid additional value for Rt in SYS aliases
> + such as BRB, provide a helpful comment instead. */
> + snprintf (comment, comment_size, "unpredictable encoding (Rt!=31): #%u",
> + opnd->reg.regno);
> + return true;
> + }
> +
> + if (opnd->type == AARCH64_OPND_Rt
> + && opcode->name
> + && strcmp (opcode->name, "sys") == 0
> + && opnd->reg.regno != 31)
> + {
> + unsigned op1 = opnds[0].imm.value & 0x7;
> + unsigned crn = opnds[1].imm.value & 0xf;
> + unsigned crm = opnds[2].imm.value & 0xf;
> + unsigned op2 = opnds[3].imm.value & 0x7;
> + aarch64_insn value = ((((1u << 19)
> + | (op1 << 16)
> + | (crn << 12)
> + | (crm << 8)
> + | (op2 << 5)) >> 5));
> + struct sysins_table
> + {
> + const char *class_name;
> + const aarch64_sys_ins_reg *table;
> + };
> + static const struct sysins_table tables[] = {
> + { "IC", aarch64_sys_regs_ic },
> + { "TLBI", aarch64_sys_regs_tlbi },
> + { "PLBI", aarch64_sys_regs_plbi },
> + { "MLBI", aarch64_sys_regs_mlbi },
> + { NULL, NULL }
> + };
> + const aarch64_sys_ins_reg *sysins_op = NULL;
> + const char *class_name = NULL;
> + int i;
> +
> + for (i = 0; tables[i].class_name != NULL; ++i)
> + {
> + int j;
> + for (j = 0; tables[i].table[j].name != NULL; ++j)
> + if (tables[i].table[j].value == value)
> + {
> + sysins_op = tables[i].table + j;
> + class_name = tables[i].class_name;
> + break;
> + }
> + if (sysins_op)
> + break;
> + }
I don't think we want to iterate over every entry in every system instruction
hash table, and this won't give the correct result for sysp aliases either.
This whole area of code is already quite complicated and could do with some
more substantial rework. While there's certainly some merit to getting a more
helpful comment for the disassembly, I suggest that we park this part of the
change for now. We can implement something like this in the future once we
have an improved system instruction framework that would allow us to handle
this more consistently and easily.
Thanks,
Alice
> +
> + if (sysins_op
> + && !aarch64_sys_ins_reg_has_xt (sysins_op)
> + && !aarch64_sys_ins_reg_tlbid_xt (sysins_op))
> + snprintf (comment, comment_size,
> + "unpredictable encoding (Rt!=31) for %s %s",
> + class_name, sysins_op->name);
> + }
> +
> + return false;
> +}
> +
> void
> aarch64_print_operand (char *buf, size_t size, bfd_vma pc,
> const aarch64_opcode *opcode,
> @@ -4193,15 +4267,9 @@ aarch64_print_operand (char *buf, size_t size, bfd_vma pc,
> if (!opnd->present)
> break;
> }
> - else if ((opnd->type == AARCH64_OPND_Rt_IN_SYS_ALIASES)
> - && (opnd->reg.regno
> - != get_optional_operand_default_value (opcode)))
> - {
> - /* Avoid printing an invalid additional value for Rt in SYS aliases such as
> - BRB, provide a helpful comment instead */
> - snprintf (comment, comment_size, "unpredictable encoding (Rt!=31): #%u", opnd->reg.regno);
> - break;
> - }
> + else if (aarch64_set_sys_alias_rt_comment (opcode, opnds, opnd,
> + comment, comment_size))
> + break;
> /* Omit the operand, e.g. RET. */
> else if (optional_operand_p (opcode, idx)
> && (opnd->reg.regno
> --
> 2.43.0
>
More information about the Binutils
mailing list