diff mbox series

[RFC] disas/hppa: drop raw opcode dump

Message ID 20240229140557.1749767-1-alex.bennee@linaro.org
State New
Headers show
Series [RFC] disas/hppa: drop raw opcode dump | expand

Commit Message

Alex Bennée Feb. 29, 2024, 2:05 p.m. UTC
The hppa disassembly is different from the others due to leading with
the raw opcode data. This confuses plugins looking for instruction
prefixes to match instructions. For plugins like execlog there is
another mechanism for getting the instruction byte data.

For the sake of consistently just present the instruction assembly
code.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 disas/hppa.c | 4 ----
 1 file changed, 4 deletions(-)

Comments

Helge Deller Feb. 29, 2024, 3:02 p.m. UTC | #1
On 2/29/24 15:05, Alex Bennée wrote:
> The hppa disassembly is different from the others due to leading with
> the raw opcode data. This confuses plugins looking for instruction
> prefixes to match instructions. For plugins like execlog there is
> another mechanism for getting the instruction byte data.
>
> For the sake of consistently just present the instruction assembly
> code.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

This effectively reverts commit 2f926bfd5b79e6219ae65a1e530b38f37d62b384
("disas/hppa: Show hexcode of instruction along with disassembly").

Sad, but Ok.

Acked-by: Helge Deller <deller@gmx.de>


> ---
>   disas/hppa.c | 4 ----
>   1 file changed, 4 deletions(-)
>
> diff --git a/disas/hppa.c b/disas/hppa.c
> index 22dce9b41bb..dd34cce211b 100644
> --- a/disas/hppa.c
> +++ b/disas/hppa.c
> @@ -1972,10 +1972,6 @@ print_insn_hppa (bfd_vma memaddr, disassemble_info *info)
>
>     insn = bfd_getb32 (buffer);
>
> -  info->fprintf_func(info->stream, " %02x %02x %02x %02x   ",
> -                (insn >> 24) & 0xff, (insn >> 16) & 0xff,
> -                (insn >>  8) & 0xff, insn & 0xff);
> -
>     for (i = 0; i < NUMOPCODES; ++i)
>       {
>         const struct pa_opcode *opcode = &pa_opcodes[i];
Alex Bennée Feb. 29, 2024, 4:41 p.m. UTC | #2
Helge Deller <deller@gmx.de> writes:

> On 2/29/24 15:05, Alex Bennée wrote:
>> The hppa disassembly is different from the others due to leading with
>> the raw opcode data. This confuses plugins looking for instruction
>> prefixes to match instructions. For plugins like execlog there is
>> another mechanism for getting the instruction byte data.
>>
>> For the sake of consistently just present the instruction assembly
>> code.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> This effectively reverts commit 2f926bfd5b79e6219ae65a1e530b38f37d62b384
> ("disas/hppa: Show hexcode of instruction along with disassembly").
>
> Sad, but Ok.
>
> Acked-by: Helge Deller <deller@gmx.de>

Well its an RFC for a reason. If this is useful we could just sneak a
flag into disassemble_info that is set by plugin_disas? Or maybe move
the insn stream to afterwards?

>
>
>> ---
>>   disas/hppa.c | 4 ----
>>   1 file changed, 4 deletions(-)
>>
>> diff --git a/disas/hppa.c b/disas/hppa.c
>> index 22dce9b41bb..dd34cce211b 100644
>> --- a/disas/hppa.c
>> +++ b/disas/hppa.c
>> @@ -1972,10 +1972,6 @@ print_insn_hppa (bfd_vma memaddr, disassemble_info *info)
>>
>>     insn = bfd_getb32 (buffer);
>>
>> -  info->fprintf_func(info->stream, " %02x %02x %02x %02x   ",
>> -                (insn >> 24) & 0xff, (insn >> 16) & 0xff,
>> -                (insn >>  8) & 0xff, insn & 0xff);
>> -
>>     for (i = 0; i < NUMOPCODES; ++i)
>>       {
>>         const struct pa_opcode *opcode = &pa_opcodes[i];
Richard Henderson Feb. 29, 2024, 6:11 p.m. UTC | #3
On 2/29/24 04:05, Alex Bennée wrote:
> The hppa disassembly is different from the others due to leading with
> the raw opcode data. This confuses plugins looking for instruction
> prefixes to match instructions. For plugins like execlog there is
> another mechanism for getting the instruction byte data.
> 
> For the sake of consistently just present the instruction assembly
> code.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   disas/hppa.c | 4 ----
>   1 file changed, 4 deletions(-)
> 
> diff --git a/disas/hppa.c b/disas/hppa.c
> index 22dce9b41bb..dd34cce211b 100644
> --- a/disas/hppa.c
> +++ b/disas/hppa.c
> @@ -1972,10 +1972,6 @@ print_insn_hppa (bfd_vma memaddr, disassemble_info *info)
>   
>     insn = bfd_getb32 (buffer);
>   
> -  info->fprintf_func(info->stream, " %02x %02x %02x %02x   ",
> -                (insn >> 24) & 0xff, (insn >> 16) & 0xff,
> -                (insn >>  8) & 0xff, insn & 0xff);
> -

It's hardly the only one doing this.  Our capstone dumper does this, and glancing at some 
others riscv.c does as well.

When you say "the others", I think you mean "everything using capstone", which has uses a 
different print function entirely for plugins just to avoid the dump.


r~
diff mbox series

Patch

diff --git a/disas/hppa.c b/disas/hppa.c
index 22dce9b41bb..dd34cce211b 100644
--- a/disas/hppa.c
+++ b/disas/hppa.c
@@ -1972,10 +1972,6 @@  print_insn_hppa (bfd_vma memaddr, disassemble_info *info)
 
   insn = bfd_getb32 (buffer);
 
-  info->fprintf_func(info->stream, " %02x %02x %02x %02x   ",
-                (insn >> 24) & 0xff, (insn >> 16) & 0xff,
-                (insn >>  8) & 0xff, insn & 0xff);
-
   for (i = 0; i < NUMOPCODES; ++i)
     {
       const struct pa_opcode *opcode = &pa_opcodes[i];