diff mbox series

[v1,1/1] opcodes: microblaze: Fix bit masking bug

Message ID 20231019113740.2071556-1-neal.frager@amd.com
State New
Headers show
Series [v1,1/1] opcodes: microblaze: Fix bit masking bug | expand

Commit Message

Neal Frager Oct. 19, 2023, 11:37 a.m. UTC 
There is currently a bug in the bit masking for the barrel shift
instructions because the bit mask is not including all of the
register bits which must be zero.  With this patch, the disassembler
can be sure that the 32-bit value is indeed a barrel shift instruction
and not a data value in memory.

This fix can be verified by assembling and disassembling the following:

	.text
	.long 0x65005f5f

With this patch, the bug is fixed, and the objdump will know that
0x65005f5f is not a barrel shift instruction.

Signed-off-by: Neal Frager <neal.frager@amd.com>
---
 gas/testsuite/gas/microblaze/allinsn.d |  4 ++--
 opcodes/microblaze-dis.c               |  4 ++--
 opcodes/microblaze-opc.h               | 11 +++++++----
 3 files changed, 11 insertions(+), 8 deletions(-)

Comments

Michael Eager Oct. 20, 2023, 12:59 a.m. UTC | #1 
On 10/19/23 04:37, Neal Frager wrote:
> There is currently a bug in the bit masking for the barrel shift
> instructions because the bit mask is not including all of the
> register bits which must be zero.  With this patch, the disassembler
> can be sure that the 32-bit value is indeed a barrel shift instruction
> and not a data value in memory.
> 
> This fix can be verified by assembling and disassembling the following:
> 
> 	.text
> 	.long 0x65005f5f
> 
> With this patch, the bug is fixed, and the objdump will know that
> 0x65005f5f is not a barrel shift instruction.
> 
> Signed-off-by: Neal Frager <neal.frager@amd.com>
> ---
>   gas/testsuite/gas/microblaze/allinsn.d |  4 ++--
>   opcodes/microblaze-dis.c               |  4 ++--
>   opcodes/microblaze-opc.h               | 11 +++++++----
>   3 files changed, 11 insertions(+), 8 deletions(-)

Committed.
Nick Clifton Nov. 10, 2023, 2:24 p.m. UTC | #2 
Hi Neal,

> There is currently a bug in the bit masking for the barrel shift
> instructions because the bit mask is not including all of the
> register bits which must be zero.  With this patch, the disassembler
> can be sure that the 32-bit value is indeed a barrel shift instruction
> and not a data value in memory.
> 
> This fix can be verified by assembling and disassembling the following:
> 
> 	.text
> 	.long 0x65005f5f
> 
> With this patch, the bug is fixed, and the objdump will know that
> 0x65005f5f is not a barrel shift instruction.
> 

Approved - please apply.

Cheers
   Nick
Michael Eager Nov. 10, 2023, 4:09 p.m. UTC | #3 
On 11/10/23 06:24, Nick Clifton wrote:
> Hi Neal,
> 
>> There is currently a bug in the bit masking for the barrel shift
>> instructions because the bit mask is not including all of the
>> register bits which must be zero.  With this patch, the disassembler
>> can be sure that the 32-bit value is indeed a barrel shift instruction
>> and not a data value in memory.
>>
>> This fix can be verified by assembling and disassembling the following:
>>
>>     .text
>>     .long 0x65005f5f
>>
>> With this patch, the bug is fixed, and the objdump will know that
>> 0x65005f5f is not a barrel shift instruction.
>>
> 
> Approved - please apply.

This patch was applied on 10/19/23.
diff mbox series

Patch

diff --git a/gas/testsuite/gas/microblaze/allinsn.d b/gas/testsuite/gas/microblaze/allinsn.d
index 312e3fb3805..83175f134dc 100644
--- a/gas/testsuite/gas/microblaze/allinsn.d
+++ b/gas/testsuite/gas/microblaze/allinsn.d
@@ -49,7 +49,7 @@  Disassembly of section .text:
   40:	900001e2 	swaph	r0, r0
 
 00000044 <bsefi>:
-  44:	64004041 	bsrli	r0, r0, 1
+  44:	64004041 	bsefi	r0, r0, 1, 1
 
 00000048 <bsifi>:
-  48:	64008041 	bsrli	r0, r0, 1
+  48:	64008041 	bsifi	r0, r0, 1, 1
diff --git a/opcodes/microblaze-dis.c b/opcodes/microblaze-dis.c
index 468797befc7..0b5262255fb 100644
--- a/opcodes/microblaze-dis.c
+++ b/opcodes/microblaze-dis.c
@@ -35,7 +35,7 @@ 
 #define get_int_field_imm(instr)   ((instr & IMM_MASK) >> IMM_LOW)
 #define get_int_field_r1(instr)    ((instr & RA_MASK) >> RA_LOW)
 
-#define NUM_STRBUFS 3
+#define NUM_STRBUFS 4
 #define STRBUF_SIZE 25
 
 struct string_buf
