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

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))
---