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);
-
   for (i = 0; i < NUMOPCODES; ++i)
     {
       const struct pa_opcode *opcode = &pa_opcodes[i];
-- 
2.39.2

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~

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];

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];

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro