public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] RISC-V: Fix big endian disassembly of data.
@ 2021-11-01  7:55 Guy Benyei
  2021-11-04  6:38 ` Guy Benyei
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Guy Benyei @ 2021-11-01  7:55 UTC (permalink / raw)
  To: binutils; +Cc: Marcus Comstedt, nelson.chu, Kito Cheng

When disassembling big endian RISC-V binary, the data displayed
with swapped endiannes. Binary generated from:

	.word 0x01020304

was disassembled as:

	.word 0x04030201

opcodes/
	* riscv-dis.c: Consider endiannes when printing data

gas/
	* testsuite/gas/riscv/mapping-03b.d: check only for little endian
	* testsuite/gas/riscv/mapping-04b.d: likewise
	* testsuite/gas/riscv/mapping-norelax-03b.d: likewise
	* testsuite/gas/riscv/mapping-norelax-04b.d: likewise
	* testsuite/gas/riscv/mapping-03bbe.d: big endian data/code mapping test
	* testsuite/gas/riscv/mapping-04bbe.d: likewise
	* testsuite/gas/riscv/mapping-norelax-03bbe.d: likewise
	* testsuite/gas/riscv/mapping-norelax-04bbe.d: likewise

Conflicts:
	gas/ChangeLog
	opcodes/ChangeLog
---
 gas/ChangeLog                                   | 10 ++++++++++
 gas/testsuite/gas/riscv/mapping-03b.d           |  2 +-
 gas/testsuite/gas/riscv/mapping-03bbe.d         | 24 ++++++++++++++++++++++++
 gas/testsuite/gas/riscv/mapping-04b.d           |  2 +-
 gas/testsuite/gas/riscv/mapping-04bbe.d         | 23 +++++++++++++++++++++++
 gas/testsuite/gas/riscv/mapping-norelax-03b.d   |  2 +-
 gas/testsuite/gas/riscv/mapping-norelax-03bbe.d | 25 +++++++++++++++++++++++++
 gas/testsuite/gas/riscv/mapping-norelax-04b.d   |  2 +-
 gas/testsuite/gas/riscv/mapping-norelax-04bbe.d | 24 ++++++++++++++++++++++++
 opcodes/ChangeLog                               |  4 ++++
 opcodes/riscv-dis.c                             | 11 ++++++-----
 11 files changed, 120 insertions(+), 9 deletions(-)
 create mode 100644 gas/testsuite/gas/riscv/mapping-03bbe.d
 create mode 100644 gas/testsuite/gas/riscv/mapping-04bbe.d
 create mode 100644 gas/testsuite/gas/riscv/mapping-norelax-03bbe.d
 create mode 100644 gas/testsuite/gas/riscv/mapping-norelax-04bbe.d

diff --git a/gas/ChangeLog b/gas/ChangeLog
index 1133847..ddfd613 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,13 @@
+2021-11-01  Guy Benyei <guybe@nvidia.com>
+	* testsuite/gas/riscv/mapping-03b.d: check only for little endian
+	* testsuite/gas/riscv/mapping-04b.d: likewise
+	* testsuite/gas/riscv/mapping-norelax-03b.d: likewise
+	* testsuite/gas/riscv/mapping-norelax-04b.d: likewise
+	* testsuite/gas/riscv/mapping-03bbe.d: big endian data/code mapping test
+	* testsuite/gas/riscv/mapping-04bbe.d: likewise
+	* testsuite/gas/riscv/mapping-norelax-03bbe.d: likewise
+	* testsuite/gas/riscv/mapping-norelax-04bbe.d: likewise
+
 2021-10-28  Markus Klein  <markus.klein@sma.de>
 
 	PR 28436
diff --git a/gas/testsuite/gas/riscv/mapping-03b.d b/gas/testsuite/gas/riscv/mapping-03b.d
index f4f6726..ecb3e31 100644
--- a/gas/testsuite/gas/riscv/mapping-03b.d
+++ b/gas/testsuite/gas/riscv/mapping-03b.d
@@ -1,4 +1,4 @@
-#as:
+#as: -mlittle-endian
 #source: mapping-03.s
 #objdump: -d
 
diff --git a/gas/testsuite/gas/riscv/mapping-03bbe.d b/gas/testsuite/gas/riscv/mapping-03bbe.d
new file mode 100644
index 0000000..9e5b429
--- /dev/null
+++ b/gas/testsuite/gas/riscv/mapping-03bbe.d
@@ -0,0 +1,24 @@
+#as: -mbig-endian
+#source: mapping-03.s
+#objdump: -d
+
+.*:[ 	]+file format .*
+
+
+Disassembly of section .text:
+
+0+000 <.text>:
+[ 	]+0:[ 	]+00a50533[ 	]+add[ 	]+a0,a0,a0
+[ 	]+4:[ 	]+00000000[ 	]+.word[ 	]+0x00000000
+[ 	]+8:[ 	]+00000013[ 	]+nop
+[ 	]+c:[ 	]+00000013[ 	]+nop
+[ 	]+10:[ 	]+00000013[ 	]+nop
+[ 	]+14:[ 	]+00000001[ 	]+.word[ 	]+0x00000001
+[ 	]+18:[ 	]+00b585b3[ 	]+add[ 	]+a1,a1,a1
+[ 	]+1c:[ 	]+02000000[ 	]+.word[ 	]+0x02000000
+[ 	]+20:[ 	]+03[ 	]+.byte[ 	]+0x03
+[ 	]+21:[ 	]+00000013[ 	]+nop
+[ 	]+25:[ 	]+00000013[ 	]+nop
+[ 	]+29:[ 	]+00000013[ 	]+nop
+[ 	]+2d:[ 	]+00000005[ 	]+.word[ 	]+0x00000005
+#...
diff --git a/gas/testsuite/gas/riscv/mapping-04b.d b/gas/testsuite/gas/riscv/mapping-04b.d
index 9735498..5616220 100644
--- a/gas/testsuite/gas/riscv/mapping-04b.d
+++ b/gas/testsuite/gas/riscv/mapping-04b.d
@@ -1,4 +1,4 @@
-#as:
+#as: -mlittle-endian
 #source: mapping-04.s
 #objdump: -d
 
diff --git a/gas/testsuite/gas/riscv/mapping-04bbe.d b/gas/testsuite/gas/riscv/mapping-04bbe.d
new file mode 100644
index 0000000..c5339c8
--- /dev/null
+++ b/gas/testsuite/gas/riscv/mapping-04bbe.d
@@ -0,0 +1,23 @@
+#as: -mbig-endian
+#source: mapping-04.s
+#objdump: -d
+
+.*:[ 	]+file format .*
+
+
+Disassembly of section .text:
+
+0+000 <.text>:
+[ 	]+0:[ 	]+00001001[ 	]+.word[ 	]+0x00001001
+[ 	]+4:[ 	]+00001001[ 	]+.word[ 	]+0x00001001
+[ 	]+8:[ 	]+01000000[ 	]+.word[ 	]+0x01000000
+[ 	]+c:[ 	]+00[ 	]+.byte[ 	]+0x00
+[ 	]+d:[ 	]+00000013[ 	]+nop
+[ 	]+11:[ 	]+00a50533[ 	]+add[ 	]+a0,a0,a0
+[ 	]+15:[ 	]+20022002[ 	]+.word[ 	]+0x20022002
+[ 	]+19:[ 	]+20022002[ 	]+.word[ 	]+0x20022002
+[ 	]+1d:[ 	]+2002[ 	]+.short[ 	]+0x2002
+[ 	]+1f:[ 	]+00b585b3[ 	]+add[ 	]+a1,a1,a1
+[ 	]+23:[ 	]+0000[ 	]+unimp
+[ 	]+25:[ 	]+0000[ 	]+unimp
+#...
diff --git a/gas/testsuite/gas/riscv/mapping-norelax-03b.d b/gas/testsuite/gas/riscv/mapping-norelax-03b.d
index ad88888..e65a922 100644
--- a/gas/testsuite/gas/riscv/mapping-norelax-03b.d
+++ b/gas/testsuite/gas/riscv/mapping-norelax-03b.d
@@ -1,4 +1,4 @@
-#as: -mno-relax
+#as: -mno-relax -mlittle-endian
 #source: mapping-03.s
 #objdump: -d
 
diff --git a/gas/testsuite/gas/riscv/mapping-norelax-03bbe.d b/gas/testsuite/gas/riscv/mapping-norelax-03bbe.d
new file mode 100644
index 0000000..c8520fd
--- /dev/null
+++ b/gas/testsuite/gas/riscv/mapping-norelax-03bbe.d
@@ -0,0 +1,25 @@
+#as: -mno-relax -mbig-endian
+#source: mapping-03.s
+#objdump: -d
+
+.*:[ 	]+file format .*
+
+
+Disassembly of section .text:
+
+0+000 <.text>:
+[ 	]+0:[ 	]+00a50533[ 	]+add[ 	]+a0,a0,a0
+[ 	]+4:[ 	]+00000000[ 	]+.word[ 	]+0x00000000
+[ 	]+8:[ 	]+00000013[ 	]+nop
+[ 	]+c:[ 	]+00000013[ 	]+nop
+[ 	]+10:[ 	]+00000001[ 	]+.word[ 	]+0x00000001
+[ 	]+14:[ 	]+00b585b3[ 	]+add[ 	]+a1,a1,a1
+[ 	]+18:[ 	]+02000000[ 	]+.word[ 	]+0x02000000
+[ 	]+1c:[ 	]+03[ 	]+.byte[ 	]+0x03
+[ 	]+1d:[ 	]+00[ 	]+.byte[ 	]+0x00
+[ 	]+1e:[ 	]+0001[ 	]+nop
+[ 	]+20:[ 	]+00000005[ 	]+.word[ 	]+0x00000005
+[ 	]+24:[ 	]+00000013[ 	]+nop
+[ 	]+28:[ 	]+00000013[ 	]+nop
+[ 	]+2c:[ 	]+00000013[ 	]+nop
+#...
diff --git a/gas/testsuite/gas/riscv/mapping-norelax-04b.d b/gas/testsuite/gas/riscv/mapping-norelax-04b.d
index 824a898..00a9dbd 100644
--- a/gas/testsuite/gas/riscv/mapping-norelax-04b.d
+++ b/gas/testsuite/gas/riscv/mapping-norelax-04b.d
@@ -1,4 +1,4 @@
-#as: -mno-relax
+#as: -mno-relax -mlittle-endian
 #source: mapping-04.s
 #objdump: -d
 
diff --git a/gas/testsuite/gas/riscv/mapping-norelax-04bbe.d b/gas/testsuite/gas/riscv/mapping-norelax-04bbe.d
new file mode 100644
index 0000000..0b987fa
--- /dev/null
+++ b/gas/testsuite/gas/riscv/mapping-norelax-04bbe.d
@@ -0,0 +1,24 @@
+#as: -mno-relax -mbig-endian
+#source: mapping-04.s
+#objdump: -d
+
+.*:[ 	]+file format .*
+
+
+Disassembly of section .text:
+
+0+000 <.text>:
+[ 	]+0:[ 	]+00001001[ 	]+.word[ 	]+0x00001001
+[ 	]+4:[ 	]+00001001[ 	]+.word[ 	]+0x00001001
+[ 	]+8:[ 	]+01000000[ 	]+.word[ 	]+0x01000000
+[ 	]+c:[ 	]+00[ 	]+.byte[ 	]+0x00
+[ 	]+d:[ 	]+00[ 	]+.byte[ 	]+0x00
+[ 	]+e:[ 	]+0001[ 	]+nop
+[ 	]+10:[ 	]+00a50533[ 	]+add[ 	]+a0,a0,a0
+[ 	]+14:[ 	]+20022002[ 	]+.word[ 	]+0x20022002
+[ 	]+18:[ 	]+20022002[ 	]+.word[ 	]+0x20022002
+[ 	]+1c:[ 	]+2002[ 	]+.short[ 	]+0x2002
+[ 	]+1e:[ 	]+00b585b3[ 	]+add[ 	]+a1,a1,a1
+[ 	]+22:[ 	]+0001[ 	]+nop
+[ 	]+24:[ 	]+00000013[ 	]+nop
+#...
diff --git a/opcodes/ChangeLog b/opcodes/ChangeLog
index f0e4b72..0f37316 100644
--- a/opcodes/ChangeLog
+++ b/opcodes/ChangeLog
@@ -1,3 +1,7 @@
+2021-11-01 Guy Benyei <guybe@nvidia.com>
+
+	* riscv-dis.c: Consider endiannes when printing data
+
 2021-10-27  Maciej W. Rozycki  <macro@embecosm.com>
 
 	* Makefile.am: Remove obsolete comment.
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 1a09440..56af8e0 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -485,13 +485,9 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info)
 
   insnlen = riscv_insn_length (word);
 
-  /* RISC-V instructions are always little-endian.  */
-  info->endian_code = BFD_ENDIAN_LITTLE;
-
   info->bytes_per_chunk = insnlen % 4 == 0 ? 4 : 2;
   info->bytes_per_line = 8;
   /* We don't support constant pools, so this must be code.  */
-  info->display_endian = info->endian_code;
   info->insn_info_valid = 1;
   info->branch_delay_insns = 0;
   info->data_size = 0;
@@ -811,6 +807,9 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
   else if (riscv_gpr_names == NULL)
     set_default_riscv_dis_options ();
 
+  /* RISC-V instructions are always little-endian.  */
+  info->endian_code = BFD_ENDIAN_LITTLE;
+
   mstate = riscv_search_mapping_symbol (memaddr, info);
   /* Save the last mapping state.  */
   last_map_state = mstate;
@@ -822,6 +821,7 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
       dump_size = riscv_data_length (memaddr, info);
       info->bytes_per_chunk = dump_size;
       riscv_disassembler = riscv_disassemble_data;
+      info->display_endian = info->endian;
     }
   else
     {
@@ -835,6 +835,7 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
       insn = (insn_t) bfd_getl16 (packet);
       dump_size = riscv_insn_length (insn);
       riscv_disassembler = riscv_disassemble_insn;
+      info->display_endian = info->endian_code;
     }
 
   /* Fetch the instruction to dump.  */
@@ -844,7 +845,7 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
       (*info->memory_error_func) (status, memaddr, info);
       return status;
     }
-  insn = (insn_t) bfd_get_bits (packet, dump_size * 8, false);
+  insn = (insn_t) bfd_get_bits (packet, dump_size * 8, info->display_endian == BFD_ENDIAN_BIG);
 
   return (*riscv_disassembler) (memaddr, insn, info);
 }
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] RISC-V: Fix big endian disassembly of data.
  2021-11-01  7:55 [PATCH] RISC-V: Fix big endian disassembly of data Guy Benyei
@ 2021-11-04  6:38 ` Guy Benyei
  2021-11-04  7:32   ` Nelson Chu
  2021-11-04  7:38 ` Marcus Comstedt
  2021-11-04 12:17 ` Maciej W. Rozycki
  2 siblings, 1 reply; 9+ messages in thread
From: Guy Benyei @ 2021-11-04  6:38 UTC (permalink / raw)
  To: binutils; +Cc: Kito Cheng, Marcus Comstedt

Ping?

-----Original Message-----
From: Binutils <binutils-bounces+guybe=mellanox.com@sourceware.org> On Behalf Of Guy Benyei via Binutils
Sent: Monday, November 1, 2021 9:55 AM
To: binutils@sourceware.org
Cc: Kito Cheng <kito.cheng@sifive.com>; Marcus Comstedt <marcus@mc.pp.se>
Subject: [PATCH] RISC-V: Fix big endian disassembly of data.

When disassembling big endian RISC-V binary, the data displayed with swapped endiannes. Binary generated from:

	.word 0x01020304

was disassembled as:

	.word 0x04030201

opcodes/
	* riscv-dis.c: Consider endiannes when printing data

gas/
	* testsuite/gas/riscv/mapping-03b.d: check only for little endian
	* testsuite/gas/riscv/mapping-04b.d: likewise
	* testsuite/gas/riscv/mapping-norelax-03b.d: likewise
	* testsuite/gas/riscv/mapping-norelax-04b.d: likewise
	* testsuite/gas/riscv/mapping-03bbe.d: big endian data/code mapping test
	* testsuite/gas/riscv/mapping-04bbe.d: likewise
	* testsuite/gas/riscv/mapping-norelax-03bbe.d: likewise
	* testsuite/gas/riscv/mapping-norelax-04bbe.d: likewise

Conflicts:
	gas/ChangeLog
	opcodes/ChangeLog
---
 gas/ChangeLog                                   | 10 ++++++++++
 gas/testsuite/gas/riscv/mapping-03b.d           |  2 +-
 gas/testsuite/gas/riscv/mapping-03bbe.d         | 24 ++++++++++++++++++++++++
 gas/testsuite/gas/riscv/mapping-04b.d           |  2 +-
 gas/testsuite/gas/riscv/mapping-04bbe.d         | 23 +++++++++++++++++++++++
 gas/testsuite/gas/riscv/mapping-norelax-03b.d   |  2 +-
 gas/testsuite/gas/riscv/mapping-norelax-03bbe.d | 25 +++++++++++++++++++++++++
 gas/testsuite/gas/riscv/mapping-norelax-04b.d   |  2 +-
 gas/testsuite/gas/riscv/mapping-norelax-04bbe.d | 24 ++++++++++++++++++++++++
 opcodes/ChangeLog                               |  4 ++++
 opcodes/riscv-dis.c                             | 11 ++++++-----
 11 files changed, 120 insertions(+), 9 deletions(-)  create mode 100644 gas/testsuite/gas/riscv/mapping-03bbe.d
 create mode 100644 gas/testsuite/gas/riscv/mapping-04bbe.d
 create mode 100644 gas/testsuite/gas/riscv/mapping-norelax-03bbe.d
 create mode 100644 gas/testsuite/gas/riscv/mapping-norelax-04bbe.d

diff --git a/gas/ChangeLog b/gas/ChangeLog index 1133847..ddfd613 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,13 @@
+2021-11-01  Guy Benyei <guybe@nvidia.com>
+	* testsuite/gas/riscv/mapping-03b.d: check only for little endian
+	* testsuite/gas/riscv/mapping-04b.d: likewise
+	* testsuite/gas/riscv/mapping-norelax-03b.d: likewise
+	* testsuite/gas/riscv/mapping-norelax-04b.d: likewise
+	* testsuite/gas/riscv/mapping-03bbe.d: big endian data/code mapping test
+	* testsuite/gas/riscv/mapping-04bbe.d: likewise
+	* testsuite/gas/riscv/mapping-norelax-03bbe.d: likewise
+	* testsuite/gas/riscv/mapping-norelax-04bbe.d: likewise
+
 2021-10-28  Markus Klein  <markus.klein@sma.de>
 
 	PR 28436
diff --git a/gas/testsuite/gas/riscv/mapping-03b.d b/gas/testsuite/gas/riscv/mapping-03b.d
index f4f6726..ecb3e31 100644
--- a/gas/testsuite/gas/riscv/mapping-03b.d
+++ b/gas/testsuite/gas/riscv/mapping-03b.d
@@ -1,4 +1,4 @@
-#as:
+#as: -mlittle-endian
 #source: mapping-03.s
 #objdump: -d
 
diff --git a/gas/testsuite/gas/riscv/mapping-03bbe.d b/gas/testsuite/gas/riscv/mapping-03bbe.d
new file mode 100644
index 0000000..9e5b429
--- /dev/null
+++ b/gas/testsuite/gas/riscv/mapping-03bbe.d
@@ -0,0 +1,24 @@
+#as: -mbig-endian
+#source: mapping-03.s
+#objdump: -d
+
+.*:[ 	]+file format .*
+
+
+Disassembly of section .text:
+
+0+000 <.text>:
+[ 	]+0:[ 	]+00a50533[ 	]+add[ 	]+a0,a0,a0
+[ 	]+4:[ 	]+00000000[ 	]+.word[ 	]+0x00000000
+[ 	]+8:[ 	]+00000013[ 	]+nop
+[ 	]+c:[ 	]+00000013[ 	]+nop
+[ 	]+10:[ 	]+00000013[ 	]+nop
+[ 	]+14:[ 	]+00000001[ 	]+.word[ 	]+0x00000001
+[ 	]+18:[ 	]+00b585b3[ 	]+add[ 	]+a1,a1,a1
+[ 	]+1c:[ 	]+02000000[ 	]+.word[ 	]+0x02000000
+[ 	]+20:[ 	]+03[ 	]+.byte[ 	]+0x03
+[ 	]+21:[ 	]+00000013[ 	]+nop
+[ 	]+25:[ 	]+00000013[ 	]+nop
+[ 	]+29:[ 	]+00000013[ 	]+nop
+[ 	]+2d:[ 	]+00000005[ 	]+.word[ 	]+0x00000005
+#...
diff --git a/gas/testsuite/gas/riscv/mapping-04b.d b/gas/testsuite/gas/riscv/mapping-04b.d
index 9735498..5616220 100644
--- a/gas/testsuite/gas/riscv/mapping-04b.d
+++ b/gas/testsuite/gas/riscv/mapping-04b.d
@@ -1,4 +1,4 @@
-#as:
+#as: -mlittle-endian
 #source: mapping-04.s
 #objdump: -d
 
diff --git a/gas/testsuite/gas/riscv/mapping-04bbe.d b/gas/testsuite/gas/riscv/mapping-04bbe.d
new file mode 100644
index 0000000..c5339c8
--- /dev/null
+++ b/gas/testsuite/gas/riscv/mapping-04bbe.d
@@ -0,0 +1,23 @@
+#as: -mbig-endian
+#source: mapping-04.s
+#objdump: -d
+
+.*:[ 	]+file format .*
+
+
+Disassembly of section .text:
+
+0+000 <.text>:
+[ 	]+0:[ 	]+00001001[ 	]+.word[ 	]+0x00001001
+[ 	]+4:[ 	]+00001001[ 	]+.word[ 	]+0x00001001
+[ 	]+8:[ 	]+01000000[ 	]+.word[ 	]+0x01000000
+[ 	]+c:[ 	]+00[ 	]+.byte[ 	]+0x00
+[ 	]+d:[ 	]+00000013[ 	]+nop
+[ 	]+11:[ 	]+00a50533[ 	]+add[ 	]+a0,a0,a0
+[ 	]+15:[ 	]+20022002[ 	]+.word[ 	]+0x20022002
+[ 	]+19:[ 	]+20022002[ 	]+.word[ 	]+0x20022002
+[ 	]+1d:[ 	]+2002[ 	]+.short[ 	]+0x2002
+[ 	]+1f:[ 	]+00b585b3[ 	]+add[ 	]+a1,a1,a1
+[ 	]+23:[ 	]+0000[ 	]+unimp
+[ 	]+25:[ 	]+0000[ 	]+unimp
+#...
diff --git a/gas/testsuite/gas/riscv/mapping-norelax-03b.d b/gas/testsuite/gas/riscv/mapping-norelax-03b.d
index ad88888..e65a922 100644
--- a/gas/testsuite/gas/riscv/mapping-norelax-03b.d
+++ b/gas/testsuite/gas/riscv/mapping-norelax-03b.d
@@ -1,4 +1,4 @@
-#as: -mno-relax
+#as: -mno-relax -mlittle-endian
 #source: mapping-03.s
 #objdump: -d
 
diff --git a/gas/testsuite/gas/riscv/mapping-norelax-03bbe.d b/gas/testsuite/gas/riscv/mapping-norelax-03bbe.d
new file mode 100644
index 0000000..c8520fd
--- /dev/null
+++ b/gas/testsuite/gas/riscv/mapping-norelax-03bbe.d
@@ -0,0 +1,25 @@
+#as: -mno-relax -mbig-endian
+#source: mapping-03.s
+#objdump: -d
+
+.*:[ 	]+file format .*
+
+
+Disassembly of section .text:
+
+0+000 <.text>:
+[ 	]+0:[ 	]+00a50533[ 	]+add[ 	]+a0,a0,a0
+[ 	]+4:[ 	]+00000000[ 	]+.word[ 	]+0x00000000
+[ 	]+8:[ 	]+00000013[ 	]+nop
+[ 	]+c:[ 	]+00000013[ 	]+nop
+[ 	]+10:[ 	]+00000001[ 	]+.word[ 	]+0x00000001
+[ 	]+14:[ 	]+00b585b3[ 	]+add[ 	]+a1,a1,a1
+[ 	]+18:[ 	]+02000000[ 	]+.word[ 	]+0x02000000
+[ 	]+1c:[ 	]+03[ 	]+.byte[ 	]+0x03
+[ 	]+1d:[ 	]+00[ 	]+.byte[ 	]+0x00
+[ 	]+1e:[ 	]+0001[ 	]+nop
+[ 	]+20:[ 	]+00000005[ 	]+.word[ 	]+0x00000005
+[ 	]+24:[ 	]+00000013[ 	]+nop
+[ 	]+28:[ 	]+00000013[ 	]+nop
+[ 	]+2c:[ 	]+00000013[ 	]+nop
+#...
diff --git a/gas/testsuite/gas/riscv/mapping-norelax-04b.d b/gas/testsuite/gas/riscv/mapping-norelax-04b.d
index 824a898..00a9dbd 100644
--- a/gas/testsuite/gas/riscv/mapping-norelax-04b.d
+++ b/gas/testsuite/gas/riscv/mapping-norelax-04b.d
@@ -1,4 +1,4 @@
-#as: -mno-relax
+#as: -mno-relax -mlittle-endian
 #source: mapping-04.s
 #objdump: -d
 
diff --git a/gas/testsuite/gas/riscv/mapping-norelax-04bbe.d b/gas/testsuite/gas/riscv/mapping-norelax-04bbe.d
new file mode 100644
index 0000000..0b987fa
--- /dev/null
+++ b/gas/testsuite/gas/riscv/mapping-norelax-04bbe.d
@@ -0,0 +1,24 @@
+#as: -mno-relax -mbig-endian
+#source: mapping-04.s
+#objdump: -d
+
+.*:[ 	]+file format .*
+
+
+Disassembly of section .text:
+
+0+000 <.text>:
+[ 	]+0:[ 	]+00001001[ 	]+.word[ 	]+0x00001001
+[ 	]+4:[ 	]+00001001[ 	]+.word[ 	]+0x00001001
+[ 	]+8:[ 	]+01000000[ 	]+.word[ 	]+0x01000000
+[ 	]+c:[ 	]+00[ 	]+.byte[ 	]+0x00
+[ 	]+d:[ 	]+00[ 	]+.byte[ 	]+0x00
+[ 	]+e:[ 	]+0001[ 	]+nop
+[ 	]+10:[ 	]+00a50533[ 	]+add[ 	]+a0,a0,a0
+[ 	]+14:[ 	]+20022002[ 	]+.word[ 	]+0x20022002
+[ 	]+18:[ 	]+20022002[ 	]+.word[ 	]+0x20022002
+[ 	]+1c:[ 	]+2002[ 	]+.short[ 	]+0x2002
+[ 	]+1e:[ 	]+00b585b3[ 	]+add[ 	]+a1,a1,a1
+[ 	]+22:[ 	]+0001[ 	]+nop
+[ 	]+24:[ 	]+00000013[ 	]+nop
+#...
diff --git a/opcodes/ChangeLog b/opcodes/ChangeLog index f0e4b72..0f37316 100644
--- a/opcodes/ChangeLog
+++ b/opcodes/ChangeLog
@@ -1,3 +1,7 @@
+2021-11-01 Guy Benyei <guybe@nvidia.com>
+
+	* riscv-dis.c: Consider endiannes when printing data
+
 2021-10-27  Maciej W. Rozycki  <macro@embecosm.com>
 
 	* Makefile.am: Remove obsolete comment.
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c index 1a09440..56af8e0 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -485,13 +485,9 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info)
 
   insnlen = riscv_insn_length (word);
 
-  /* RISC-V instructions are always little-endian.  */
-  info->endian_code = BFD_ENDIAN_LITTLE;
-
   info->bytes_per_chunk = insnlen % 4 == 0 ? 4 : 2;
   info->bytes_per_line = 8;
   /* We don't support constant pools, so this must be code.  */
-  info->display_endian = info->endian_code;
   info->insn_info_valid = 1;
   info->branch_delay_insns = 0;
   info->data_size = 0;
@@ -811,6 +807,9 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
   else if (riscv_gpr_names == NULL)
     set_default_riscv_dis_options ();
 
+  /* RISC-V instructions are always little-endian.  */  
+ info->endian_code = BFD_ENDIAN_LITTLE;
+
   mstate = riscv_search_mapping_symbol (memaddr, info);
   /* Save the last mapping state.  */
   last_map_state = mstate;
@@ -822,6 +821,7 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
       dump_size = riscv_data_length (memaddr, info);
       info->bytes_per_chunk = dump_size;
       riscv_disassembler = riscv_disassemble_data;
+      info->display_endian = info->endian;
     }
   else
     {
@@ -835,6 +835,7 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
       insn = (insn_t) bfd_getl16 (packet);
       dump_size = riscv_insn_length (insn);
       riscv_disassembler = riscv_disassemble_insn;
+      info->display_endian = info->endian_code;
     }
 
   /* Fetch the instruction to dump.  */ @@ -844,7 +845,7 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
       (*info->memory_error_func) (status, memaddr, info);
       return status;
     }
-  insn = (insn_t) bfd_get_bits (packet, dump_size * 8, false);
+  insn = (insn_t) bfd_get_bits (packet, dump_size * 8, 
+ info->display_endian == BFD_ENDIAN_BIG);
 
   return (*riscv_disassembler) (memaddr, insn, info);  }
--
1.8.3.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] RISC-V: Fix big endian disassembly of data.
  2021-11-04  6:38 ` Guy Benyei
