The VM I did the work in doesn't have internet access and I was unsure how to do a text only email with gmail. With that said, the line that removed the env->gpr[1] is redudant as a few lines below in the original source it is set with newsp. The removed line would seg fault due to trying to write the value of env->gpr[1] into newsp, which is not valid in host.
I can not speak to the line a bit further with h2g(sc). Samuel On Wed, Jan 2, 2013 at 7:00 AM, Peter Maydell <peter.mayd...@linaro.org>wrote: > On 2 January 2013 04:58, Samuel Seay <lightnin...@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 >
