[PATCH 2/7] ntdll: Factor out setup_raise_exception.

[Jacek Caban]

Signed-off-by: Jacek Caban <jacek(a)codeweavers.com>
---
  dlls/ntdll/signal_x86_64.c | 75 +++++++++++++++++++++++---------------
  1 file changed, 46 insertions(+), 29 deletions(-)

[Alexandre Julliard]

Jacek Caban <jacek(a)codeweavers.com> writes:
> @@ -2818,24 +2819,27 @@ static EXCEPTION_RECORD *setup_exception( ucontext_t *sigcontext, raise_func fun
>      stack->rec.NumberParameters = 0;
>      save_context( &stack->context, sigcontext );
>
> -    /* store return address and %rbp without aligning, so that the offset is fixed */
> -    rsp_ptr = (ULONG64 *)RSP_sig(sigcontext) - 16;
> -    *(--rsp_ptr) = RIP_sig(sigcontext);
> -    *(--rsp_ptr) = RBP_sig(sigcontext);
> -    *(--rsp_ptr) = RDI_sig(sigcontext);
> -    *(--rsp_ptr) = RSI_sig(sigcontext);

Is there a reason for changing this part instead of moving it as is?

-- 
Alexandre Julliard
julliard(a)winehq.org

[Jacek Caban]

On 03/09/2019 21:11, Alexandre Julliard wrote:
> Jacek Caban <jacek(a)codeweavers.com> writes:
> > @@ -2818,24 +2819,27 @@ static EXCEPTION_RECORD *setup_exception( ucontext_t *sigcontext, raise_func fun
> >       stack->rec.NumberParameters = 0;
> >       save_context( &stack->context, sigcontext );
> >
> > -    /* store return address and %rbp without aligning, so that the offset is fixed */
> > -    rsp_ptr = (ULONG64 *)RSP_sig(sigcontext) - 16;
> > -    *(--rsp_ptr) = RIP_sig(sigcontext);
> > -    *(--rsp_ptr) = RBP_sig(sigcontext);
> > -    *(--rsp_ptr) = RDI_sig(sigcontext);
> > -    *(--rsp_ptr) = RSI_sig(sigcontext);
> > Is there a reason for changing this part instead of moving it as is?

It matters for patch 7. The debugger may change the context while thread 
is sleeping in send_debug_event. I think we should use modified values 
in this case.


Thanks,

Jacek

[Alexandre Julliard]

Jacek Caban <jacek(a)codeweavers.com> writes:
> On 03/09/2019 21:11, Alexandre Julliard wrote:
> > Jacek Caban <jacek(a)codeweavers.com> writes:
> > > @@ -2818,24 +2819,27 @@ static EXCEPTION_RECORD *setup_exception( ucontext_t *sigcontext, raise_func fun
> > >       stack->rec.NumberParameters = 0;
> > >       save_context( &stack->context, sigcontext );
> > >   -    /* store return address and %rbp without aligning, so that
> > > the offset is fixed */
> > > -    rsp_ptr = (ULONG64 *)RSP_sig(sigcontext) - 16;
> > > -    *(--rsp_ptr) = RIP_sig(sigcontext);
> > > -    *(--rsp_ptr) = RBP_sig(sigcontext);
> > > -    *(--rsp_ptr) = RDI_sig(sigcontext);
> > > -    *(--rsp_ptr) = RSI_sig(sigcontext);
> > > Is there a reason for changing this part instead of moving it as is?
>
> It matters for patch 7. The debugger may change the context while
> thread is sleeping in send_debug_event. I think we should use modified
> values in this case.

Yes, but why are you removing the "without aligning" part?  Accessing
the registers through the (aligned) structure is not going to use the
correct offsets.

-- 
Alexandre Julliard
julliard(a)winehq.org

[Jacek Caban]

On 9/3/19 9:41 PM, Alexandre Julliard wrote:
> Jacek Caban <jacek(a)codeweavers.com> writes:
> > On 03/09/2019 21:11, Alexandre Julliard wrote:
> > > Jacek Caban <jacek(a)codeweavers.com> writes:
> > > > @@ -2818,24 +2819,27 @@ static EXCEPTION_RECORD *setup_exception( ucontext_t *sigcontext, raise_func fun
> > > >        stack->rec.NumberParameters = 0;
> > > >        save_context( &stack->context, sigcontext );
> > > >    -    /* store return address and %rbp without aligning, so that
> > > > the offset is fixed */
> > > > -    rsp_ptr = (ULONG64 *)RSP_sig(sigcontext) - 16;
> > > > -    *(--rsp_ptr) = RIP_sig(sigcontext);
> > > > -    *(--rsp_ptr) = RBP_sig(sigcontext);
> > > > -    *(--rsp_ptr) = RDI_sig(sigcontext);
> > > > -    *(--rsp_ptr) = RSI_sig(sigcontext);
> > > > Is there a reason for changing this part instead of moving it as is?
> >
> > It matters for patch 7. The debugger may change the context while
> > thread is sleeping in send_debug_event. I think we should use modified
> > values in this case.
> > Yes, but why are you removing the "without aligning" part?  Accessing
> > the registers through the (aligned) structure is not going to use the
> > correct offsets.

Oh, right, I messed that up. Sorry about that, I will send a fixed version.


Thanks,

Jacek