@ 2021-11-04  7:32   ` Nelson Chu
  2021-11-04  7:51     ` Guy Benyei
  0 siblings, 1 reply; 9+ messages in thread
From: Nelson Chu @ 2021-11-04  7:32 UTC (permalink / raw)
  To: Guy Benyei; +Cc: binutils, Kito Cheng, Marcus Comstedt

Hi Guy,

The change of opcodes/riscv-dis.c looks reasonable, and I'm OK with
this change, thank you.  But unfortunately I cannot find either your
or Nvidia's copyright assignment of binutil, so this may cause a
problem.  However, the trivial 10-line change should be accepted
without the copyright assignment, but I had heard that the rule is
that - 10-line for all patches from the person total, not each patch
individually.  I think the 11-line change of opcodes/riscv-dis.c
should be OK under this rule, so I would suggest that,

1. Keep the 11-line change of opcodes/riscv-dis.c, or you can reduce
the number of lines.
2. Moving the changes of ChangLog files into the commit comments
should be enough.

For the big endian data testcase, you probably can find another person
who has the copyright and can help to send the patch.  I would suggest
that adding a simple big endian data test should be enough, since this
will double the test cases of the mapping symbol, but in fact they are
testing similar things.  However, I can add and update all the
testcases in the future patches, so you probably can only keep the
changes of the opcodes/riscv-dis.c.

