On 2013-09-17 23:24, Peter Maydell wrote: > On 17 September 2013 18:03, Stefan Weil <[email protected]> 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
