On 03/01/2010 10:33 PM, Aurelien Jarno wrote:
While trying to implement setcond on TCG ARM, I have discovered it does
not work anymore. I have bisected this regression to:
commit 6113d6d3169393c323ac4c82d756a850145a5e7a
Author: Paolo Bonzini<pbonz...@redhat.com>
Date: Fri Jan 15 09:42:09 2010 +0100
change while to if
The while loop will be executed exactly 0 or 1 times, depending on
env->exit_request.
Signed-off-by: Paolo Bonzini<pbonz...@redhat.com>
Signed-off-by: Anthony Liguori<aligu...@us.ibm.com>
The assertion is actually triggered. When the next patch removing the
assertion is also applied it segfaults instead.
Looks like a race. The only piece of logic that is changed by that
commit is reverted in the attached patch, can you try it? If it passes,
I can resubmit with S-o-b.
If it doesn't pass, I wonder whether the while loop was there to trick
the compiler into not optimizing something. Seems a bit too clever though.
Paolo
diff --git a/cpu-exec.c b/cpu-exec.c
index 5d6dd51..61b1c59 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -602,9 +602,15 @@ int cpu_exec(CPUState *env1)
/* cpu_interrupt might be called while translating the
TB, but before it is linked into a potentially
infinite loop and becomes env->current_tb. Avoid
- starting execution if there is a pending interrupt. */
- if (!unlikely (env->exit_request)) {
- env->current_tb = tb;
+ starting execution if there is a pending interrupt.
+ Doing it this way is necessary to avoid races with
+ cpu_unlink_tb (called by cpu_exit). */
+ env->current_tb = tb;
+ if (unlikely (env->exit_request)) {
+ env->current_tb = NULL;
+ }
+
+ if (likely (env->current_tb)) {
tc_ptr = tb->tc_ptr;
/* execute the generated code */
#if defined(__sparc__) && !defined(CONFIG_SOLARIS)