Thanks
Nelson

On Thu, Nov 4, 2021 at 2:38 PM Guy Benyei via Binutils
<binutils@sourceware.org> wrote:
>
> Ping?
>
> -----Original Message-----
> From: Binutils <binutils-bounces+guybe=mellanox.com@sourceware.org> On Behalf Of Guy Benyei via Binutils
> Sent: Monday, November 1, 2021 9:55 AM
> To: binutils@sourceware.org
> Cc: Kito Cheng <kito.cheng@sifive.com>; Marcus Comstedt <marcus@mc.pp.se>
> Subject: [PATCH] RISC-V: Fix big endian disassembly of data.
>
> When disassembling big endian RISC-V binary, the data displayed with swapped endiannes. Binary generated from:
>
>         .word 0x01020304
>
> was disassembled as:
>
>         .word 0x04030201
>
> opcodes/
>         * riscv-dis.c: Consider endiannes when printing data
>
> gas/
>         * testsuite/gas/riscv/mapping-03b.d: check only for little endian
>         * testsuite/gas/riscv/mapping-04b.d: likewise
>         * testsuite/gas/riscv/mapping-norelax-03b.d: likewise
>         * testsuite/gas/riscv/mapping-norelax-04b.d: likewise
>         * testsuite/gas/riscv/mapping-03bbe.d: big endian data/code mapping test
>         * testsuite/gas/riscv/mapping-04bbe.d: likewise
>         * testsuite/gas/riscv/mapping-norelax-03bbe.d: likewise
>         * testsuite/gas/riscv/mapping-norelax-04bbe.d: likewise
>
> Conflicts:
>         gas/ChangeLog
>         opcodes/ChangeLog
> ---
>  gas/ChangeLog                                   | 10 ++++++++++
>  gas/testsuite/gas/riscv/mapping-03b.d           |  2 +-
>  gas/testsuite/gas/riscv/mapping-03bbe.d         | 24 ++++++++++++++++++++++++
>  gas/testsuite/gas/riscv/mapping-04b.d           |  2 +-
>  gas/testsuite/gas/riscv/mapping-04bbe.d         | 23 +++++++++++++++++++++++
>  gas/testsuite/gas/riscv/mapping-norelax-03b.d   |  2 +-
>  gas/testsuite/gas/riscv/mapping-norelax-03bbe.d | 25 +++++++++++++++++++++++++
>  gas/testsuite/gas/riscv/mapping-norelax-04b.d   |  2 +-
>  gas/testsuite/gas/riscv/mapping-norelax-04bbe.d | 24 ++++++++++++++++++++++++
>  opcodes/ChangeLog                               |  4 ++++
>  opcodes/riscv-dis.c                             | 11 ++++++-----
>  11 files changed, 120 insertions(+), 9 deletions(-)  create mode 100644 gas/testsuite/gas/riscv/mapping-03bbe.d
>  create mode 100644 gas/testsuite/gas/riscv/mapping-04bbe.d
>  create mode 100644 gas/testsuite/gas/riscv/mapping-norelax-03bbe.d
>  create mode 100644 gas/testsuite/gas/riscv/mapping-norelax-04bbe.d
>
> diff --git a/gas/ChangeLog b/gas/ChangeLog index 1133847..ddfd613 100644
> --- a/gas/ChangeLog
> +++ b/gas/ChangeLog
> @@ -1,3 +1,13 @@
> +2021-11-01  Guy Benyei <guybe@nvidia.com>
> +       * testsuite/gas/riscv/mapping-03b.d: check only for little endian
> +       * testsuite/gas/riscv/mapping-04b.d: likewise
> +       * testsuite/gas/riscv/mapping-norelax-03b.d: likewise
> +       * testsuite/gas/riscv/mapping-norelax-04b.d: likewise
> +       * testsuite/gas/riscv/mapping-03bbe.d: big endian data/code mapping test
> +       * testsuite/gas/riscv/mapping-04bbe.d: likewise
> +       * testsuite/gas/riscv/mapping-norelax-03bbe.d: likewise
> +       * testsuite/gas/riscv/mapping-norelax-04bbe.d: likewise
> +
>  2021-10-28  Markus Klein  <markus.klein@sma.de>
>
>         PR 28436
> diff --git a/gas/testsuite/gas/riscv/mapping-03b.d b/gas/testsuite/gas/riscv/mapping-03b.d
> index f4f6726..ecb3e31 100644
> --- a/gas/testsuite/gas/riscv/mapping-03b.d
> +++ b/gas/testsuite/gas/riscv/mapping-03b.d
> @@ -1,4 +1,4 @@
> -#as:
> +#as: -mlittle-endian
>  #source: mapping-03.s
>  #objdump: -d
>
> diff --git a/gas/testsuite/gas/riscv/mapping-03bbe.d b/gas/testsuite/gas/riscv/mapping-03bbe.d
> new file mode 100644
> index 0000000..9e5b429
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/mapping-03bbe.d
> @@ -0,0 +1,24 @@
> +#as: -mbig-endian
> +#source: mapping-03.s
> +#objdump: -d
> +
> +.*:[   ]+file format .*
> +
> +
> +Disassembly of section .text:
> +
> +0+000 <.text>:
> +[      ]+0:[   ]+00a50533[     ]+add[  ]+a0,a0,a0
> +[      ]+4:[   ]+00000000[     ]+.word[        ]+0x00000000
> +[      ]+8:[   ]+00000013[     ]+nop
> +[      ]+c:[   ]+00000013[     ]+nop
> +[      ]+10:[  ]+00000013[     ]+nop
> +[      ]+14:[  ]+00000001[     ]+.word[        ]+0x00000001
> +[      ]+18:[  ]+00b585b3[     ]+add[  ]+a1,a1,a1
> +[      ]+1c:[  ]+02000000[     ]+.word[        ]+0x02000000
> +[      ]+20:[  ]+03[   ]+.byte[        ]+0x03
> +[      ]+21:[  ]+00000013[     ]+nop
> +[      ]+25:[  ]+00000013[     ]+nop
> +[      ]+29:[  ]+00000013[     ]+nop
> +[      ]+2d:[  ]+00000005[     ]+.word[        ]+0x00000005
> +#...
> diff --git a/gas/testsuite/gas/riscv/mapping-04b.d b/gas/testsuite/gas/riscv/mapping-04b.d
> index 9735498..5616220 100644
> --- a/gas/testsuite/gas/riscv/mapping-04b.d
> +++ b/gas/testsuite/gas/riscv/mapping-04b.d
> @@ -1,4 +1,4 @@
> -#as:
> +#as: -mlittle-endian
>  #source: mapping-04.s
>  #objdump: -d
>
> diff --git a/gas/testsuite/gas/riscv/mapping-04bbe.d b/gas/testsuite/gas/riscv/mapping-04bbe.d
> new file mode 100644
> index 0000000..c5339c8
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/mapping-04bbe.d
> @@ -0,0 +1,23 @@
> +#as: -mbig-endian
> +#source: mapping-04.s
> +#objdump: -d
> +
> +.*:[   ]+file format .*
> +
> +
> +Disassembly of section .text:
> +
> +0+000 <.text>:
> +[      ]+0:[   ]+00001001[     ]+.word[        ]+0x00001001
> +[      ]+4:[   ]+00001001[     ]+.word[        ]+0x00001001
> +[      ]+8:[   ]+01000000[     ]+.word[        ]+0x01000000
> +[      ]+c:[   ]+00[   ]+.byte[        ]+0x00
> +[      ]+d:[   ]+00000013[     ]+nop
> +[      ]+11:[  ]+00a50533[     ]+add[  ]+a0,a0,a0
> +[      ]+15:[  ]+20022002[     ]+.word[        ]+0x20022002
> +[      ]+19:[  ]+20022002[     ]+.word[        ]+0x20022002
> +[      ]+1d:[  ]+2002[         ]+.short[       ]+0x2002
> +[      ]+1f:[  ]+00b585b3[     ]+add[  ]+a1,a1,a1
> +[      ]+23:[  ]+0000[         ]+unimp
> +[      ]+25:[  ]+0000[         ]+unimp
> +#...
> diff --git a/gas/testsuite/gas/riscv/mapping-norelax-03b.d b/gas/testsuite/gas/riscv/mapping-norelax-03b.d
> index ad88888..e65a922 100644
> --- a/gas/testsuite/gas/riscv/mapping-norelax-03b.d
> +++ b/gas/testsuite/gas/riscv/mapping-norelax-03b.d
> @@ -1,4 +1,4 @@
> -#as: -mno-relax
> +#as: -mno-relax -mlittle-endian
>  #source: mapping-03.s
>  #objdump: -d
>
> diff --git a/gas/testsuite/gas/riscv/mapping-norelax-03bbe.d b/gas/testsuite/gas/riscv/mapping-norelax-03bbe.d
> new file mode 100644
> index 0000000..c8520fd
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/mapping-norelax-03bbe.d
> @@ -0,0 +1,25 @@
> +#as: -mno-relax -mbig-endian
> +#source: mapping-03.s
> +#objdump: -d
> +
> +.*:[   ]+file format .*
> +
> +
> +Disassembly of section .text:
> +
> +0+000 <.text>:
> +[      ]+0:[   ]+00a50533[     ]+add[  ]+a0,a0,a0
> +[      ]+4:[   ]+00000000[     ]+.word[        ]+0x00000000
> +[      ]+8:[   ]+00000013[     ]+nop
> +[      ]+c:[   ]+00000013[     ]+nop
> +[      ]+10:[  ]+00000001[     ]+.word[        ]+0x00000001
> +[      ]+14:[  ]+00b585b3[     ]+add[  ]+a1,a1,a1
> +[      ]+18:[  ]+02000000[     ]+.word[        ]+0x02000000
> +[      ]+1c:[  ]+03[   ]+.byte[        ]+0x03
> +[      ]+1d:[  ]+00[   ]+.byte[        ]+0x00
> +[      ]+1e:[  ]+0001[         ]+nop
> +[      ]+20:[  ]+00000005[     ]+.word[        ]+0x00000005
> +[      ]+24:[  ]+00000013[     ]+nop
> +[      ]+28:[  ]+00000013[     ]+nop
> +[      ]+2c:[  ]+00000013[     ]+nop
> +#...
> diff --git a/gas/testsuite/gas/riscv/mapping-norelax-04b.d b/gas/testsuite/gas/riscv/mapping-norelax-04b.d
> index 824a898..00a9dbd 100644
> --- a/gas/testsuite/gas/riscv/mapping-norelax-04b.d
> +++ b/gas/testsuite/gas/riscv/mapping-norelax-04b.d
> @@ -1,4 +1,4 @@
> -#as: -mno-relax
> +#as: -mno-relax -mlittle-endian
>  #source: mapping-04.s
>  #objdump: -d
>
> diff --git a/gas/testsuite/gas/riscv/mapping-norelax-04bbe.d b/gas/testsuite/gas/riscv/mapping-norelax-04bbe.d
> new file mode 100644
> index 0000000..0b987fa
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/mapping-norelax-04bbe.d
> @@ -0,0 +1,24 @@
> +#as: -mno-relax -mbig-endian
> +#source: mapping-04.s
> +#objdump: -d
> +
> +.*:[   ]+file format .*
> +
> +
> +Disassembly of section .text:
> +
> +0+000 <.text>:
> +[      ]+0:[   ]+00001001[     ]+.word[        ]+0x00001001
> +[      ]+4:[   ]+00001001[     ]+.word[        ]+0x00001001
> +[      ]+8:[   ]+01000000[     ]+.word[        ]+0x01000000
> +[      ]+c:[   ]+00[   ]+.byte[        ]+0x00
> +[      ]+d:[   ]+00[   ]+.byte[        ]+0x00
> +[      ]+e:[   ]+0001[         ]+nop
> +[      ]+10:[  ]+00a50533[     ]+add[  ]+a0,a0,a0
> +[      ]+14:[  ]+20022002[     ]+.word[        ]+0x20022002
> +[      ]+18:[  ]+20022002[     ]+.word[        ]+0x20022002
> +[      ]+1c:[  ]+2002[         ]+.short[       ]+0x2002
> +[      ]+1e:[  ]+00b585b3[     ]+add[  ]+a1,a1,a1
> +[      ]+22:[  ]+0001[         ]+nop
> +[      ]+24:[  ]+00000013[     ]+nop
> +#...
> diff --git a/opcodes/ChangeLog b/opcodes/ChangeLog index f0e4b72..0f37316 100644
> --- a/opcodes/ChangeLog
> +++ b/opcodes/ChangeLog
> @@ -1,3 +1,7 @@
> +2021-11-01 Guy Benyei <guybe@nvidia.com>
> +
> +       * riscv-dis.c: Consider endiannes when printing data
> +
>  2021-10-27  Maciej W. Rozycki  <macro@embecosm.com>
>
>         * Makefile.am: Remove obsolete comment.
> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c index 1a09440..56af8e0 100644
> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -485,13 +485,9 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info)
>
>    insnlen = riscv_insn_length (word);
>
> -  /* RISC-V instructions are always little-endian.  */
> -  info->endian_code = BFD_ENDIAN_LITTLE;
> -
>    info->bytes_per_chunk = insnlen % 4 == 0 ? 4 : 2;
>    info->bytes_per_line = 8;
>    /* We don't support constant pools, so this must be code.  */
> -  info->display_endian = info->endian_code;
>    info->insn_info_valid = 1;
>    info->branch_delay_insns = 0;
>    info->data_size = 0;
> @@ -811,6 +807,9 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
>    else if (riscv_gpr_names == NULL)
>      set_default_riscv_dis_options ();
>
> +  /* RISC-V instructions are always little-endian.  */
> + info->endian_code = BFD_ENDIAN_LITTLE;
> +
>    mstate = riscv_search_mapping_symbol (memaddr, info);
>    /* Save the last mapping state.  */
>    last_map_state = mstate;
> @@ -822,6 +821,7 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
>        dump_size = riscv_data_length (memaddr, info);
>        info->bytes_per_chunk = dump_size;
>        riscv_disassembler = riscv_disassemble_data;
> +      info->display_endian = info->endian;
>      }
>    else
>      {
> @@ -835,6 +835,7 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
>        insn = (insn_t) bfd_getl16 (packet);
>        dump_size = riscv_insn_length (insn);
>        riscv_disassembler = riscv_disassemble_insn;
> +      info->display_endian = info->endian_code;
>      }
>
>    /* Fetch the instruction to dump.  */ @@ -844,7 +845,7 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
>        (*info->memory_error_func) (status, memaddr, info);
>        return status;
>      }
> -  insn = (insn_t) bfd_get_bits (packet, dump_size * 8, false);
> +  insn = (insn_t) bfd_get_bits (packet, dump_size * 8,
> + info->display_endian == BFD_ENDIAN_BIG);
>
>    return (*riscv_disassembler) (memaddr, insn, info);  }
> --
> 1.8.3.1
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] RISC-V: Fix big endian disassembly of data.
  2021-11-01  7:55 [PATCH] RISC-V: Fix big endian disassembly of data Guy Benyei
  2021-11-04  6:38 ` Guy Benyei
@ 2021-11-04  7:38 ` Marcus Comstedt
  2021-11-04 12:17 ` Maciej W. Rozycki
  2 siblings, 0 replies; 9+ messages in thread
From: Marcus Comstedt @ 2021-11-04  7:38 UTC (permalink / raw)
  To: Guy Benyei; +Cc: binutils, nelson.chu, Kito Cheng


Guy Benyei <guybe@nvidia.com> writes:

[...]
>    info->bytes_per_line = 8;
>    /* We don't support constant pools, so this must be code.  */
> -  info->display_endian = info->endian_code;
>    info->insn_info_valid = 1;
[...]

I guess the comment should also be removed, seeing as it 1) is no
longer true, and 2) refers to the line that was removed.   :-)


  // Marcus



^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] RISC-V: Fix big endian disassembly of data.
  2021-11-04  7:32   ` Nelson Chu
