Re: PATCH 1/7] msvcrt: Translate SEH exception on each call to __CxxFrameHandler.

[Piotr Caban]

On 05/24/17 02:54, Daniel Lehman wrote:
> +extern void __wine_rtl_unwind( EXCEPTION_REGISTRATION_RECORD* frame, EXCEPTION_RECORD *record,
> -
>                           void (*target)(void) );

It's already defined in wine/exception.h.
> +extern void CDECL __DestructExceptionObject(EXCEPTION_RECORD *rec);

This is also already defined (in msvcrt.h)
> -
>
> +static void DECLSPEC_NORETURN seh_unwind_target(void)
> +{
> - seh_frame_info *seh_frame = (seh_frame_info *)__wine_get_frame();
> - __wine_pop_frame( &seh_frame->frame );

There's no need to call __wine_pop_frame here, it will be done after the 
jump anyway.

I think that the whole series will be cleaner/easier to read if we add 
the __EXCEPT_CTX/__FINALLY_CTX macros and use them here. I'm attaching a 
patch with proposition of changes for this functions. With such changes 
your patch will look like this (this is not tested):
typedef struct {
     ULONG64 dest_frame;
     ULONG64 orig_frame;
     DISPATCHER_CONTEXT *dispatch;
     const cxx_function_descr *descr;
} se_translator_ctx;

static DWORD se_translation_filter(EXCEPTION_POINTERS *ep, void *c)
{
     se_translator_ctx *ctx = (se_translator_ctx*)c;

/* this is copied from original patch but I think that */
     /* terminate should be called on non C++ exceptions here */
     if (ep->rec->ExceptionCode != CXX_EXCEPTION)
         return ExceptionContinueSearch;

find_catch_block(...);
     __DestructExceptionObject(ep->rec);
     return ExceptionExecuteHandler;
}

in cxx_frame_handler:
if (data->se_translator) {
     se_translator_ctx ctx;
     //initialize ctx

__TRY
     {
         except_ptrs.ExceptionRecord = rec;
         except_ptrs.ContextRecord = context;
         data->se_translator(rec->ExceptionCode, &except_ptrs);
     }
     __EXCEPT_CTX(se_translation_filter, &ctx)
     {
     }
     __END_TRY
}

With such changes second patch can be also changed to use 
__TRY/__FINALLY_CTX around "ret_addr = handler(0, frame);" file.

What do you think about it?

Thanks,
Piotr