On 2013-09-17 23:24, Peter Maydell wrote: > On 17 September 2013 18:03, Stefan Weil <s...@weilnetz.de> 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