@ 2021-11-04  7:51     ` Guy Benyei
  2021-11-04  8:59       ` Nelson Chu
  0 siblings, 1 reply; 9+ messages in thread
From: Guy Benyei @ 2021-11-04  7:51 UTC (permalink / raw)
  To: Nelson Chu; +Cc: binutils, Kito Cheng, Marcus Comstedt

Hi,
Thanks for the answer.
After the NVIDIA/Mellanox merger, the Mellanox documents are still in place - I personally took the copyright assignment to our senior VP to sign (and he is still senior VP at NVIDIA). So please look up the Mellanox copyright assignment.

Thanks
         Guy

-----Original Message-----
From: Nelson Chu <nelson.chu@sifive.com> 
Sent: Thursday, November 4, 2021 9:32 AM
To: Guy Benyei <guybe@nvidia.com>
Cc: binutils@sourceware.org; Kito Cheng <kito.cheng@sifive.com>; Marcus Comstedt <marcus@mc.pp.se>
Subject: Re: [PATCH] RISC-V: Fix big endian disassembly of data.

External email: Use caution opening links or attachments


Hi Guy,

The change of opcodes/riscv-dis.c looks reasonable, and I'm OK with this change, thank you.  But unfortunately I cannot find either your or Nvidia's copyright assignment of binutil, so this may cause a problem.  However, the trivial 10-line change should be accepted without the copyright assignment, but I had heard that the rule is that - 10-line for all patches from the person total, not each patch individually.  I think the 11-line change of opcodes/riscv-dis.c should be OK under this rule, so I would suggest that,

1. Keep the 11-line change of opcodes/riscv-dis.c, or you can reduce the number of lines.
2. Moving the changes of ChangLog files into the commit comments should be enough.

For the big endian data testcase, you probably can find another person who has the copyright and can help to send the patch.  I would suggest that adding a simple big endian data test should be enough, since this will double the test cases of the mapping symbol, but in fact they are testing similar things.  However, I can add and update all the testcases in the future patches, so you probably can only keep the changes of the opcodes/riscv-dis.c.

Thanks
Nelson

On Thu, Nov 4, 2021 at 2:38 PM Guy Benyei via Binutils <binutils@sourceware.org> wrote:
>
> Ping?
>
> -----Original Message-----
> From: Binutils <binutils-bounces+guybe=mellanox.com@sourceware.org> On 
> Behalf Of Guy Benyei via Binutils
> Sent: Monday, November 1, 2021 9:55 AM
> To: binutils@sourceware.org
> Cc: Kito Cheng <kito.cheng@sifive.com>; Marcus Comstedt 
> <marcus@mc.pp.se>
> Subject: [PATCH] RISC-V: Fix big endian disassembly of data.
>
> When disassembling big endian RISC-V binary, the data displayed with swapped endiannes. Binary generated from:
>
>         .word 0x01020304
>
> was disassembled as:
>
>         .word 0x04030201
>
> opcodes/
>         * riscv-dis.c: Consider endiannes when printing data
>
> gas/
>         * testsuite/gas/riscv/mapping-03b.d: check only for little endian
>         * testsuite/gas/riscv/mapping-04b.d: likewise
>         * testsuite/gas/riscv/mapping-norelax-03b.d: likewise
>         * testsuite/gas/riscv/mapping-norelax-04b.d: likewise
>         * testsuite/gas/riscv/mapping-03bbe.d: big endian data/code mapping test
>         * testsuite/gas/riscv/mapping-04bbe.d: likewise
>         * testsuite/gas/riscv/mapping-norelax-03bbe.d: likewise
>         * testsuite/gas/riscv/mapping-norelax-04bbe.d: likewise
>
> Conflicts:
>         gas/ChangeLog
>         opcodes/ChangeLog
> ---
>  gas/ChangeLog                                   | 10 ++++++++++
>  gas/testsuite/gas/riscv/mapping-03b.d           |  2 +-
>  gas/testsuite/gas/riscv/mapping-03bbe.d         | 24 ++++++++++++++++++++++++
>  gas/testsuite/gas/riscv/mapping-04b.d           |  2 +-
>  gas/testsuite/gas/riscv/mapping-04bbe.d         | 23 +++++++++++++++++++++++
>  gas/testsuite/gas/riscv/mapping-norelax-03b.d   |  2 +-
>  gas/testsuite/gas/riscv/mapping-norelax-03bbe.d | 25 +++++++++++++++++++++++++
>  gas/testsuite/gas/riscv/mapping-norelax-04b.d   |  2 +-
>  gas/testsuite/gas/riscv/mapping-norelax-04bbe.d | 24 ++++++++++++++++++++++++
>  opcodes/ChangeLog                               |  4 ++++
>  opcodes/riscv-dis.c                             | 11 ++++++-----
>  11 files changed, 120 insertions(+), 9 deletions(-)  create mode 
> 100644 gas/testsuite/gas/riscv/mapping-03bbe.d
>  create mode 100644 gas/testsuite/gas/riscv/mapping-04bbe.d
>  create mode 100644 gas/testsuite/gas/riscv/mapping-norelax-03bbe.d
>  create mode 100644 gas/testsuite/gas/riscv/mapping-norelax-04bbe.d
>
> diff --git a/gas/ChangeLog b/gas/ChangeLog index 1133847..ddfd613 
> 100644
> --- a/gas/ChangeLog
> +++ b/gas/ChangeLog
> @@ -1,3 +1,13 @@
> +2021-11-01  Guy Benyei <guybe@nvidia.com>
> +       * testsuite/gas/riscv/mapping-03b.d: check only for little endian
> +       * testsuite/gas/riscv/mapping-04b.d: likewise
> +       * testsuite/gas/riscv/mapping-norelax-03b.d: likewise
> +       * testsuite/gas/riscv/mapping-norelax-04b.d: likewise
> +       * testsuite/gas/riscv/mapping-03bbe.d: big endian data/code mapping test
> +       * testsuite/gas/riscv/mapping-04bbe.d: likewise
> +       * testsuite/gas/riscv/mapping-norelax-03bbe.d: likewise
> +       * testsuite/gas/riscv/mapping-norelax-04bbe.d: likewise
> +
>  2021-10-28  Markus Klein  <markus.klein@sma.de>
>
>         PR 28436
> diff --git a/gas/testsuite/gas/riscv/mapping-03b.d 
> b/gas/testsuite/gas/riscv/mapping-03b.d
> index f4f6726..ecb3e31 100644
> --- a/gas/testsuite/gas/riscv/mapping-03b.d
> +++ b/gas/testsuite/gas/riscv/mapping-03b.d
> @@ -1,4 +1,4 @@
> -#as:
> +#as: -mlittle-endian
>  #source: mapping-03.s
>  #objdump: -d
>
> diff --git a/gas/testsuite/gas/riscv/mapping-03bbe.d 
> b/gas/testsuite/gas/riscv/mapping-03bbe.d
> new file mode 100644
> index 0000000..9e5b429
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/mapping-03bbe.d
> @@ -0,0 +1,24 @@
> +#as: -mbig-endian
> +#source: mapping-03.s
> +#objdump: -d
> +
> +.*:[   ]+file format .*
> +
> +
> +Disassembly of section .text:
> +
> +0+000 <.text>:
> +[      ]+0:[   ]+00a50533[     ]+add[  ]+a0,a0,a0
> +[      ]+4:[   ]+00000000[     ]+.word[        ]+0x00000000
> +[      ]+8:[   ]+00000013[     ]+nop
> +[      ]+c:[   ]+00000013[     ]+nop
> +[      ]+10:[  ]+00000013[     ]+nop
> +[      ]+14:[  ]+00000001[     ]+.word[        ]+0x00000001
> +[      ]+18:[  ]+00b585b3[     ]+add[  ]+a1,a1,a1
> +[      ]+1c:[  ]+02000000[     ]+.word[        ]+0x02000000
> +[      ]+20:[  ]+03[   ]+.byte[        ]+0x03
> +[      ]+21:[  ]+00000013[     ]+nop
> +[      ]+25:[  ]+00000013[     ]+nop
> +[      ]+29:[  ]+00000013[     ]+nop
> +[      ]+2d:[  ]+00000005[     ]+.word[        ]+0x00000005
> +#...
> diff --git a/gas/testsuite/gas/riscv/mapping-04b.d 
> b/gas/testsuite/gas/riscv/mapping-04b.d
> index 9735498..5616220 100644
> --- a/gas/testsuite/gas/riscv/mapping-04b.d
> +++ b/gas/testsuite/gas/riscv/mapping-04b.d
> @@ -1,4 +1,4 @@
> -#as:
> +#as: -mlittle-endian
>  #source: mapping-04.s
>  #objdump: -d
>
> diff --git a/gas/testsuite/gas/riscv/mapping-04bbe.d 
> b/gas/testsuite/gas/riscv/mapping-04bbe.d
> new file mode 100644
> index 0000000..c5339c8
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/mapping-04bbe.d
> @@ -0,0 +1,23 @@
> +#as: -mbig-endian
> +#source: mapping-04.s
> +#objdump: -d
> +
> +.*:[   ]+file format .*
> +
> +
> +Disassembly of section .text:
> +
> +0+000 <.text>:
> +[      ]+0:[   ]+00001001[     ]+.word[        ]+0x00001001
> +[      ]+4:[   ]+00001001[     ]+.word[        ]+0x00001001
> +[      ]+8:[   ]+01000000[     ]+.word[        ]+0x01000000
> +[      ]+c:[   ]+00[   ]+.byte[        ]+0x00
> +[      ]+d:[   ]+00000013[     ]+nop
> +[      ]+11:[  ]+00a50533[     ]+add[  ]+a0,a0,a0
> +[      ]+15:[  ]+20022002[     ]+.word[        ]+0x20022002
> +[      ]+19:[  ]+20022002[     ]+.word[        ]+0x20022002
> +[      ]+1d:[  ]+2002[         ]+.short[       ]+0x2002
> +[      ]+1f:[  ]+00b585b3[     ]+add[  ]+a1,a1,a1
> +[      ]+23:[  ]+0000[         ]+unimp
> +[      ]+25:[  ]+0000[         ]+unimp
> +#...
> diff --git a/gas/testsuite/gas/riscv/mapping-norelax-03b.d 
> b/gas/testsuite/gas/riscv/mapping-norelax-03b.d
> index ad88888..e65a922 100644
> --- a/gas/testsuite/gas/riscv/mapping-norelax-03b.d
> +++ b/gas/testsuite/gas/riscv/mapping-norelax-03b.d
> @@ -1,4 +1,4 @@
> -#as: -mno-relax
> +#as: -mno-relax -mlittle-endian
>  #source: mapping-03.s
>  #objdump: -d
>
> diff --git a/gas/testsuite/gas/riscv/mapping-norelax-03bbe.d 
> b/gas/testsuite/gas/riscv/mapping-norelax-03bbe.d
> new file mode 100644
> index 0000000..c8520fd
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/mapping-norelax-03bbe.d
> @@ -0,0 +1,25 @@
> +#as: -mno-relax -mbig-endian
> +#source: mapping-03.s
> +#objdump: -d
> +
> +.*:[   ]+file format .*
> +
> +
> +Disassembly of section .text:
> +
> +0+000 <.text>:
> +[      ]+0:[   ]+00a50533[     ]+add[  ]+a0,a0,a0
> +[      ]+4:[   ]+00000000[     ]+.word[        ]+0x00000000
> +[      ]+8:[   ]+00000013[     ]+nop
> +[      ]+c:[   ]+00000013[     ]+nop
> +[      ]+10:[  ]+00000001[     ]+.word[        ]+0x00000001
> +[      ]+14:[  ]+00b585b3[     ]+add[  ]+a1,a1,a1
> +[      ]+18:[  ]+02000000[     ]+.word[        ]+0x02000000
> +[      ]+1c:[  ]+03[   ]+.byte[        ]+0x03
> +[      ]+1d:[  ]+00[   ]+.byte[        ]+0x00
> +[      ]+1e:[  ]+0001[         ]+nop
> +[      ]+20:[  ]+00000005[     ]+.word[        ]+0x00000005
> +[      ]+24:[  ]+00000013[     ]+nop
> +[      ]+28:[  ]+00000013[     ]+nop
> +[      ]+2c:[  ]+00000013[     ]+nop
> +#...
> diff --git a/gas/testsuite/gas/riscv/mapping-norelax-04b.d 
> b/gas/testsuite/gas/riscv/mapping-norelax-04b.d
> index 824a898..00a9dbd 100644
> --- a/gas/testsuite/gas/riscv/mapping-norelax-04b.d
> +++ b/gas/testsuite/gas/riscv/mapping-norelax-04b.d
> @@ -1,4 +1,4 @@
> -#as: -mno-relax
> +#as: -mno-relax -mlittle-endian
>  #source: mapping-04.s
>  #objdump: -d
>
> diff --git a/gas/testsuite/gas/riscv/mapping-norelax-04bbe.d 
> b/gas/testsuite/gas/riscv/mapping-norelax-04bbe.d
> new file mode 100644
> index 0000000..0b987fa
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/mapping-norelax-04bbe.d
> @@ -0,0 +1,24 @@
> +#as: -mno-relax -mbig-endian
> +#source: mapping-04.s
> +#objdump: -d
> +
> +.*:[   ]+file format .*
> +
> +
> +Disassembly of section .text:
> +
> +0+000 <.text>:
> +[      ]+0:[   ]+00001001[     ]+.word[        ]+0x00001001
> +[      ]+4:[   ]+00001001[     ]+.word[        ]+0x00001001
> +[      ]+8:[   ]+01000000[     ]+.word[        ]+0x01000000
> +[      ]+c:[   ]+00[   ]+.byte[        ]+0x00
> +[      ]+d:[   ]+00[   ]+.byte[        ]+0x00
> +[      ]+e:[   ]+0001[         ]+nop
> +[      ]+10:[  ]+00a50533[     ]+add[  ]+a0,a0,a0
> +[      ]+14:[  ]+20022002[     ]+.word[        ]+0x20022002
> +[      ]+18:[  ]+20022002[     ]+.word[        ]+0x20022002
> +[      ]+1c:[  ]+2002[         ]+.short[       ]+0x2002
> +[      ]+1e:[  ]+00b585b3[     ]+add[  ]+a1,a1,a1
> +[      ]+22:[  ]+0001[         ]+nop
> +[      ]+24:[  ]+00000013[     ]+nop
> +#...
> diff --git a/opcodes/ChangeLog b/opcodes/ChangeLog index 
> f0e4b72..0f37316 100644
> --- a/opcodes/ChangeLog
> +++ b/opcodes/ChangeLog
> @@ -1,3 +1,7 @@
> +2021-11-01 Guy Benyei <guybe@nvidia.com>
> +
> +       * riscv-dis.c: Consider endiannes when printing data
> +
>  2021-10-27  Maciej W. Rozycki  <macro@embecosm.com>
>
>         * Makefile.am: Remove obsolete comment.
> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c index 
> 1a09440..56af8e0 100644
> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -485,13 +485,9 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t 
> word, disassemble_info *info)
>
>    insnlen = riscv_insn_length (word);
>
> -  /* RISC-V instructions are always little-endian.  */
> -  info->endian_code = BFD_ENDIAN_LITTLE;
> -
>    info->bytes_per_chunk = insnlen % 4 == 0 ? 4 : 2;
>    info->bytes_per_line = 8;
>    /* We don't support constant pools, so this must be code.  */
> -  info->display_endian = info->endian_code;
>    info->insn_info_valid = 1;
>    info->branch_delay_insns = 0;
>    info->data_size = 0;
> @@ -811,6 +807,9 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
>    else if (riscv_gpr_names == NULL)
>      set_default_riscv_dis_options ();
>
> +  /* RISC-V instructions are always little-endian.  */
> + info->endian_code = BFD_ENDIAN_LITTLE;
> +
>    mstate = riscv_search_mapping_symbol (memaddr, info);
>    /* Save the last mapping state.  */
>    last_map_state = mstate;
> @@ -822,6 +821,7 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
>        dump_size = riscv_data_length (memaddr, info);
>        info->bytes_per_chunk = dump_size;
>        riscv_disassembler = riscv_disassemble_data;
> +      info->display_endian = info->endian;
>      }
>    else
>      {
> @@ -835,6 +835,7 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
>        insn = (insn_t) bfd_getl16 (packet);
>        dump_size = riscv_insn_length (insn);
>        riscv_disassembler = riscv_disassemble_insn;
> +      info->display_endian = info->endian_code;
>      }
>
>    /* Fetch the instruction to dump.  */ @@ -844,7 +845,7 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
>        (*info->memory_error_func) (status, memaddr, info);
>        return status;
>      }
> -  insn = (insn_t) bfd_get_bits (packet, dump_size * 8, false);
> +  insn = (insn_t) bfd_get_bits (packet, dump_size * 8,
> + info->display_endian == BFD_ENDIAN_BIG);
>
>    return (*riscv_disassembler) (memaddr, insn, info);  }
> --
> 1.8.3.1
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] RISC-V: Fix big endian disassembly of data.
  2021-11-04  7:51     ` Guy Benyei
@ 2021-11-04  8:59       ` Nelson Chu
  2021-11-30 15:05         ` Guy Benyei
  0 siblings, 1 reply; 9+ messages in thread
From: Nelson Chu @ 2021-11-04  8:59 UTC (permalink / raw)
  To: Nick Clifton, Jim Wilson
  Cc: binutils, Kito Cheng, Marcus Comstedt, Guy Benyei

Hi Nick and Jim,

On Thu, Nov 4, 2021 at 3:51 PM Guy Benyei <guybe@nvidia.com> wrote:
>
> Hi,
> Thanks for the answer.
> After the NVIDIA/Mellanox merger, the Mellanox documents are still in place - I personally took the copyright assignment to our senior VP to sign (and he is still senior VP at NVIDIA). So please look up the Mellanox copyright assignment.

How do we usually deal with this situation?  I do see the copyright of
Mellanox, but I'm not sure what to do with the copyright after the
merger of the companies.

Thanks
Nelson

> -----Original Message-----
> From: Nelson Chu <nelson.chu@sifive.com>
> Sent: Thursday, November 4, 2021 9:32 AM
> To: Guy Benyei <guybe@nvidia.com>
> Cc: binutils@sourceware.org; Kito Cheng <kito.cheng@sifive.com>; Marcus Comstedt <marcus@mc.pp.se>
> Subject: Re: [PATCH] RISC-V: Fix big endian disassembly of data.
>
> External email: Use caution opening links or attachments
>
>
> Hi Guy,
>
> The change of opcodes/riscv-dis.c looks reasonable, and I'm OK with this change, thank you.  But unfortunately I cannot find either your or Nvidia's copyright assignment of binutil, so this may cause a problem.  However, the trivial 10-line change should be accepted without the copyright assignment, but I had heard that the rule is that - 10-line for all patches from the person total, not each patch individually.  I think the 11-line change of opcodes/riscv-dis.c should be OK under this rule, so I would suggest that,
>
> 1. Keep the 11-line change of opcodes/riscv-dis.c, or you can reduce the number of lines.
> 2. Moving the changes of ChangLog files into the commit comments should be enough.
>
> For the big endian data testcase, you probably can find another person who has the copyright and can help to send the patch.  I would suggest that adding a simple big endian data test should be enough, since this will double the test cases of the mapping symbol, but in fact they are testing similar things.  However, I can add and update all the testcases in the future patches, so you probably can only keep the changes of the opcodes/riscv-dis.c.
>
> Thanks
> Nelson
>
> On Thu, Nov 4, 2021 at 2:38 PM Guy Benyei via Binutils <binutils@sourceware.org> wrote:
> >
> > Ping?
> >
> > -----Original Message-----
> > From: Binutils <binutils-bounces+guybe=mellanox.com@sourceware.org> On
> > Behalf Of Guy Benyei via Binutils
> > Sent: Monday, November 1, 2021 9:55 AM
> > To: binutils@sourceware.org
> > Cc: Kito Cheng <kito.cheng@sifive.com>; Marcus Comstedt
> > <marcus@mc.pp.se>
> > Subject: [PATCH] RISC-V: Fix big endian disassembly of data.
> >
> > When disassembling big endian RISC-V binary, the data displayed with swapped endiannes. Binary generated from:
> >
> >         .word 0x01020304
> >
> > was disassembled as:
> >
> >         .word 0x04030201
> >
> > opcodes/
> >         * riscv-dis.c: Consider endiannes when printing data
> >
> > gas/
> >         * testsuite/gas/riscv/mapping-03b.d: check only for little endian
> >         * testsuite/gas/riscv/mapping-04b.d: likewise
> >         * testsuite/gas/riscv/mapping-norelax-03b.d: likewise
> >         * testsuite/gas/riscv/mapping-norelax-04b.d: likewise
> >         * testsuite/gas/riscv/mapping-03bbe.d: big endian data/code mapping test
> >         * testsuite/gas/riscv/mapping-04bbe.d: likewise
> >         * testsuite/gas/riscv/mapping-norelax-03bbe.d: likewise
> >         * testsuite/gas/riscv/mapping-norelax-04bbe.d: likewise
> >
> > Conflicts:
> >         gas/ChangeLog
> >         opcodes/ChangeLog
> > ---
> >  gas/ChangeLog                                   | 10 ++++++++++
> >  gas/testsuite/gas/riscv/mapping-03b.d           |  2 +-
> >  gas/testsuite/gas/riscv/mapping-03bbe.d         | 24 ++++++++++++++++++++++++
> >  gas/testsuite/gas/riscv/mapping-04b.d           |  2 +-
> >  gas/testsuite/gas/riscv/mapping-04bbe.d         | 23 +++++++++++++++++++++++
> >  gas/testsuite/gas/riscv/mapping-norelax-03b.d   |  2 +-
> >  gas/testsuite/gas/riscv/mapping-norelax-03bbe.d | 25 +++++++++++++++++++++++++
> >  gas/testsuite/gas/riscv/mapping-norelax-04b.d   |  2 +-
> >  gas/testsuite/gas/riscv/mapping-norelax-04bbe.d | 24 ++++++++++++++++++++++++
> >  opcodes/ChangeLog                               |  4 ++++
> >  opcodes/riscv-dis.c                             | 11 ++++++-----
> >  11 files changed, 120 insertions(+), 9 deletions(-)  create mode
> > 100644 gas/testsuite/gas/riscv/mapping-03bbe.d
> >  create mode 100644 gas/testsuite/gas/riscv/mapping-04bbe.d
> >  create mode 100644 gas/testsuite/gas/riscv/mapping-norelax-03bbe.d
> >  create mode 100644 gas/testsuite/gas/riscv/mapping-norelax-04bbe.d
> >
> > diff --git a/gas/ChangeLog b/gas/ChangeLog index 1133847..ddfd613
> > 100644
> > --- a/gas/ChangeLog
> > +++ b/gas/ChangeLog
> > @@ -1,3 +1,13 @@
> > +2021-11-01  Guy Benyei <guybe@nvidia.com>
> > +       * testsuite/gas/riscv/mapping-03b.d: check only for little endian
> > +       * testsuite/gas/riscv/mapping-04b.d: likewise
> > +       * testsuite/gas/riscv/mapping-norelax-03b.d: likewise
> > +       * testsuite/gas/riscv/mapping-norelax-04b.d: likewise
> > +       * testsuite/gas/riscv/mapping-03bbe.d: big endian data/code mapping test
> > +       * testsuite/gas/riscv/mapping-04bbe.d: likewise
> > +       * testsuite/gas/riscv/mapping-norelax-03bbe.d: likewise
> > +       * testsuite/gas/riscv/mapping-norelax-04bbe.d: likewise
> > +
> >  2021-10-28  Markus Klein  <markus.klein@sma.de>
> >
> >         PR 28436
> > diff --git a/gas/testsuite/gas/riscv/mapping-03b.d
> > b/gas/testsuite/gas/riscv/mapping-03b.d
> > index f4f6726..ecb3e31 100644
> > --- a/gas/testsuite/gas/riscv/mapping-03b.d
> > +++ b/gas/testsuite/gas/riscv/mapping-03b.d
> > @@ -1,4 +1,4 @@
> > -#as:
> > +#as: -mlittle-endian
> >  #source: mapping-03.s
> >  #objdump: -d
> >
> > diff --git a/gas/testsuite/gas/riscv/mapping-03bbe.d
> > b/gas/testsuite/gas/riscv/mapping-03bbe.d
> > new file mode 100644
> > index 0000000..9e5b429
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/mapping-03bbe.d
> > @@ -0,0 +1,24 @@
> > +#as: -mbig-endian
> > +#source: mapping-03.s
> > +#objdump: -d
> > +
> > +.*:[   ]+file format .*
> > +
> > +
> > +Disassembly of section .text:
> > +
> > +0+000 <.text>:
> > +[      ]+0:[   ]+00a50533[     ]+add[  ]+a0,a0,a0
> > +[      ]+4:[   ]+00000000[     ]+.word[        ]+0x00000000
> > +[      ]+8:[   ]+00000013[     ]+nop
> > +[      ]+c:[   ]+00000013[     ]+nop
> > +[      ]+10:[  ]+00000013[     ]+nop
> > +[      ]+14:[  ]+00000001[     ]+.word[        ]+0x00000001
> > +[      ]+18:[  ]+00b585b3[     ]+add[  ]+a1,a1,a1
> > +[      ]+1c:[  ]+02000000[     ]+.word[        ]+0x02000000
> > +[      ]+20:[  ]+03[   ]+.byte[        ]+0x03
> > +[      ]+21:[  ]+00000013[     ]+nop
> > +[      ]+25:[  ]+00000013[     ]+nop
> > +[      ]+29:[  ]+00000013[     ]+nop
> > +[      ]+2d:[  ]+00000005[     ]+.word[        ]+0x00000005
> > +#...
> > diff --git a/gas/testsuite/gas/riscv/mapping-04b.d
> > b/gas/testsuite/gas/riscv/mapping-04b.d
> > index 9735498..5616220 100644
> > --- a/gas/testsuite/gas/riscv/mapping-04b.d
> > +++ b/gas/testsuite/gas/riscv/mapping-04b.d
> > @@ -1,4 +1,4 @@
> > -#as:
> > +#as: -mlittle-endian
> >  #source: mapping-04.s
> >  #objdump: -d
> >
> > diff --git a/gas/testsuite/gas/riscv/mapping-04bbe.d
> > b/gas/testsuite/gas/riscv/mapping-04bbe.d
> > new file mode 100644
> > index 0000000..c5339c8
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/mapping-04bbe.d
> > @@ -0,0 +1,23 @@
> > +#as: -mbig-endian
> > +#source: mapping-04.s
> > +#objdump: -d
> > +
> > +.*:[   ]+file format .*
> > +
> > +
> > +Disassembly of section .text:
> > +
> > +0+000 <.text>:
> > +[      ]+0:[   ]+00001001[     ]+.word[        ]+0x00001001
> > +[      ]+4:[   ]+00001001[     ]+.word[        ]+0x00001001
> > +[      ]+8:[   ]+01000000[     ]+.word[        ]+0x01000000
> > +[      ]+c:[   ]+00[   ]+.byte[        ]+0x00
> > +[      ]+d:[   ]+00000013[     ]+nop
> > +[      ]+11:[  ]+00a50533[     ]+add[  ]+a0,a0,a0
> > +[      ]+15:[  ]+20022002[     ]+.word[        ]+0x20022002
> > +[      ]+19:[  ]+20022002[     ]+.word[        ]+0x20022002
> > +[      ]+1d:[  ]+2002[         ]+.short[       ]+0x2002
> > +[      ]+1f:[  ]+00b585b3[     ]+add[  ]+a1,a1,a1
> > +[      ]+23:[  ]+0000[         ]+unimp
> > +[      ]+25:[  ]+0000[         ]+unimp
> > +#...
> > diff --git a/gas/testsuite/gas/riscv/mapping-norelax-03b.d
> > b/gas/testsuite/gas/riscv/mapping-norelax-03b.d
> > index ad88888..e65a922 100644
> > --- a/gas/testsuite/gas/riscv/mapping-norelax-03b.d
> > +++ b/gas/testsuite/gas/riscv/mapping-norelax-03b.d
> > @@ -1,4 +1,4 @@
> > -#as: -mno-relax
> > +#as: -mno-relax -mlittle-endian
> >  #source: mapping-03.s
> >  #objdump: -d
> >
> > diff --git a/gas/testsuite/gas/riscv/mapping-norelax-03bbe.d
> > b/gas/testsuite/gas/riscv/mapping-norelax-03bbe.d
> > new file mode 100644
> > index 0000000..c8520fd
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/mapping-norelax-03bbe.d
> > @@ -0,0 +1,25 @@
> > +#as: -mno-relax -mbig-endian
> > +#source: mapping-03.s
> > +#objdump: -d
> > +
> > +.*:[   ]+file format .*
> > +
> > +
> > +Disassembly of section .text:
> > +
> > +0+000 <.text>:
> > +[      ]+0:[   ]+00a50533[     ]+add[  ]+a0,a0,a0
> > +[      ]+4:[   ]+00000000[     ]+.word[        ]+0x00000000
> > +[      ]+8:[   ]+00000013[     ]+nop
> > +[      ]+c:[   ]+00000013[     ]+nop
> > +[      ]+10:[  ]+00000001[     ]+.word[        ]+0x00000001
> > +[      ]+14:[  ]+00b585b3[     ]+add[  ]+a1,a1,a1
> > +[      ]+18:[  ]+02000000[     ]+.word[        ]+0x02000000
> > +[      ]+1c:[  ]+03[   ]+.byte[        ]+0x03
> > +[      ]+1d:[  ]+00[   ]+.byte[        ]+0x00
> > +[      ]+1e:[  ]+0001[         ]+nop
> > +[      ]+20:[  ]+00000005[     ]+.word[        ]+0x00000005
> > +[      ]+24:[  ]+00000013[     ]+nop
> > +[      ]+28:[  ]+00000013[     ]+nop
> > +[      ]+2c:[  ]+00000013[     ]+nop
> > +#...
> > diff --git a/gas/testsuite/gas/riscv/mapping-norelax-04b.d
> > b/gas/testsuite/gas/riscv/mapping-norelax-04b.d
> > index 824a898..00a9dbd 100644
> > --- a/gas/testsuite/gas/riscv/mapping-norelax-04b.d
> > +++ b/gas/testsuite/gas/riscv/mapping-norelax-04b.d
> > @@ -1,4 +1,4 @@
> > -#as: -mno-relax
> > +#as: -mno-relax -mlittle-endian
> >  #source: mapping-04.s
> >  #objdump: -d
> >
> > diff --git a/gas/testsuite/gas/riscv/mapping-norelax-04bbe.d
> > b/gas/testsuite/gas/riscv/mapping-norelax-04bbe.d
> > new file mode 100644
> > index 0000000..0b987fa
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/mapping-norelax-04bbe.d
> > @@ -0,0 +1,24 @@
> > +#as: -mno-relax -mbig-endian
> > +#source: mapping-04.s
> > +#objdump: -d
> > +
> > +.*:[   ]+file format .*
> > +
> > +
> > +Disassembly of section .text:
> > +
> > +0+000 <.text>:
> > +[      ]+0:[   ]+00001001[     ]+.word[        ]+0x00001001
> > +[      ]+4:[   ]+00001001[     ]+.word[        ]+0x00001001
> > +[      ]+8:[   ]+01000000[     ]+.word[        ]+0x01000000
> > +[      ]+c:[   ]+00[   ]+.byte[        ]+0x00
> > +[      ]+d:[   ]+00[   ]+.byte[        ]+0x00
> > +[      ]+e:[   ]+0001[         ]+nop
> > +[      ]+10:[  ]+00a50533[     ]+add[  ]+a0,a0,a0
> > +[      ]+14:[  ]+20022002[     ]+.word[        ]+0x20022002
> > +[      ]+18:[  ]+20022002[     ]+.word[        ]+0x20022002
> > +[      ]+1c:[  ]+2002[         ]+.short[       ]+0x2002
> > +[      ]+1e:[  ]+00b585b3[     ]+add[  ]+a1,a1,a1
> > +[      ]+22:[  ]+0001[         ]+nop
> > +[      ]+24:[  ]+00000013[     ]+nop
> > +#...
> > diff --git a/opcodes/ChangeLog b/opcodes/ChangeLog index
> > f0e4b72..0f37316 100644
> > --- a/opcodes/ChangeLog
> > +++ b/opcodes/ChangeLog
> > @@ -1,3 +1,7 @@
> > +2021-11-01 Guy Benyei <guybe@nvidia.com>
> > +
> > +       * riscv-dis.c: Consider endiannes when printing data
> > +
> >  2021-10-27  Maciej W. Rozycki  <macro@embecosm.com>
> >
> >         * Makefile.am: Remove obsolete comment.
> > diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c index
> > 1a09440..56af8e0 100644
> > --- a/opcodes/riscv-dis.c
> > +++ b/opcodes/riscv-dis.c
> > @@ -485,13 +485,9 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t
> > word, disassemble_info *info)
> >
> >    insnlen = riscv_insn_length (word);
> >
> > -  /* RISC-V instructions are always little-endian.  */
> > -  info->endian_code = BFD_ENDIAN_LITTLE;
> > -
> >    info->bytes_per_chunk = insnlen % 4 == 0 ? 4 : 2;
> >    info->bytes_per_line = 8;
> >    /* We don't support constant pools, so this must be code.  */
> > -  info->display_endian = info->endian_code;
> >    info->insn_info_valid = 1;
> >    info->branch_delay_insns = 0;
> >    info->data_size = 0;
> > @@ -811,6 +807,9 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
> >    else if (riscv_gpr_names == NULL)
> >      set_default_riscv_dis_options ();
> >
> > +  /* RISC-V instructions are always little-endian.  */
> > + info->endian_code = BFD_ENDIAN_LITTLE;
> > +
> >    mstate = riscv_search_mapping_symbol (memaddr, info);
> >    /* Save the last mapping state.  */
> >    last_map_state = mstate;
> > @@ -822,6 +821,7 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
> >        dump_size = riscv_data_length (memaddr, info);
> >        info->bytes_per_chunk = dump_size;
> >        riscv_disassembler = riscv_disassemble_data;
> > +      info->display_endian = info->endian;
> >      }
> >    else
> >      {
> > @@ -835,6 +835,7 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
> >        insn = (insn_t) bfd_getl16 (packet);
> >        dump_size = riscv_insn_length (insn);
> >        riscv_disassembler = riscv_disassemble_insn;
> > +      info->display_endian = info->endian_code;
> >      }
> >
> >    /* Fetch the instruction to dump.  */ @@ -844,7 +845,7 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
> >        (*info->memory_error_func) (status, memaddr, info);
> >        return status;
> >      }
> > -  insn = (insn_t) bfd_get_bits (packet, dump_size * 8, false);
> > +  insn = (insn_t) bfd_get_bits (packet, dump_size * 8,
> > + info->display_endian == BFD_ENDIAN_BIG);
> >
> >    return (*riscv_disassembler) (memaddr, insn, info);  }
> > --
> > 1.8.3.1
> >

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] RISC-V: Fix big endian disassembly of data.
  2021-11-01  7:55 [PATCH] RISC-V: Fix big endian disassembly of data Guy Benyei
  2021-11-04  6:38 ` Guy Benyei
  2021-11-04  7:38 ` Marcus Comstedt
@ 2021-11-04 12:17 ` Maciej W. Rozycki
  2021-11-04 13:43   ` Guy Benyei
  2 siblings, 1 reply; 9+ messages in thread
From: Maciej W. Rozycki @ 2021-11-04 12:17 UTC (permalink / raw)
  To: Guy Benyei; +Cc: binutils, Kito Cheng, Marcus Comstedt

On Mon, 1 Nov 2021, Guy Benyei via Binutils wrote:

> opcodes/
> 	* riscv-dis.c: Consider endiannes when printing data
> 
> gas/
> 	* testsuite/gas/riscv/mapping-03b.d: check only for little endian
> 	* testsuite/gas/riscv/mapping-04b.d: likewise
> 	* testsuite/gas/riscv/mapping-norelax-03b.d: likewise
> 	* testsuite/gas/riscv/mapping-norelax-04b.d: likewise
> 	* testsuite/gas/riscv/mapping-03bbe.d: big endian data/code mapping test
> 	* testsuite/gas/riscv/mapping-04bbe.d: likewise
> 	* testsuite/gas/riscv/mapping-norelax-03bbe.d: likewise
> 	* testsuite/gas/riscv/mapping-norelax-04bbe.d: likewise

 You need to improve these entries a bit, by making them proper sentences 
starting with a capital letter and ending with a full stop, e.g.:

	* testsuite/gas/riscv/mapping-03b.d: Check only for little endian.

(I think "endianness" would be more correct here too).  Also do not post 
actual ChangeLog diffs as that makes a patch not apply anymore very soon 
-- the committer will make them from the template given in the commit 
description.

 Given that these test cases cover a disassembler rather than GAS feature 
please consider putting them in the binutils testsuite instead, that is in 
binutils/testsuite/binutils-all/riscv/.

  Maciej

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] RISC-V: Fix big endian disassembly of data.
  2021-11-04 12:17 ` Maciej W. Rozycki
