For plugins we don't expect the raw opcodes in the disassembly. We
already deal with this by hand crafting our capstone call but for
other diassemblers we need a flag. Introduce show_opcodes which
defaults to off.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20240305121005.3528075-27-alex.bennee@linaro.org>

diff --git a/include/disas/dis-asm.h b/include/disas/dis-asm.h
index 2324f6b1a46..b26867b6417 100644
--- a/include/disas/dis-asm.h
+++ b/include/disas/dis-asm.h
@@ -396,6 +396,14 @@ typedef struct disassemble_info {
   /* Command line options specific to the target disassembler.  */
   char * disassembler_options;
 
+  /*
+   * When true instruct the disassembler it may preface the
+   * disassembly with the opcodes values if it wants to. This is
+   * mainly for the benefit of the plugin interface which doesn't want
+   * that.
+   */
+  bool show_opcodes;
+
   /* Field intended to be used by targets in any way they deem suitable.  */
   void *target_info;
 
diff --git a/disas/disas.c b/disas/disas.c
index 0d2d06c2ecc..17170d291ec 100644
--- a/disas/disas.c
+++ b/disas/disas.c
@@ -299,6 +299,7 @@ void disas(FILE *out, const void *code, size_t size)
     s.info.buffer = code;
     s.info.buffer_vma = (uintptr_t)code;
     s.info.buffer_length = size;
+    s.info.show_opcodes = true;
 
     if (s.info.cap_arch >= 0 && cap_disas_host(&s.info, code, size)) {
         return;
-- 
2.39.2

On 06/03/2024 15.40, Alex Bennée wrote:
> For plugins we don't expect the raw opcodes in the disassembly. We
> already deal with this by hand crafting our capstone call but for
> other diassemblers we need a flag. Introduce show_opcodes which
> defaults to off.
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20240305121005.3528075-27-alex.bennee@linaro.org>
> 
> diff --git a/include/disas/dis-asm.h b/include/disas/dis-asm.h
> index 2324f6b1a46..b26867b6417 100644
> --- a/include/disas/dis-asm.h
> +++ b/include/disas/dis-asm.h
> @@ -396,6 +396,14 @@ typedef struct disassemble_info {
>     /* Command line options specific to the target disassembler.  */
>     char * disassembler_options;
>   
> +  /*
> +   * When true instruct the disassembler it may preface the
> +   * disassembly with the opcodes values if it wants to. This is
> +   * mainly for the benefit of the plugin interface which doesn't want
> +   * that.
> +   */
> +  bool show_opcodes;
> +
>     /* Field intended to be used by targets in any way they deem suitable.  */
>     void *target_info;
>   
> diff --git a/disas/disas.c b/disas/disas.c
> index 0d2d06c2ecc..17170d291ec 100644
> --- a/disas/disas.c
> +++ b/disas/disas.c
> @@ -299,6 +299,7 @@ void disas(FILE *out, const void *code, size_t size)
>       s.info.buffer = code;
>       s.info.buffer_vma = (uintptr_t)code;
>       s.info.buffer_length = size;
> +    s.info.show_opcodes = true;
>   
>       if (s.info.cap_arch >= 0 && cap_disas_host(&s.info, code, size)) {
>           return;

I know it's too late now for a patch review, but anyway: What about the 
other spots that set up a "CPUDebug" struct? Like monitor_disas() or 
target_disas() ? Shouldn't we initialize the new struct member there, too?

  Thomas

Thomas Huth <thuth@redhat.com> writes:

> On 06/03/2024 15.40, Alex Bennée wrote:
>> For plugins we don't expect the raw opcodes in the disassembly. We
>> already deal with this by hand crafting our capstone call but for
>> other diassemblers we need a flag. Introduce show_opcodes which
>> defaults to off.
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Message-Id: <20240305121005.3528075-27-alex.bennee@linaro.org>
>> diff --git a/include/disas/dis-asm.h b/include/disas/dis-asm.h
>> index 2324f6b1a46..b26867b6417 100644
>> --- a/include/disas/dis-asm.h
>> +++ b/include/disas/dis-asm.h
>> @@ -396,6 +396,14 @@ typedef struct disassemble_info {
>>     /* Command line options specific to the target disassembler.  */
>>     char * disassembler_options;
>>   +  /*
>> +   * When true instruct the disassembler it may preface the
>> +   * disassembly with the opcodes values if it wants to. This is
>> +   * mainly for the benefit of the plugin interface which doesn't want
>> +   * that.
>> +   */
>> +  bool show_opcodes;
>> +
>>     /* Field intended to be used by targets in any way they deem suitable.  */
>>     void *target_info;
>>   diff --git a/disas/disas.c b/disas/disas.c
>> index 0d2d06c2ecc..17170d291ec 100644
>> --- a/disas/disas.c
>> +++ b/disas/disas.c
>> @@ -299,6 +299,7 @@ void disas(FILE *out, const void *code, size_t size)
>>       s.info.buffer = code;
>>       s.info.buffer_vma = (uintptr_t)code;
>>       s.info.buffer_length = size;
>> +    s.info.show_opcodes = true;
>>         if (s.info.cap_arch >= 0 && cap_disas_host(&s.info, code,
>> size)) {
>>           return;
>
> I know it's too late now for a patch review, but anyway: What about
> the other spots that set up a "CPUDebug" struct? Like monitor_disas()
> or target_disas() ? Shouldn't we initialize the new struct member
> there, too?

Hmm maybe. I can post some follow up fixes.

>
>  Thomas

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro