RISC-V: Fix big endian disassembly of data.

Message ID CH0PR12MB50270E88515945BF81B9BFDDC48A9@CH0PR12MB5027.namprd12.prod.outlook.com
State New
Headers show
Series
  • RISC-V: Fix big endian disassembly of data.
Related show

Commit Message

H.J. Lu via Binutils Nov. 1, 2021, 7:55 a.m.
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

-- 
1.8.3.1

Comments

H.J. Lu via Binutils Nov. 4, 2021, 6:38 a.m. | #1
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
Nelson Chu Nov. 4, 2021, 7:32 a.m. | #2
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

>
Marcus Comstedt Nov. 4, 2021, 7:38 a.m. | #3
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
H.J. Lu via Binutils Nov. 4, 2021, 7:51 a.m. | #4
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

>
Nelson Chu Nov. 4, 2021, 8:59 a.m. | #5
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

> >
Maciej W. Rozycki Nov. 4, 2021, 12:17 p.m. | #6
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
H.J. Lu via Binutils Nov. 4, 2021, 1:43 p.m. | #7
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

Patch

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);
 }