@ 2021-11-04 13:43   ` Guy Benyei
  0 siblings, 0 replies; 9+ messages in thread
From: Guy Benyei @ 2021-11-04 13:43 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: binutils, Kito Cheng, Marcus Comstedt

Thanks, I'll send an updated patch.
      Guy

-----Original Message-----
From: Maciej W. Rozycki <macro@embecosm.com> 
Sent: Thursday, November 4, 2021 2:18 PM
To: Guy Benyei <guybe@nvidia.com>
Cc: binutils@sourceware.org; Kito Cheng <kito.cheng@sifive.com>; Marcus Comstedt <marcus@mc.pp.se>
Subject: Re: [PATCH] RISC-V: Fix big endian disassembly of data.

External email: Use caution opening links or attachments


On Mon, 1 Nov 2021, Guy Benyei via Binutils wrote:

> opcodes/
>       * riscv-dis.c: Consider endiannes when printing data
>
> gas/
>       * testsuite/gas/riscv/mapping-03b.d: check only for little endian
>       * testsuite/gas/riscv/mapping-04b.d: likewise
>       * testsuite/gas/riscv/mapping-norelax-03b.d: likewise
>       * testsuite/gas/riscv/mapping-norelax-04b.d: likewise
>       * testsuite/gas/riscv/mapping-03bbe.d: big endian data/code mapping test
>       * testsuite/gas/riscv/mapping-04bbe.d: likewise
>       * testsuite/gas/riscv/mapping-norelax-03bbe.d: likewise
>       * testsuite/gas/riscv/mapping-norelax-04bbe.d: likewise

 You need to improve these entries a bit, by making them proper sentences starting with a capital letter and ending with a full stop, e.g.:

        * testsuite/gas/riscv/mapping-03b.d: Check only for little endian.

(I think "endianness" would be more correct here too).  Also do not post actual ChangeLog diffs as that makes a patch not apply anymore very soon
-- the committer will make them from the template given in the commit description.

 Given that these test cases cover a disassembler rather than GAS feature please consider putting them in the binutils testsuite instead, that is in binutils/testsuite/binutils-all/riscv/.

  Maciej

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] RISC-V: Fix big endian disassembly of data.
  2021-11-04  8:59       ` Nelson Chu
