Re: [PATCH] target/arm: Display helpful message when hflags mismatch

[Alex Bennée]

Philippe Mathieu-Daudé <address@hidden> writes:

> Instead of crashing in a confuse way, give some hint to the user
> about why we aborted. He might report the issue without having
> to use a debugger.
>
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
>  target/arm/helper.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 0bf8f53d4b..6bfb62672b 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -11348,6 +11348,20 @@ void HELPER(rebuild_hflags_a64)(CPUARMState *env, 
> int el)
>      env->hflags = rebuild_hflags_a64(env, el, fp_el, mmu_idx);
>  }
>  
> +static inline void assert_hflags_rebuild_correctly(CPUARMState *env)
> +{
> +#ifdef CONFIG_DEBUG_TCG
> +    uint32_t env_flags_current = env->hflags;
> +    uint32_t env_flags_rebuilt = rebuild_hflags_internal(env);
> +
> +    if (unlikely(env_flags_current != env_flags_rebuilt)) {
> +        fprintf(stderr, "TCG hflags mismatch (current:0x%08x 
> rebuilt:0x%08x)\n",
> +                env_flags_current, env_flags_rebuilt);
> +        abort();
> +    }
> +#endif
> +}
> +
>  void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
>                            target_ulong *cs_base, uint32_t *pflags)
>  {
> @@ -11355,9 +11369,7 @@ void cpu_get_tb_cpu_state(CPUARMState *env, 
> target_ulong *pc,
>      uint32_t pstate_for_ss;
>  
>      *cs_base = 0;
> -#ifdef CONFIG_DEBUG_TCG
> -    assert(flags == rebuild_hflags_internal(env));
> -#endif
> +    assert_hflags_rebuild_correctly(env);

I'm trying to recall why we don't just use:

  g_assert_cmphex(flags, =, rebuild_hflags_internal(env))

I think it came up in one of the reviews.

>  
>      if (FIELD_EX32(flags, TBFLAG_ANY, AARCH64_STATE)) {
>          *pc = env->pc;


-- 
Alex Bennée