Re: [Qemu-devel] [PATCH] cpu-exec: Fix compiler warning (-Werror=clobbered)

[Jan Kiszka]

On 2013-09-17 23:24, Peter Maydell wrote:
> On 17 September 2013 18:03, Stefan Weil <address@hidden> wrote:
>> could you please review this patch which removes code added by you earlier?
>> I have run tests with the old code and assertions to see whether the values
>> were really smashed. They never were, and from the documentation of setjmp
>> I'd not expect that they ever might be.
> 
> We had a discussion about this back in 2011. Any compiler which needs
> these statements is definitely buggy -- the C standard mandates that
> they're not needed.

I'm not that sure about this. We have a no-return function involved
between setjmp and the actual longjmp. Why should the compiler have to
preserve local variables when entering cpu_loop_exit?

> Unfortunately Jan never said what compiler he was using;

If you look down the thread: gcc 4.5.0. However, gcc 4.5.1 does not seem
to reproduce the issue anymore.

On the other hand, reloading that variable outside the setjmp/longjmp
section should not make the compiler worry about clobbering. The warning
seems false positive: if you pull the assignment before the setjmp (see
below), it's fine, if you move it at the end of the very same loop, my
compiler starts to grumble.

diff --git a/cpu-exec.c b/cpu-exec.c
index 5a43995..3882d74 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -266,6 +266,10 @@ int cpu_exec(CPUArchState *env)
 
     /* prepare setjmp context for exception handling */
     for(;;) {
+        /* Reload env after longjmp - the compiler may have smashed all
+         * local variables as cpu_loop_exit is marked 'noreturn'. */
+        cpu = current_cpu;
+        env = cpu->env_ptr;
         if (sigsetjmp(env->jmp_env, 0) == 0) {
             /* if an exception is pending, we execute it here */
             if (env->exception_index >= 0) {
@@ -676,11 +680,6 @@ int cpu_exec(CPUArchState *env)
                 /* reset soft MMU for next block (it can currently
                    only be set by a memory fault) */
             } /* for(;;) */
-        } else {
-            /* Reload env after longjmp - the compiler may have smashed all
-             * local variables as longjmp is marked 'noreturn'. */
-            cpu = current_cpu;
-            env = cpu->env_ptr;
         }
     } /* for(;;) */
 

Jan

signature.asc
Description: OpenPGP digital signature