@ 2021-11-30 15:05         ` Guy Benyei
  0 siblings, 0 replies; 9+ messages in thread
From: Guy Benyei @ 2021-11-30 15:05 UTC (permalink / raw)
  To: Nelson Chu, Nick Clifton, Jim Wilson
  Cc: binutils, Kito Cheng, Marcus Comstedt

A month passed by without answer.
I also tried to contact assign@gnu.org, but no luck there. Does anyone still handle the FSF legal stuff?
Our legal department reviewed the old contract, and stated, that our old contracts are still hold, as Mellanox still exists as a company, as one of Nvidia's subsidiaries.
We're still interested in contributing code - but seems like there is no way we can do that. Not getting answers from assign@gnu.org means that even re-signing the copyright assignment would be impossible.

Thanks
        Guy

-----Original Message-----
From: Nelson Chu <nelson.chu@sifive.com> 
Sent: Thursday, November 4, 2021 10:59 AM
To: Nick Clifton <nickc@redhat.com>; Jim Wilson <jimw@sifive.com>
Cc: binutils@sourceware.org; Kito Cheng <kito.cheng@sifive.com>; Marcus Comstedt <marcus@mc.pp.se>; Guy Benyei <guybe@nvidia.com>
Subject: Re: [PATCH] RISC-V: Fix big endian disassembly of data.

External email: Use caution opening links or attachments


Hi Nick and Jim,

On Thu, Nov 4, 2021 at 3:51 PM Guy Benyei <guybe@nvidia.com> wrote:
>
> Hi,
> Thanks for the answer.
> After the NVIDIA/Mellanox merger, the Mellanox documents are still in place - I personally took the copyright assignment to our senior VP to sign (and he is still senior VP at NVIDIA). So please look up the Mellanox copyright assignment.

How do we usually deal with this situation?  I do see the copyright of Mellanox, but I'm not sure what to do with the copyright after the merger of the companies.

Thanks
Nelson

> -----Original Message-----
> From: Nelson Chu <nelson.chu@sifive.com>
> Sent: Thursday, November 4, 2021 9:32 AM
> To: Guy Benyei <guybe@nvidia.com>
> Cc: binutils@sourceware.org; Kito Cheng <kito.cheng@sifive.com>; 
> Marcus Comstedt <marcus@mc.pp.se>
> Subject: Re: [PATCH] RISC-V: Fix big endian disassembly of data.
>
> External email: Use caution opening links or attachments
>
>
> Hi Guy,
>
> The change of opcodes/riscv-dis.c looks reasonable, and I'm OK with 
> this change, thank you.  But unfortunately I cannot find either your 
> or Nvidia's copyright assignment of binutil, so this may cause a 
> problem.  However, the trivial 10-line change should be accepted 
> without the copyright assignment, but I had heard that the rule is 
> that - 10-line for all patches from the person total, not each patch 
> individually.  I think the 11-line change of opcodes/riscv-dis.c 
> should be OK under this rule, so I would suggest that,
>
> 1. Keep the 11-line change of opcodes/riscv-dis.c, or you can reduce the number of lines.
> 2. Moving the changes of ChangLog files into the commit comments should be enough.
>
> For the big endian data testcase, you probably can find another person who has the copyright and can help to send the patch.  I would suggest that adding a simple big endian data test should be enough, since this will double the test cases of the mapping symbol, but in fact they are testing similar things.  However, I can add and update all the testcases in the future patches, so you probably can only keep the changes of the opcodes/riscv-dis.c.
>
> Thanks
> Nelson
>
> On Thu, Nov 4, 2021 at 2:38 PM Guy Benyei via Binutils <binutils@sourceware.org> wrote:
> >
> > Ping?
> >
> > -----Original Message-----
> > From: Binutils <binutils-bounces+guybe=mellanox.com@sourceware.org> 
> > On Behalf Of Guy Benyei via Binutils
> > Sent: Monday, November 1, 2021 9:55 AM
> > To: binutils@sourceware.org
> > Cc: Kito Cheng <kito.cheng@sifive.com>; Marcus Comstedt 
> > <marcus@mc.pp.se>
> > Subject: [PATCH] RISC-V: Fix big endian disassembly of data.
> >
> > When disassembling big endian RISC-V binary, the data displayed with swapped endiannes. Binary generated from:
> >
> >         .word 0x01020304
> >
> > was disassembled as:
> >
> >         .word 0x04030201
> >
> > opcodes/
> >         * riscv-dis.c: Consider endiannes when printing data
> >
> > gas/
> >         * testsuite/gas/riscv/mapping-03b.d: check only for little endian
> >         * testsuite/gas/riscv/mapping-04b.d: likewise
> >         * testsuite/gas/riscv/mapping-norelax-03b.d: likewise
> >         * testsuite/gas/riscv/mapping-norelax-04b.d: likewise
> >         * testsuite/gas/riscv/mapping-03bbe.d: big endian data/code mapping test
> >         * testsuite/gas/riscv/mapping-04bbe.d: likewise
> >         * testsuite/gas/riscv/mapping-norelax-03bbe.d: likewise
> >         * testsuite/gas/riscv/mapping-norelax-04bbe.d: likewise
> >
> > Conflicts:
> >         gas/ChangeLog
> >         opcodes/ChangeLog
> > ---
> >  gas/ChangeLog                                   | 10 ++++++++++
> >  gas/testsuite/gas/riscv/mapping-03b.d           |  2 +-
> >  gas/testsuite/gas/riscv/mapping-03bbe.d         | 24 ++++++++++++++++++++++++
> >  gas/testsuite/gas/riscv/mapping-04b.d           |  2 +-
> >  gas/testsuite/gas/riscv/mapping-04bbe.d         | 23 +++++++++++++++++++++++
> >  gas/testsuite/gas/riscv/mapping-norelax-03b.d   |  2 +-
> >  gas/testsuite/gas/riscv/mapping-norelax-03bbe.d | 25 +++++++++++++++++++++++++
> >  gas/testsuite/gas/riscv/mapping-norelax-04b.d   |  2 +-
> >  gas/testsuite/gas/riscv/mapping-norelax-04bbe.d | 24 ++++++++++++++++++++++++
> >  opcodes/ChangeLog                               |  4 ++++
> >  opcodes/riscv-dis.c                             | 11 ++++++-----
> >  11 files changed, 120 insertions(+), 9 deletions(-)  create mode
> > 100644 gas/testsuite/gas/riscv/mapping-03bbe.d
> >  create mode 100644 gas/testsuite/gas/riscv/mapping-04bbe.d
> >  create mode 100644 gas/testsuite/gas/riscv/mapping-norelax-03bbe.d
> >  create mode 100644 gas/testsuite/gas/riscv/mapping-norelax-04bbe.d
> >
> > diff --git a/gas/ChangeLog b/gas/ChangeLog index 1133847..ddfd613
> > 100644
> > --- a/gas/ChangeLog
> > +++ b/gas/ChangeLog
> > @@ -1,3 +1,13 @@
> > +2021-11-01  Guy Benyei <guybe@nvidia.com>
> > +       * testsuite/gas/riscv/mapping-03b.d: check only for little endian
> > +       * testsuite/gas/riscv/mapping-04b.d: likewise
> > +       * testsuite/gas/riscv/mapping-norelax-03b.d: likewise
> > +       * testsuite/gas/riscv/mapping-norelax-04b.d: likewise
> > +       * testsuite/gas/riscv/mapping-03bbe.d: big endian data/code mapping test
> > +       * testsuite/gas/riscv/mapping-04bbe.d: likewise
> > +       * testsuite/gas/riscv/mapping-norelax-03bbe.d: likewise
> > +       * testsuite/gas/riscv/mapping-norelax-04bbe.d: likewise
> > +
> >  2021-10-28  Markus Klein  <markus.klein@sma.de>
> >
> >         PR 28436
> > diff --git a/gas/testsuite/gas/riscv/mapping-03b.d
> > b/gas/testsuite/gas/riscv/mapping-03b.d
> > index f4f6726..ecb3e31 100644
> > --- a/gas/testsuite/gas/riscv/mapping-03b.d
> > +++ b/gas/testsuite/gas/riscv/mapping-03b.d
> > @@ -1,4 +1,4 @@
> > -#as:
> > +#as: -mlittle-endian
> >  #source: mapping-03.s
> >  #objdump: -d
> >
> > diff --git a/gas/testsuite/gas/riscv/mapping-03bbe.d
> > b/gas/testsuite/gas/riscv/mapping-03bbe.d
> > new file mode 100644
> > index 0000000..9e5b429
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/mapping-03bbe.d
> > @@ -0,0 +1,24 @@
> > +#as: -mbig-endian
> > +#source: mapping-03.s
> > +#objdump: -d
> > +
> > +.*:[   ]+file format .*
> > +
> > +
> > +Disassembly of section .text:
> > +
> > +0+000 <.text>:
> > +[      ]+0:[   ]+00a50533[     ]+add[  ]+a0,a0,a0
> > +[      ]+4:[   ]+00000000[     ]+.word[        ]+0x00000000
> > +[      ]+8:[   ]+00000013[     ]+nop
> > +[      ]+c:[   ]+00000013[     ]+nop
> > +[      ]+10:[  ]+00000013[     ]+nop
> > +[      ]+14:[  ]+00000001[     ]+.word[        ]+0x00000001
> > +[      ]+18:[  ]+00b585b3[     ]+add[  ]+a1,a1,a1
> > +[      ]+1c:[  ]+02000000[     ]+.word[        ]+0x02000000
> > +[      ]+20:[  ]+03[   ]+.byte[        ]+0x03
> > +[      ]+21:[  ]+00000013[     ]+nop
> > +[      ]+25:[  ]+00000013[     ]+nop
> > +[      ]+29:[  ]+00000013[     ]+nop
> > +[      ]+2d:[  ]+00000005[     ]+.word[        ]+0x00000005
> > +#...
> > diff --git a/gas/testsuite/gas/riscv/mapping-04b.d
> > b/gas/testsuite/gas/riscv/mapping-04b.d
> > index 9735498..5616220 100644
> > --- a/gas/testsuite/gas/riscv/mapping-04b.d
> > +++ b/gas/testsuite/gas/riscv/mapping-04b.d
> > @@ -1,4 +1,4 @@
> > -#as:
> > +#as: -mlittle-endian
> >  #source: mapping-04.s
> >  #objdump: -d
> >
> > diff --git a/gas/testsuite/gas/riscv/mapping-04bbe.d
> > b/gas/testsuite/gas/riscv/mapping-04bbe.d
> > new file mode 100644
> > index 0000000..c5339c8
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/mapping-04bbe.d
> > @@ -0,0 +1,23 @@
> > +#as: -mbig-endian
> > +#source: mapping-04.s
> > +#objdump: -d
> > +
> > +.*:[   ]+file format .*
> > +
> > +
> > +Disassembly of section .text:
> > +
> > +0+000 <.text>:
> > +[      ]+0:[   ]+00001001[     ]+.word[        ]+0x00001001
> > +[      ]+4:[   ]+00001001[     ]+.word[        ]+0x00001001
> > +[      ]+8:[   ]+01000000[     ]+.word[        ]+0x01000000
> > +[      ]+c:[   ]+00[   ]+.byte[        ]+0x00
> > +[      ]+d:[   ]+00000013[     ]+nop
> > +[      ]+11:[  ]+00a50533[     ]+add[  ]+a0,a0,a0
> > +[      ]+15:[  ]+20022002[     ]+.word[        ]+0x20022002
> > +[      ]+19:[  ]+20022002[     ]+.word[        ]+0x20022002
> > +[      ]+1d:[  ]+2002[         ]+.short[       ]+0x2002
> > +[      ]+1f:[  ]+00b585b3[     ]+add[  ]+a1,a1,a1
> > +[      ]+23:[  ]+0000[         ]+unimp
> > +[      ]+25:[  ]+0000[         ]+unimp
> > +#...
> > diff --git a/gas/testsuite/gas/riscv/mapping-norelax-03b.d
> > b/gas/testsuite/gas/riscv/mapping-norelax-03b.d
> > index ad88888..e65a922 100644
> > --- a/gas/testsuite/gas/riscv/mapping-norelax-03b.d
> > +++ b/gas/testsuite/gas/riscv/mapping-norelax-03b.d
> > @@ -1,4 +1,4 @@
> > -#as: -mno-relax
> > +#as: -mno-relax -mlittle-endian
> >  #source: mapping-03.s
> >  #objdump: -d
> >
> > diff --git a/gas/testsuite/gas/riscv/mapping-norelax-03bbe.d
> > b/gas/testsuite/gas/riscv/mapping-norelax-03bbe.d
> > new file mode 100644
> > index 0000000..c8520fd
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/mapping-norelax-03bbe.d
> > @@ -0,0 +1,25 @@
> > +#as: -mno-relax -mbig-endian
> > +#source: mapping-03.s
> > +#objdump: -d
> > +
> > +.*:[   ]+file format .*
> > +
> > +
> > +Disassembly of section .text:
> > +
> > +0+000 <.text>:
> > +[      ]+0:[   ]+00a50533[     ]+add[  ]+a0,a0,a0
> > +[      ]+4:[   ]+00000000[     ]+.word[        ]+0x00000000
> > +[      ]+8:[   ]+00000013[     ]+nop
> > +[      ]+c:[   ]+00000013[     ]+nop
> > +[      ]+10:[  ]+00000001[     ]+.word[        ]+0x00000001
> > +[      ]+14:[  ]+00b585b3[     ]+add[  ]+a1,a1,a1
> > +[      ]+18:[  ]+02000000[     ]+.word[        ]+0x02000000
> > +[      ]+1c:[  ]+03[   ]+.byte[        ]+0x03
> > +[      ]+1d:[  ]+00[   ]+.byte[        ]+0x00
> > +[      ]+1e:[  ]+0001[         ]+nop
> > +[      ]+20:[  ]+00000005[     ]+.word[        ]+0x00000005
> > +[      ]+24:[  ]+00000013[     ]+nop
> > +[      ]+28:[  ]+00000013[     ]+nop
> > +[      ]+2c:[  ]+00000013[     ]+nop
> > +#...
> > diff --git a/gas/testsuite/gas/riscv/mapping-norelax-04b.d
> > b/gas/testsuite/gas/riscv/mapping-norelax-04b.d
> > index 824a898..00a9dbd 100644
> > --- a/gas/testsuite/gas/riscv/mapping-norelax-04b.d
> > +++ b/gas/testsuite/gas/riscv/mapping-norelax-04b.d
> > @@ -1,4 +1,4 @@
> > -#as: -mno-relax
> > +#as: -mno-relax -mlittle-endian
> >  #source: mapping-04.s
> >  #objdump: -d
> >
> > diff --git a/gas/testsuite/gas/riscv/mapping-norelax-04bbe.d
> > b/gas/testsuite/gas/riscv/mapping-norelax-04bbe.d
> > new file mode 100644
> > index 0000000..0b987fa
> > --- /dev/null
> > +++ b/gas/testsuite/gas/riscv/mapping-norelax-04bbe.d
> > @@ -0,0 +1,24 @@
> > +#as: -mno-relax -mbig-endian
> > +#source: mapping-04.s
> > +#objdump: -d
> > +
> > +.*:[   ]+file format .*
> > +
> > +
> > +Disassembly of section .text:
> > +
> > +0+000 <.text>:
> > +[      ]+0:[   ]+00001001[     ]+.word[        ]+0x00001001
> > +[      ]+4:[   ]+00001001[     ]+.word[        ]+0x00001001
> > +[      ]+8:[   ]+01000000[     ]+.word[        ]+0x01000000
> > +[      ]+c:[   ]+00[   ]+.byte[        ]+0x00
> > +[      ]+d:[   ]+00[   ]+.byte[        ]+0x00
> > +[      ]+e:[   ]+0001[         ]+nop
> > +[      ]+10:[  ]+00a50533[     ]+add[  ]+a0,a0,a0
> > +[      ]+14:[  ]+20022002[     ]+.word[        ]+0x20022002
> > +[      ]+18:[  ]+20022002[     ]+.word[        ]+0x20022002
> > +[      ]+1c:[  ]+2002[         ]+.short[       ]+0x2002
> > +[      ]+1e:[  ]+00b585b3[     ]+add[  ]+a1,a1,a1
> > +[      ]+22:[  ]+0001[         ]+nop
> > +[      ]+24:[  ]+00000013[     ]+nop
> > +#...
> > diff --git a/opcodes/ChangeLog b/opcodes/ChangeLog index
> > f0e4b72..0f37316 100644
> > --- a/opcodes/ChangeLog
> > +++ b/opcodes/ChangeLog
> > @@ -1,3 +1,7 @@
> > +2021-11-01 Guy Benyei <guybe@nvidia.com>
> > +
> > +       * riscv-dis.c: Consider endiannes when printing data
> > +
> >  2021-10-27  Maciej W. Rozycki  <macro@embecosm.com>
> >
> >         * Makefile.am: Remove obsolete comment.
> > diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c index
> > 1a09440..56af8e0 100644
> > --- a/opcodes/riscv-dis.c
> > +++ b/opcodes/riscv-dis.c
> > @@ -485,13 +485,9 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t 
> > word, disassemble_info *info)
> >
> >    insnlen = riscv_insn_length (word);
> >
> > -  /* RISC-V instructions are always little-endian.  */
> > -  info->endian_code = BFD_ENDIAN_LITTLE;
> > -
> >    info->bytes_per_chunk = insnlen % 4 == 0 ? 4 : 2;
> >    info->bytes_per_line = 8;
> >    /* We don't support constant pools, so this must be code.  */
> > -  info->display_endian = info->endian_code;
> >    info->insn_info_valid = 1;
> >    info->branch_delay_insns = 0;
> >    info->data_size = 0;
> > @@ -811,6 +807,9 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
> >    else if (riscv_gpr_names == NULL)
> >      set_default_riscv_dis_options ();
> >
> > +  /* RISC-V instructions are always little-endian.  */
> > + info->endian_code = BFD_ENDIAN_LITTLE;
> > +
> >    mstate = riscv_search_mapping_symbol (memaddr, info);
> >    /* Save the last mapping state.  */
> >    last_map_state = mstate;
> > @@ -822,6 +821,7 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
> >        dump_size = riscv_data_length (memaddr, info);
> >        info->bytes_per_chunk = dump_size;
> >        riscv_disassembler = riscv_disassemble_data;
> > +      info->display_endian = info->endian;
> >      }
> >    else
> >      {
> > @@ -835,6 +835,7 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
> >        insn = (insn_t) bfd_getl16 (packet);
> >        dump_size = riscv_insn_length (insn);
> >        riscv_disassembler = riscv_disassemble_insn;
> > +      info->display_endian = info->endian_code;
> >      }
> >
> >    /* Fetch the instruction to dump.  */ @@ -844,7 +845,7 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
> >        (*info->memory_error_func) (status, memaddr, info);
> >        return status;
> >      }
> > -  insn = (insn_t) bfd_get_bits (packet, dump_size * 8, false);
> > +  insn = (insn_t) bfd_get_bits (packet, dump_size * 8,
> > + info->display_endian == BFD_ENDIAN_BIG);
> >
> >    return (*riscv_disassembler) (memaddr, insn, info);  }
> > --
> > 1.8.3.1
> >

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-11-30 15:05 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-01  7:55 [PATCH] RISC-V: Fix big endian disassembly of data Guy Benyei
2021-11-04  6:38 ` Guy Benyei
2021-11-04  7:32   ` Nelson Chu
2021-11-04  7:51     ` Guy Benyei
2021-11-04  8:59       ` Nelson Chu
2021-11-30 15:05         ` Guy Benyei
2021-11-04  7:38 ` Marcus Comstedt
2021-11-04 12:17 ` Maciej W. Rozycki
2021-11-04 13:43   ` Guy Benyei

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).