Re: [Qemu-devel] [PATCH] Change to correct PowerPC on a 64bit host

[Peter Maydell <peter.maydell@linaro.org>]

On 2 January 2013 04:58, Samuel Seay <lightningth@gmail.com> wrote:
> Attached is a patch for fixing bug #1052857. My local tests show it working
> properly on 32 and 64bit.

--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -4584,7 +4584,7 @@ static void setup_frame(int sig, struct
target_sigaction *ka,

     signal = current_exec_domain_sig(sig);

-    err |= __put_user(h2g(ka->_sa_handler), &sc->handler);
+    err |= __put_user(ka->_sa_handler, &sc->handler);
     err |= __put_user(set->sig[0], &sc->oldmask);
 #if defined(TARGET_PPC64)
     err |= __put_user(set->sig[0] >> 32, &sc->_unused[3]);

This looks OK...


@@ -4606,8 +4606,6 @@ static void setup_frame(int sig, struct
target_sigaction *ka,

     /* Create a stack frame for the caller of the handler.  */
     newsp = frame_addr - SIGNAL_FRAMESIZE;
-    err |= __put_user(env->gpr[1], (target_ulong *)(uintptr_t) newsp);
-
     if (err)
         goto sigsegv;

...but this bit doesn't. We need to save the old SP to the stack frame,
and your patch just skips this step. You're right that the line in question
is broken though; it has two problems:
 * it's using newsp (a guest address) as an argument to __put_user(),
   which wants a host address
 * it's using __put_user() which works on locked addresses, but newsp
   is below the area we locked with lock_user_struct earlier

Another dodgy line in this function:
    env->gpr[4] = (target_ulong) h2g(sc);
Since sc is an offset into the struct returned by lock_user_struct(),
if DEBUG_REMAP is defined then we're passing the guest a pointer
to memory that is free()d by unlock_user_struct(). This should probably
be setting gpr[4] to frame_addr + offsetof(something) instead.

-- PMM