Re: [PATCH v3 02/15] cpus: Add const-qualified CPU environment accessors

On 2026-08-21 13:33, Philippe Mathieu-Daudé wrote:
> On 21/8/26 12:49, Philippe Mathieu-Daudé wrote:
>> From: Philippe Mathieu-Daudé <[email protected]>
>>
>> Use _Generic() controlling-expression to add the const-qualified
>> variants of cpu_env(), env_cpu() and env_archcpu(). This allows to
>> safely access CPU architecture state when it should not be modified.
>>
>> Alias env_cpu_const() which is still used.
>>
>> Suggested-by: Richard Henderson <[email protected]>
>> Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
>> Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
>> Reviewed-by: Marc-André Lureau <[email protected]>
>> Reviewed-by: Richard Henderson <[email protected]>
>> ---
>> Following checkpatch.pl errors ignored:
>>
>>    ERROR: spaces required around that ':' (ctx:VxE)
>>    #46: FILE: include/exec/cpu-common.h:89:
>>    +            CPUArchState: \
>>                             ^
>>    ERROR: spaces required around that ':' (ctx:VxE)
>>    #62: FILE: include/exec/cpu-common.h:101:
>>    +            CPUArchState: \
>>                             ^
>>    ERROR: spaces required around that ':' (ctx:VxE)
>>    #90: FILE: include/hw/core/cpu.h:605:
>>    +        CPUState: \
>>                     ^
>>    total: 3 errors, 0 warnings, 64 lines checked
>> ---
>>   include/exec/cpu-common.h | 30 +++++++++++-------------------
>>   include/hw/core/cpu.h     | 17 ++++++++++++-----
>>   2 files changed, 23 insertions(+), 24 deletions(-)
>>
>> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
>> index 6594f7fa1be..bffef677607 100644
>> --- a/include/exec/cpu-common.h
>> +++ b/include/exec/cpu-common.h
>> @@ -85,21 +85,11 @@ static inline bool cpu_loop_exit_requested(const 
>> CPUState *cpu)
>>    *
>>    * Return the ArchCPU associated with the environment.
>>    */
>> -static inline ArchCPU *env_archcpu(CPUArchState *env)
>> -{
>> -    return (void *)env - sizeof(CPUState);
>> -}
>> -
>> -/**
>> - * env_cpu_const(env)
>> - * @env: The architecture environment
>> - *
>> - * Return the CPUState associated with the environment.
>> - */
>> -static inline const CPUState *env_cpu_const(const CPUArchState *env)
>> -{
>> -    return (void *)env - sizeof(CPUState);
>> -}
>> +#define env_archcpu(env) _Generic(*(env), \
>> +            CPUArchState: \
>> +                (ArchCPU *)((void *)env - sizeof(CPUState)), \
>> +            const CPUArchState: \
>> +                (const ArchCPU *)((const void *)env - sizeof(CPUState)))
> I was testing with a C17-ready compiler. Apparently C11 is ambiguous in
> how it treats qualifiers inside _Generic:
> 
> ../../target/arm/internals.h:1777:25: warning: due to lvalue conversion 
> of the controlling expression, association of type 'const CPUArchState' 
> (aka 'const struct CPUArchState') will never be selected because it is 
> qualified [-Wunreachable-code-generic-assoc]
>   1777 |     const ARMCPU *cpu = env_archcpu(env);
>        |                         ^
> include/exec/cpu-common.h:91:19: note: expanded from macro 'env_archcpu'
>     91 |             const CPUArchState: \
>        |                   ^
> 
> Using typeof() makes it happier:
> 
> -- >8 --
> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
> index d67d008236f..e3a5e40899f 100644
> --- a/include/exec/cpu-common.h
> +++ b/include/exec/cpu-common.h
> @@ -87,6 +87,6 @@ static inline bool cpu_loop_exit_requested(const 
> CPUState *cpu)
>    */
> -#define env_archcpu(env) _Generic(*(env), \
> -            CPUArchState: \
> +#define env_archcpu(env) _Generic(typeof(*env), \
> +            typeof(CPUArchState): \
>                   (ArchCPU *)((void *)env - sizeof(CPUState)), \
> -            const CPUArchState: \
> +            typeof(const CPUArchState): \
>                   (const ArchCPU *)((const void *)env - sizeof(CPUState)))
> @@ -99,6 +99,6 @@ static inline bool cpu_loop_exit_requested(const 
> CPUState *cpu)
>    */
> -#define env_cpu(env) _Generic(*(env), \
> -            CPUArchState: \
> +#define env_cpu(env) _Generic(typeof(*env), \
> +            typeof(CPUArchState): \
>                   (CPUState *)((void *)env - sizeof(CPUState)), \
> -            const CPUArchState: \
> +            typeof(const CPUArchState): \
>                   (const CPUState *)((const void *)env - sizeof(CPUState)))
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 372485a2e54..ee522e1e705 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -602,7 +602,7 @@ QEMU_BUILD_BUG_ON(offsetof(CPUState, neg) !=
>    */
> -#define cpu_env(cpu) _Generic(*(cpu), \
> +#define cpu_env(cpu) _Generic(typeof(*cpu), \
>           /* We validate that CPUArchState follows CPUState in cpu- 
> target.c */ \
> -        CPUState: \
> +        typeof(CPUState): \
>               (CPUArchState *)(cpu + 1), \
> -        const CPUState: \
> +        typeof(const CPUState): \
>               (const CPUArchState *)(cpu + 1))
> ---

No, this now fails distinctly on Ubuntu 24.04:

In file included from ../accel/tcg/tcg-runtime.c:26:
../accel/tcg/tcg-runtime.c: In function ‘helper_exit_atomic’:
/home/runner/work/qemu/qemu/include/exec/cpu-common.h:100:31: error: 
expected expression before ‘typeof’
   100 | #define env_cpu(env) _Generic(typeof(*env), \
       |                               ^~~~~~
../accel/tcg/tcg-runtime.c:135:26: note: in expansion of macro ‘env_cpu’
   135 |     cpu_loop_exit_atomic(env_cpu(env), GETPC());
       |                          ^~~~~~~

I'm a bit clueless. In case you want to have a look, the
series is available here:
https://github.com/qualcomm/qemu/commits/refs/heads/philmd/cpu_has_work_const/

Thanks,

Phil.