* [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-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-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
* 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-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
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).