[v1] disas: Have CPUClass::disas_set_info() callback set the endianness

[PATCH 03/10] target/arm: Set disassemble_info::endian value in disas_set_info()

Posted by Philippe Mathieu-Daudé 2 months, 1 week ago

Have the CPUClass::disas_set_info() callback set the
disassemble_info::endian field.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/cpu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index f9fdf708653..9de8c799c77 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1220,6 +1220,8 @@ static void arm_disas_set_info(CPUState *cpu, disassemble_info *info)
 #else
         info->endian = BFD_ENDIAN_BIG;
 #endif
+    } else {
+        info->endian = BFD_ENDIAN_LITTLE;
     }
     info->flags &= ~INSN_ARM_BE32;
 #ifndef CONFIG_USER_ONLY
-- 
2.47.1

Re: [PATCH 03/10] target/arm: Set disassemble_info::endian value in disas_set_info()

Posted by Thomas Huth 2 months, 1 week ago

On 27/01/2025 12.54, Philippe Mathieu-Daudé wrote:
> Have the CPUClass::disas_set_info() callback set the
> disassemble_info::endian field.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/arm/cpu.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index f9fdf708653..9de8c799c77 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1220,6 +1220,8 @@ static void arm_disas_set_info(CPUState *cpu, disassemble_info *info)
>   #else
>           info->endian = BFD_ENDIAN_BIG;
>   #endif
> +    } else {
> +        info->endian = BFD_ENDIAN_LITTLE;
>       }

I'd maybe rather go with something like this:

     info->endian = BFD_ENDIAN_LITTLE;
#if TARGET_BIG_ENDIAN
     if (bswap_code(sctlr_b)) {
         info->endian = BFD_ENDIAN_LITTLE;
     }
#endif

What do you think?

  Thomas

Re: [PATCH 03/10] target/arm: Set disassemble_info::endian value in disas_set_info()

Posted by Philippe Mathieu-Daudé 1 month, 3 weeks ago

On 27/1/25 14:57, Thomas Huth wrote:
> On 27/01/2025 12.54, Philippe Mathieu-Daudé wrote:
>> Have the CPUClass::disas_set_info() callback set the
>> disassemble_info::endian field.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   target/arm/cpu.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
>> index f9fdf708653..9de8c799c77 100644
>> --- a/target/arm/cpu.c
>> +++ b/target/arm/cpu.c
>> @@ -1220,6 +1220,8 @@ static void arm_disas_set_info(CPUState *cpu, 
>> disassemble_info *info)
>>   #else
>>           info->endian = BFD_ENDIAN_BIG;
>>   #endif
>> +    } else {
>> +        info->endian = BFD_ENDIAN_LITTLE;
>>       }
> 
> I'd maybe rather go with something like this:
> 
>      info->endian = BFD_ENDIAN_LITTLE;
> #if TARGET_BIG_ENDIAN
>      if (bswap_code(sctlr_b)) {
>          info->endian = BFD_ENDIAN_LITTLE;
>      }

This misses:

        else {
            info->endian = BFD_ENDIAN_BIG;
        }

> #endif
> 
> What do you think?

I'll go with Richard's ternary suggestion for v2 and see :)