[Bug binutils/21446] New: Incorrect disassembly for msr to dbgdtrtx_el0 in AArch64

[chris_s_jones at yahoo dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=21446

Bug ID: 21446
Summary: Incorrect disassembly for msr to dbgdtrtx_el0 in
AArch64
Product: binutils
Version: 2.28
Status: UNCONFIRMED
Severity: normal
Priority: P2
Component: binutils
Assignee: unassigned at sourceware dot org
Reporter: chris_s_jones at yahoo dot com
Target Milestone: ---

Created attachment 10032
--> https://sourceware.org/bugzilla/attachment.cgi?id=10032&action=edit
Test case

The AArch64 system registers dbgdtrrx_el0 and dbgdtrtx_el0 use the same
encoding, but the former is only used for MRS instructions and the latter for
MSR.

Assembling the following code:
.text
msr dbgdtrtx_el0, x3
mrs x3, dbgdtrrx_el0

And then disassembling with objdump gives the incorrect register name for the
MSR

0000000000000000 <.text>:
0: d5130503 msr dbgdtrrx_el0, x3
4: d5330503 mrs x3, dbgdtrrx_el0

The resulting binary is, of course, fine, but the disassembly output is off for
the MSR.

[chris_s_jones at yahoo dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=21446

--- Comment #1 from Chris <chris_s_jones at yahoo dot com> ---
Created attachment 10033
--> https://sourceware.org/bugzilla/attachment.cgi?id=10033&action=edit
Pre-assembled object file

[tnfchris at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=21446

Tamar Christina <tnfchris at sourceware dot org> changed:

What |Removed |Added
----------------------------------------------------------------------------
Assignee|unassigned at sourceware dot org |tnfchris at sourceware dot org

[tnfchris at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=21446

Tamar Christina <tnfchris at sourceware dot org> changed:

What |Removed |Added
----------------------------------------------------------------------------
Target| |aarch64-*
Target Milestone|--- |2.31

[tnfchris at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=21446

Tamar Christina <tnfchris at sourceware dot org> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |NEW
Last reconfirmed| |2018-04-18
CC| |tnfchris at sourceware dot org
Ever confirmed|0 |1

[tnfchris at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=21446

Tamar Christina <tnfchris at sourceware dot org> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=21446

--- Comment #3 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tamar Christina
<***@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=7d02540ab73206249779ced77a6abe0be156442e

commit 7d02540ab73206249779ced77a6abe0be156442e
Author: Tamar Christina <***@arm.com>
Date: Tue May 15 16:34:54 2018 +0100

Allow non-fatal errors to be emitted and for disassembly notes be placed on
AArch64

This patch adds a new platform option "notes" that can be used to indicate
if
disassembly notes should be placed in the disassembly as comments.

These notes can contain information about a failing constraint such as
reading
from a write-only register. The disassembly will not be blocked because of
this
but -M notes will emit a comment saying that the operation is not allowed.

For assembly this patch adds a new non-fatal status for errors. This is
essentially a warning. The reason for not creating an actual warning type
is
that this causes the interaction between the ordering of warnings and
errors to
be problematic. Currently the error buffer is almost always filled because
of
the way operands are matched during assembly. An earlier template may have
put
an error there that would only be displayed if no other template matches or
generates a higher priority error. But by definition a warning is lower
priority than a warning, so the error (which is incorrect if another
template
matched) will supersede the warning. By treating warnings as errors and
only
later relaxing the severity this relationship keeps working and the
existing
reporting infrastructure can be re-used.

binutils/

PR binutils/21446
* doc/binutils.texi (-M): Document AArch64 options.
* NEWS: Document notes and warnings.

gas/

PR binutils/21446
* config/tc-aarch64.c (print_operands): Indicate no notes.
(output_operand_error_record): Support non-fatal errors.
(output_operand_error_report, warn_unpredictable_ldst, md_assemble):
Likewise.

include/

PR binutils/21446
* opcode/aarch64.h (aarch64_operand_error): Add non_fatal.
(aarch64_print_operand): Support notes.

opcodes/

PR binutils/21446
* aarch64-dis.c (no_notes: New.
(parse_aarch64_dis_option): Support notes.
(aarch64_decode_insn, print_operands): Likewise.
(print_aarch64_disassembler_options): Document notes.
* aarch64-opc.c (aarch64_print_operand): Support notes.

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=21446

--- Comment #2 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tamar Christina
<***@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=561a72d4ddf825ffaf8e88551e9bd6707cd6c59f

commit 561a72d4ddf825ffaf8e88551e9bd6707cd6c59f
Author: Tamar Christina <***@arm.com>
Date: Tue May 15 16:11:42 2018 +0100

Modify AArch64 Assembly and disassembly functions to be able to fail and
report why.

This patch if the first patch in a series to add the ability to add
constraints
to system registers that an instruction must adhere to in order for the
register
to be usable with that instruction.

These constraints can also be used to disambiguate between registers with
the
same encoding during disassembly.

This patch adds a new flags entry in the sysreg structures and ensures it
is
filled in and read out during assembly/disassembly. It also adds the
ability for
the assemble and disassemble functions to be able to gracefully fail and
re-use
the existing error reporting infrastructure.

The return type of these functions are changed to a boolean to denote
success or
failure and the error structure is passed around to them. This requires
aarch64-gen changes so a lot of the changes here are just mechanical.

gas/

PR binutils/21446
* config/tc-aarch64.c (parse_sys_reg): Return register flags.
(parse_operands): Fill in register flags.

gdb/

PR binutils/21446
* aarch64-tdep.c (aarch64_analyze_prologue,
aarch64_software_single_step, aarch64_displaced_step_copy_insn):
Indicate not interested in errors.

include/

PR binutils/21446
* opcode/aarch64.h (aarch64_opnd_info): Change sysreg to struct.
(aarch64_decode_insn): Accept error struct.

opcodes/

PR binutils/21446
* aarch64-asm.h (aarch64_insert_operand, aarch64_##x): Return boolean
and take error struct.
* aarch64-asm.c (aarch64_ext_regno, aarch64_ins_reglane,
aarch64_ins_reglist, aarch64_ins_ldst_reglist,
aarch64_ins_ldst_reglist_r, aarch64_ins_ldst_elemlist,
aarch64_ins_advsimd_imm_shift, aarch64_ins_imm, aarch64_ins_imm_half,
aarch64_ins_advsimd_imm_modified, aarch64_ins_fpimm,
aarch64_ins_imm_rotate1, aarch64_ins_imm_rotate2, aarch64_ins_fbits,
aarch64_ins_aimm, aarch64_ins_limm_1, aarch64_ins_limm,
aarch64_ins_inv_limm, aarch64_ins_ft, aarch64_ins_addr_simple,
aarch64_ins_addr_regoff, aarch64_ins_addr_offset,
aarch64_ins_addr_simm,
aarch64_ins_addr_simm10, aarch64_ins_addr_uimm12,
aarch64_ins_simd_addr_post, aarch64_ins_cond, aarch64_ins_sysreg,
aarch64_ins_pstatefield, aarch64_ins_sysins_op, aarch64_ins_barrier,
aarch64_ins_prfop, aarch64_ins_hint, aarch64_ins_reg_extended,
aarch64_ins_reg_shifted, aarch64_ins_sve_addr_ri_s4xvl,
aarch64_ins_sve_addr_ri_s6xvl, aarch64_ins_sve_addr_ri_s9xvl,
aarch64_ins_sve_addr_ri_s4, aarch64_ins_sve_addr_ri_u6,
aarch64_ins_sve_addr_rr_lsl, aarch64_ins_sve_addr_rz_xtw,
aarch64_ins_sve_addr_zi_u5, aarch64_ext_sve_addr_zz,
aarch64_ins_sve_addr_zz_lsl, aarch64_ins_sve_addr_zz_sxtw,
aarch64_ins_sve_addr_zz_uxtw, aarch64_ins_sve_aimm,
aarch64_ins_sve_asimm, aarch64_ins_sve_index, aarch64_ins_sve_limm_mov,
aarch64_ins_sve_quad_index, aarch64_ins_sve_reglist,
aarch64_ins_sve_scale, aarch64_ins_sve_shlimm, aarch64_ins_sve_shrimm,
aarch64_ins_sve_float_half_one, aarch64_ins_sve_float_half_two,
aarch64_ins_sve_float_zero_one, aarch64_opcode_encode): Likewise.
* aarch64-dis.h (aarch64_extract_operand, aarch64_##x): Likewise.
* aarch64-dis.c (aarch64_ext_regno, aarch64_ext_reglane,
aarch64_ext_reglist, aarch64_ext_ldst_reglist,
aarch64_ext_ldst_reglist_r, aarch64_ext_ldst_elemlist,
aarch64_ext_advsimd_imm_shift, aarch64_ext_imm, aarch64_ext_imm_half,
aarch64_ext_advsimd_imm_modified, aarch64_ext_fpimm,
aarch64_ext_imm_rotate1, aarch64_ext_imm_rotate2, aarch64_ext_fbits,
aarch64_ext_aimm, aarch64_ext_limm_1, aarch64_ext_limm, decode_limm,
aarch64_ext_inv_limm, aarch64_ext_ft, aarch64_ext_addr_simple,
aarch64_ext_addr_regoff, aarch64_ext_addr_offset,
aarch64_ext_addr_simm,
aarch64_ext_addr_simm10, aarch64_ext_addr_uimm12,
aarch64_ext_simd_addr_post, aarch64_ext_cond, aarch64_ext_sysreg,
aarch64_ext_pstatefield, aarch64_ext_sysins_op, aarch64_ext_barrier,
aarch64_ext_prfop, aarch64_ext_hint, aarch64_ext_reg_extended,
aarch64_ext_reg_shifted, aarch64_ext_sve_addr_ri_s4xvl,
aarch64_ext_sve_addr_ri_s6xvl, aarch64_ext_sve_addr_ri_s9xvl,
aarch64_ext_sve_addr_ri_s4, aarch64_ext_sve_addr_ri_u6,
aarch64_ext_sve_addr_rr_lsl, aarch64_ext_sve_addr_rz_xtw,
aarch64_ext_sve_addr_zi_u5, aarch64_ext_sve_addr_zz,
aarch64_ext_sve_addr_zz_lsl, aarch64_ext_sve_addr_zz_sxtw,
aarch64_ext_sve_addr_zz_uxtw, aarch64_ext_sve_aimm,
aarch64_ext_sve_asimm, aarch64_ext_sve_index, aarch64_ext_sve_limm_mov,
aarch64_ext_sve_quad_index, aarch64_ext_sve_reglist,
aarch64_ext_sve_scale, aarch64_ext_sve_shlimm, aarch64_ext_sve_shrimm,
aarch64_ext_sve_float_half_one, aarch64_ext_sve_float_half_two,
aarch64_ext_sve_float_zero_one, aarch64_opcode_decode): Likewise.
(determine_disassembling_preference, aarch64_decode_insn,
print_insn_aarch64_word, print_insn_data): Take errors struct.
(print_insn_aarch64): Use errors.
* aarch64-asm-2.c: Regenerate.
* aarch64-dis-2.c: Regenerate.
* aarch64-gen.c (print_operand_inserter): Use errors and change type to
boolean in aarch64_insert_operan.
(print_operand_extractor): Likewise.
* aarch64-opc.c (aarch64_print_operand): Use sysreg struct.

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=21446

--- Comment #4 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tamar Christina
<***@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=f9830ec1655e7cc2aa88c9c34a20503978d9dc88

commit f9830ec1655e7cc2aa88c9c34a20503978d9dc88
Author: Tamar Christina <***@arm.com>
Date: Tue May 15 16:37:20 2018 +0100

Implement Read/Write constraints on system registers on AArch64

This patch adds constraints for read and write only system registers with
the
msr and mrs instructions. The code will treat having both flags set and
none
set as the same. These flags add constraints that must be matched up. e.g.
a
system register with a READ only flag set, can only be used with mrs. If
The
constraint fails a warning is emitted.

Examples of the warnings generated:

test.s: Assembler messages:
test.s:5: Warning: specified register cannot be written to at operand 1 --
`msr dbgdtrrx_el0,x3'
test.s:7: Warning: specified register cannot be read from at operand 2 --
`mrs x3,dbgdtrtx_el0'
test.s:8: Warning: specified register cannot be written to at operand 1 --
`msr midr_el1,x3'

and disassembly notes:

0000000000000000 <main>:
0: d5130503 msr dbgdtrtx_el0, x3
4: d5130503 msr dbgdtrtx_el0, x3
8: d5330503 mrs x3, dbgdtrrx_el0
c: d5330503 mrs x3, dbgdtrrx_el0
10: d5180003 msr midr_el1, x3 ; note: writing to a
read-only register.

Note that because dbgdtrrx_el0 and dbgdtrtx_el0 have the same encoding,
during
disassembly the constraints are use to disambiguate between the two. An
exact
constraint match is always prefered over partial ones if available.

As always the warnings can be suppressed with -w and also be made errors
using
warnings as errors.

binutils/

PR binutils/21446
* doc/binutils.texi (-M): Document AArch64 options.

gas/

PR binutils/21446
* testsuite/gas/aarch64/illegal-sysreg-2.s: Fix pmbidr_el1 test.
* testsuite/gas/aarch64/illegal-sysreg-2.l: Likewise.
* testsuite/gas/aarch64/illegal-sysreg-2.d: Likewise.
* testsuite/gas/aarch64/sysreg-diagnostic.s: New.
* testsuite/gas/aarch64/sysreg-diagnostic.l: New.
* testsuite/gas/aarch64/sysreg-diagnostic.d: New.

include/

PR binutils/21446
* opcode/aarch64.h (F_SYS_READ, F_SYS_WRITE): New.

opcodes/

PR binutils/21446
* aarch64-asm.c (opintl.h): Include.
(aarch64_ins_sysreg): Enforce read/write constraints.
* aarch64-dis.c (aarch64_ext_sysreg): Likewise.
* aarch64-opc.h (F_DEPRECATED, F_ARCHEXT, F_HASXT): Moved here.
(F_REG_READ, F_REG_WRITE): New.
* aarch64-opc.c (aarch64_print_operand): Generate notes for
AARCH64_OPND_SYSREG.
(F_DEPRECATED, F_ARCHEXT, F_HASXT): Move to aarch64-opc.h.
(aarch64_sys_regs): Add constraints to currentel, midr_el1, ctr_el0,
mpidr_el1, revidr_el1, aidr_el1, dczid_el0, id_dfr0_el1, id_pfr0_el1,
id_pfr1_el1, id_afr0_el1, id_mmfr0_el1, id_mmfr1_el1, id_mmfr2_el1,
id_mmfr3_el1, id_mmfr4_el1, id_isar0_el1, id_isar1_el1, id_isar2_el1,
id_isar3_el1, id_isar4_el1, id_isar5_el1, mvfr0_el1, mvfr1_el1,
mvfr2_el1, ccsidr_el1, id_aa64pfr0_el1, id_aa64pfr1_el1,
id_aa64dfr0_el1, id_aa64dfr1_el1, id_aa64isar0_el1, id_aa64isar1_el1,
id_aa64mmfr0_el1, id_aa64mmfr1_el1, id_aa64mmfr2_el1, id_aa64afr0_el1,
id_aa64afr0_el1, id_aa64afr1_el1, id_aa64zfr0_el1, clidr_el1,
csselr_el1, vsesr_el2, erridr_el1, erxfr_el1, rvbar_el1, rvbar_el2,
rvbar_el3, isr_el1, tpidrro_el0, cntfrq_el0, cntpct_el0, cntvct_el0,
mdccsr_el0, dbgdtrrx_el0, dbgdtrtx_el0, osdtrrx_el1, osdtrtx_el1,
mdrar_el1, oslar_el1, oslsr_el1, dbgauthstatus_el1, pmbidr_el1,
pmsidr_el1, pmswinc_el0, pmceid0_el0, pmceid1_el0.
* aarch64-tbl.h (aarch64_opcode_table): Add constraints to
msr (F_SYS_WRITE), mrs (F_SYS_READ).

[tnfchris at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=21446

Tamar Christina <tnfchris at sourceware dot org> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Resolution|--- |FIXED

--- Comment #5 from Tamar Christina <tnfchris at sourceware dot org> ---
Issue is now fixed in master.