Re: wine/dlls/ntdll signal_i386.c exception.c

[Robert Shearman]

Alexandre Julliard wrote:
> Index: wine/dlls/ntdll/signal_i386.c
> diff -u -p wine/dlls/ntdll/signal_i386.c:1.100 wine/dlls/ntdll/signal_i386.c:1.101
> --- wine/dlls/ntdll/signal_i386.c	31 Oct 2005 14: 8: 5 -0000
> +++ /dev/null	31 Oct 2005 14: 8: 5 -0000
> @@ -1384,4 +1384,36 @@ __ASM_GLOBAL_FUNC( DbgBreakPoint, "int $
>  */
> __ASM_GLOBAL_FUNC( DbgUserBreakPoint, "int $3; ret");
> -
>
> +/**********************************************************************
> -
> -
> EXC_CallHandler   (internal)
> - */
>
> +__ASM_GLOBAL_FUNC( EXC_CallHandler,
> +"	pushl	%ebp\n"
> +"	movl	%esp, %ebp\n"
> +"	subl	$12, %esp\n"
> +"	movl	28(%ebp), %eax\n"
> +"	movl	12(%ebp), %edx\n"
> +"	movl	%eax, -8(%ebp)\n"
> +"	movl	%edx, -4(%ebp)\n"
> +"	leal	-12(%ebp), %eax\n"
> +"	.byte 0x64\n"
> +"	movl (0),%ecx\n"

Is it not possible to use the proper %fs prefix instead of having to put 
a bytecode in manually?
> +"	movl %ecx,(%eax)\n"
> +"	.byte 0x64\n"
> +"	movl %eax,(0)\n"
> +"	movl	20(%ebp), %eax\n"
> +"	pushl	%eax\n"

There is no need to move the variable into %eax before pushing, as 
"pushl 20(%ebp)" will work as shortens the assembly.
> +"	movl	16(%ebp), %eax\n"
> +"	pushl	%eax\n"
> +"	movl	8(%ebp), %eax\n"
> +"	pushl	%edx\n"
> +"	pushl	%eax\n"
> +"	call	*24(%ebp)\n"
> +"	movl	-12(%ebp), %edx\n"
> +"	.byte 0x64\n"
> +"	movl %edx,(0)\n"
> +"	leave\n"
> +"	ret\n"
> +);
> #endif  /* __i386__ */
> Index: wine/dlls/ntdll/exception.c
> diff -u -p wine/dlls/ntdll/exception.c:1.85 wine/dlls/ntdll/exception.c:1.86
> --- wine/dlls/ntdll/exception.c	31 Oct 2005 14: 8: 5 -0000
> +++ /dev/null	31 Oct 2005 14: 8: 5 -0000
> @@ -116,7 +116,10 @@ static DWORD EXC_UnwindHandler( EXCEPTIO
> - happening during the handler execution.
> - Please do not change the first 4 parameters order in any way - some exceptions handlers
> - rely on Base Pointer (EBP) to have a fixed position related to the exception frame
> -
> -
> -
> - For i386 this function is implemented in assembler in signal_i386.c.

It seems bad to split the function's implementation up like this? We 
also now lose the TRACE's that were previously printed from within 
EXC_CallHandler. If they are superfluous to requirements we might as 
well delete them from the generic function too.
> */
> +#ifndef __i386__
> static DWORD EXC_CallHandler( EXCEPTION_RECORD *record, EXCEPTION_REGISTRATION_RECORD *frame,
>                               CONTEXT *context, EXCEPTION_REGISTRATION_RECORD **dispatcher,
>                               PEXCEPTION_HANDLER handler, PEXCEPTION_HANDLER nested_handler)

wine/dlls/ntdll signal_i386.c exception.c

[Marcus Meissner]

> > +"	leal	-12(%ebp), %eax\n"
> > +"	.byte 0x64\n"
> > +"	movl (0),%ecx\n"
>
> Is it not possible to use the proper %fs prefix instead of having to put 
> a bytecode in manually?

It will likely not build with all assemblers. This is taken from the
wine_push_frame() inline assemblercode directly.
> > +"	movl %ecx,(%eax)\n"
> > +"	.byte 0x64\n"
> > +"	movl %eax,(0)\n"
> > +"	movl	20(%ebp), %eax\n"
> > +"	pushl	%eax\n"
>
> There is no need to move the variable into %eax before pushing, as 
> "pushl 20(%ebp)" will work as shortens the assembly.

I used the assembler code that gcc generated directly. :/
> > -
> > - For i386 this function is implemented in assembler in signal_i386.c.
>
> It seems bad to split the function's implementation up like this? We 
> also now lose the TRACE's that were previously printed from within 
> EXC_CallHandler. If they are superfluous to requirements we might as 
> well delete them from the generic function too.

I did not want to merge the TRACE()s into assembler, because this one might
change and thenthe assembler would need to be ported.

They could go from the generic one, yes.

Ciao, Marcus