@@ -279,7 +279,7 @@  print_insn_microblaze (bfd_vma memaddr, struct disassemble_info * info)
   prev_insn_vma = curr_insn_vma;
 
   if (op->name == NULL)
-    print_func (stream, ".short 0x%04x", (unsigned int) inst);
+    print_func (stream, ".long 0x%04x", (unsigned int) inst);
   else
     {
       print_func (stream, "%s", op->name);
diff --git a/opcodes/microblaze-opc.h b/opcodes/microblaze-opc.h
index 811b5cbeb0f..b9045f67969 100644
--- a/opcodes/microblaze-opc.h
+++ b/opcodes/microblaze-opc.h
@@ -92,8 +92,11 @@ 
 #define OPCODE_MASK_H124  0xFFFF07FF /* High 16, and low 11 bits.  */
 #define OPCODE_MASK_H1234 0xFFFFFFFF /* All 32 bits.  */
 #define OPCODE_MASK_H3    0xFC000600 /* High 6 bits and bits 21, 22.  */
+#define OPCODE_MASK_H3B   0xFC00F9E0 /* High 6 bits and bits 16:20 and
+					bits 23:26. */
 #define OPCODE_MASK_H32   0xFC00FC00 /* High 6 bits and bit 16-21.  */
-#define OPCODE_MASK_H32B  0xFC00C000 /* High 6 bits and bit 16, 17.  */
+#define OPCODE_MASK_H32B  0xFC00F820 /* High 6 bits and bits 16:20 and
+					bit 26 */
 #define OPCODE_MASK_H34B  0xFC0000FF /* High 6 bits and low 8 bits.  */
 #define OPCODE_MASK_H35B  0xFC0004FF /* High 6 bits and low 9 bits.  */
 #define OPCODE_MASK_H34C  0xFC0007E0 /* High 6 bits and bits 21-26.  */
@@ -160,9 +163,9 @@  const struct op_code_struct
   {"ncget", INST_TYPE_RD_RFSL, INST_NO_OFFSET, NO_DELAY_SLOT, IMMVAL_MASK_NON_SPECIAL, 0x6C006000, OPCODE_MASK_H32, ncget, anyware_inst },
   {"ncput", INST_TYPE_R1_RFSL, INST_NO_OFFSET, NO_DELAY_SLOT, IMMVAL_MASK_NON_SPECIAL, 0x6C00E000, OPCODE_MASK_H32, ncput, anyware_inst },
   {"muli",  INST_TYPE_RD_R1_IMM, INST_NO_OFFSET, NO_DELAY_SLOT, IMMVAL_MASK_NON_SPECIAL, 0x60000000, OPCODE_MASK_H, muli, mult_inst },
-  {"bslli", INST_TYPE_RD_R1_IMM5, INST_NO_OFFSET, NO_DELAY_SLOT, IMMVAL_MASK_NON_SPECIAL, 0x64000400, OPCODE_MASK_H3, bslli, barrel_shift_inst },
-  {"bsrai", INST_TYPE_RD_R1_IMM5, INST_NO_OFFSET, NO_DELAY_SLOT, IMMVAL_MASK_NON_SPECIAL, 0x64000200, OPCODE_MASK_H3, bsrai, barrel_shift_inst },
-  {"bsrli", INST_TYPE_RD_R1_IMM5, INST_NO_OFFSET, NO_DELAY_SLOT, IMMVAL_MASK_NON_SPECIAL, 0x64000000, OPCODE_MASK_H3, bsrli, barrel_shift_inst },
+  {"bslli", INST_TYPE_RD_R1_IMM5, INST_NO_OFFSET, NO_DELAY_SLOT, IMMVAL_MASK_NON_SPECIAL, 0x64000400, OPCODE_MASK_H3B, bslli, barrel_shift_inst },
+  {"bsrai", INST_TYPE_RD_R1_IMM5, INST_NO_OFFSET, NO_DELAY_SLOT, IMMVAL_MASK_NON_SPECIAL, 0x64000200, OPCODE_MASK_H3B, bsrai, barrel_shift_inst },
+  {"bsrli", INST_TYPE_RD_R1_IMM5, INST_NO_OFFSET, NO_DELAY_SLOT, IMMVAL_MASK_NON_SPECIAL, 0x64000000, OPCODE_MASK_H3B, bsrli, barrel_shift_inst },
   {"bsefi", INST_TYPE_RD_R1_IMMW_IMMS, INST_NO_OFFSET, NO_DELAY_SLOT, IMMVAL_MASK_NON_SPECIAL, 0x64004000, OPCODE_MASK_H32B, bsefi, barrel_shift_inst },
   {"bsifi", INST_TYPE_RD_R1_IMMW_IMMS, INST_NO_OFFSET, NO_DELAY_SLOT, IMMVAL_MASK_NON_SPECIAL, 0x64008000, OPCODE_MASK_H32B, bsifi, barrel_shift_inst },
   {"or",    INST_TYPE_RD_R1_R2, INST_NO_OFFSET, NO_DELAY_SLOT, IMMVAL_MASK_NON_SPECIAL, 0x80000000, OPCODE_MASK_H4, microblaze_or, logical_inst },