Re: Make VIRTUAL_SetFaultHandler an internal function
"Dimitrie O. Paun" <dpaun(a)rogers.com> writes:
I guess this has no chance of going away until we have a DIB engine, which is not even on the horizon.
Actually it should be possible to handle the fault using a vectored handler, without requiring internal functions at all. -- Alexandre Julliard julliard(a)winehq.org
Alexandre Julliard a écrit :
"Dimitrie O. Paun" <dpaun(a)rogers.com> writes:
I guess this has no chance of going away until we have a DIB engine, which is not even on the horizon.
Actually it should be possible to handle the fault using a vectored handler, without requiring internal functions at all.
If you suppose no app would ever set its own vectored handler at the beginnnig of the vector... One could argue that no well behaved application should do it, but well... A+ -- Eric Pouech
Eric Pouech <pouech-eric(a)wanadoo.fr> writes:
If you suppose no app would ever set its own vectored handler at the beginnnig of the vector... One could argue that no well behaved application should do it, but well...
That would only be a problem if the app tries to handle the exception. I think that's unlikely since even on Windows you can get internal exceptions (for instance the IsBad* functions) so an app would have to be able to pass through the things it doesn't know how to handle. -- Alexandre Julliard julliard(a)winehq.org
On Thu, Apr 14, 2005 at 03:05:36PM +0200, Alexandre Julliard wrote:
Actually it should be possible to handle the fault using a vectored handler, without requiring internal functions at all.
Completely untested (what do people use to test DIB handling?), but it compiles. Is something like this what you had it mind? Index: dlls/ntdll/ntdll.spec =================================================================== RCS file: /var/cvs/wine/dlls/ntdll/ntdll.spec,v retrieving revision 1.175 diff -u -p -r1.175 ntdll.spec --- dlls/ntdll/ntdll.spec 30 Mar 2005 10:22:51 -0000 1.175 +++ dlls/ntdll/ntdll.spec 17 Apr 2005 04:18:44 -0000 @@ -1035,4 +1035,3 @@ @ cdecl MODULE_DllThreadAttach(ptr) @ cdecl MODULE_GetLoadOrderW(ptr wstr wstr) @ cdecl VERSION_Init(wstr) -@ cdecl VIRTUAL_SetFaultHandler(ptr ptr ptr) Index: dlls/ntdll/virtual.c =================================================================== RCS file: /var/cvs/wine/dlls/ntdll/virtual.c,v retrieving revision 1.46 diff -u -p -r1.46 virtual.c --- dlls/ntdll/virtual.c 22 Feb 2005 19:33:50 -0000 1.46 +++ dlls/ntdll/virtual.c 17 Apr 2005 04:20:14 -0000 @@ -212,11 +212,14 @@ void VIRTUAL_Dump(void) * * Find the view containing a given address. The csVirtual section must be held by caller. * + * PARAMS + * addr [I] Address + * * RETURNS * View: Success * NULL: Failure */ -static struct file_view *VIRTUAL_FindView( const void *addr ) /* [in] Address */ +static struct file_view *VIRTUAL_FindView( const void *addr ) { struct list *ptr; @@ -473,10 +476,13 @@ static void VIRTUAL_GetWin32Prot( * * Build page protections from Win32 flags. * + * PARAMS + * protect [I] Win32 protection flags + * * RETURNS * Value of page protection flags */ -static BYTE VIRTUAL_GetProt( DWORD protect ) /* [in] Win32 protection flags */ +static BYTE VIRTUAL_GetProt( DWORD protect ) { BYTE vprot; @@ -1098,25 +1104,6 @@ void virtual_init(void) /*********************************************************************** - * VIRTUAL_SetFaultHandler - */ -BOOL VIRTUAL_SetFaultHandler( LPCVOID addr, HANDLERPROC proc, LPVOID arg ) -{ - FILE_VIEW *view; - BOOL ret = FALSE; - - RtlEnterCriticalSection( &csVirtual ); - if ((view = VIRTUAL_FindView( addr ))) - { - view->handlerProc = proc; - view->handlerArg = arg; - ret = TRUE; - } - RtlLeaveCriticalSection( &csVirtual ); - return ret; -} - -/*********************************************************************** * VIRTUAL_HandleFault */ DWORD VIRTUAL_HandleFault( LPCVOID addr ) Index: dlls/x11drv/dib.c =================================================================== RCS file: /var/cvs/wine/dlls/x11drv/dib.c,v retrieving revision 1.35 diff -u -p -r1.35 dib.c --- dlls/x11drv/dib.c 14 Apr 2005 12:48:31 -0000 1.35 +++ dlls/x11drv/dib.c 17 Apr 2005 03:54:45 -0000 @@ -38,11 +38,32 @@ #include "winbase.h" #include "wingdi.h" #include "x11drv.h" +#include "excpt.h" +#include "wine/list.h" #include "wine/debug.h" WINE_DEFAULT_DEBUG_CHANNEL(bitmap); WINE_DECLARE_DEBUG_CHANNEL(x11drv); +struct mem_area +{ + struct list entry; /* Entry in global mem area list */ + const void *base; /* Base address */ + UINT size; /* Size in bytes */ + void *data; /* Data associated with this area */ +}; + +static struct list mem_areas_list = LIST_INIT(mem_areas_list); + +static CRITICAL_SECTION csMemAreas; +static CRITICAL_SECTION_DEBUG csMemAreas_debug = +{ + 0, 0, &csMemAreas, + { &csMemAreas_debug.ProcessLocksList, &csMemAreas_debug.ProcessLocksList }, + 0, 0, { 0, (DWORD)(__FILE__ ": csMemAreas") } +}; +static CRITICAL_SECTION csMemAreas = { &csMemAreas_debug, -1, 0, 0, 0, 0 }; + static int ximageDepthTable[32]; /* This structure holds the arguments for DIB_SetImageBits() */ @@ -88,6 +109,89 @@ static INT X11DRV_DIB_Coerce(X_PHYSBITMA static INT X11DRV_DIB_Lock(X_PHYSBITMAP *,INT,BOOL); static void X11DRV_DIB_Unlock(X_PHYSBITMAP *,BOOL); +/*********************************************************************** + * mem_area_find + * + * Find the memory area containing a given address. + * The csMemAreas section must be held by caller. + * + * PARAMS + * addr [I] Address + * + * RETURNS + * Success: area + * Failure: NULL + */ +static struct mem_area *mem_area_find( const void *addr ) +{ + struct list *ptr; + + LIST_FOR_EACH( ptr, &mem_areas_list ) + { + struct mem_area *area = LIST_ENTRY( ptr, struct mem_area, entry ); + if (area->base > addr) break; + if ((const char*)addr < (const char*)area->base + area->size) return area; + } + return NULL; +} + +/*********************************************************************** + * mem_area_add + * + * Adds a memory area to the list. + * + * PARAMS + * base [I] Beginning address of the area + * size [I] Size of the area + * data [I] Data associated with the area + * + */ +static struct mem_area *mem_area_add( const void *base, UINT size, void *data ) +{ + struct mem_area *area; + + EnterCriticalSection( &csMemAreas ); + + area = HeapAlloc(GetProcessHeap(), 0, sizeof(*area)); + if (area) + { + area->base = base; + area->size = size; + area->data = data; + list_add_head( &mem_areas_list, &area->entry ); + } + + LeaveCriticalSection( &csMemAreas ); + + return area; +} + + +/*********************************************************************** + * mem_area_del + * + * Removes a memory area from the list. + * + * PARAMS + * addr [I] Address + * + * RETURNS + * Success: TRUE + * Failure: FALSE + */ +static BOOL mem_area_del( const void *addr ) +{ + struct mem_area *area; + + EnterCriticalSection( &csMemAreas ); + area = mem_area_find( addr ); + if (area) list_remove( &area->entry ); + LeaveCriticalSection( &csMemAreas ); + + return area != NULL; +} + + /* Some of the following helper functions are duplicated in dlls/gdi/dib.c @@ -4268,22 +4372,34 @@ static void X11DRV_DIB_DoUpdateDIBSectio /*********************************************************************** * X11DRV_DIB_FaultHandler */ -static BOOL X11DRV_DIB_FaultHandler( LPVOID res, LPCVOID addr ) +LONG CALLBACK X11DRV_DIB_FaultHandler( PEXCEPTION_POINTERS ep ) { - X_PHYSBITMAP *physBitmap = res; - INT state = X11DRV_DIB_Lock( physBitmap, DIB_Status_None, FALSE ); + struct mem_area *area; + X_PHYSBITMAP *physBitmap; + INT state; - if (state != DIB_Status_InSync) { - /* no way to tell whether app needs read or write yet, - * try read first */ - X11DRV_DIB_Coerce( physBitmap, DIB_Status_InSync, FALSE ); - } else { - /* hm, apparently the app must have write access */ - X11DRV_DIB_Coerce( physBitmap, DIB_Status_AppMod, FALSE ); - } - X11DRV_DIB_Unlock( physBitmap, TRUE ); + if (ep->ExceptionRecord->ExceptionCode != EXCEPTION_ACCESS_VIOLATION) + return EXCEPTION_CONTINUE_SEARCH; - return TRUE; + EnterCriticalSection(&csMemAreas); + area = mem_area_find(ep->ExceptionRecord->ExceptionAddress); + if (area) physBitmap = (X_PHYSBITMAP *)area->data; + else physBitmap = NULL; + LeaveCriticalSection(&csMemAreas); + + if (!physBitmap) return EXCEPTION_CONTINUE_SEARCH; + + state = X11DRV_DIB_Lock( physBitmap, DIB_Status_None, FALSE ); + if (state != DIB_Status_InSync) { + /* no way to tell whether app needs read or write yet, try read first */ + X11DRV_DIB_Coerce( physBitmap, DIB_Status_InSync, FALSE ); + } else { + /* hm, apparently the app must have write access */ + X11DRV_DIB_Coerce( physBitmap, DIB_Status_AppMod, FALSE ); + } + X11DRV_DIB_Unlock( physBitmap, TRUE ); + + return EXCEPTION_CONTINUE_EXECUTION; } /*********************************************************************** @@ -4566,7 +4682,6 @@ static XImage *X11DRV_XShmCreateImage( i HBITMAP X11DRV_CreateDIBSection( X11DRV_PDEVICE *physDev, HBITMAP hbitmap, const BITMAPINFO *bmi, UINT usage ) { - extern BOOL VIRTUAL_SetFaultHandler(LPCVOID addr, BOOL (*proc)(LPVOID, LPCVOID), LPVOID arg); X_PHYSBITMAP *physBitmap; DIBSECTION dib; @@ -4602,7 +4717,7 @@ HBITMAP X11DRV_CreateDIBSection( X11DRV_ /* install fault handler */ InitializeCriticalSection( &physBitmap->lock ); - if (VIRTUAL_SetFaultHandler(dib.dsBm.bmBits, X11DRV_DIB_FaultHandler, physBitmap)) + if (mem_area_add(dib.dsBm.bmBits, dib.dsBmih.biSize, physBitmap)) { X11DRV_DIB_DoProtectDIBSection( physBitmap, PAGE_READWRITE ); physBitmap->status = DIB_Status_AppMod; @@ -4616,6 +4731,8 @@ HBITMAP X11DRV_CreateDIBSection( X11DRV_ */ void X11DRV_DIB_DeleteDIBSection(X_PHYSBITMAP *physBitmap, DIBSECTION *dib) { + mem_area_del(dib->dsBm.bmBits); + if (dib->dshSection) X11DRV_DIB_Coerce(physBitmap, DIB_Status_InSync, FALSE); Index: dlls/x11drv/x11drv.h =================================================================== RCS file: /var/cvs/wine/dlls/x11drv/x11drv.h,v retrieving revision 1.69 diff -u -p -r1.69 x11drv.h --- dlls/x11drv/x11drv.h 14 Apr 2005 12:48:11 -0000 1.69 +++ dlls/x11drv/x11drv.h 17 Apr 2005 03:42:32 -0000 @@ -234,6 +234,7 @@ extern BOOL X11DRV_SwapBuffers(X11DRV_PD extern void X11DRV_BITMAP_Init(void); extern void X11DRV_FONT_Init( int log_pixels_x, int log_pixels_y ); +extern LONG CALLBACK X11DRV_DIB_FaultHandler( PEXCEPTION_POINTERS ep ); extern int X11DRV_DIB_BitmapInfoSize( const BITMAPINFO * info, WORD coloruse ); extern XImage *X11DRV_DIB_CreateXImage( int width, int height, int depth ); extern HGLOBAL X11DRV_DIB_CreateDIBFromBitmap(HDC hdc, HBITMAP hBmp); Index: dlls/x11drv/x11drv_main.c =================================================================== RCS file: /var/cvs/wine/dlls/x11drv/x11drv_main.c,v retrieving revision 1.102 diff -u -p -r1.102 x11drv_main.c --- dlls/x11drv/x11drv_main.c 14 Apr 2005 12:48:11 -0000 1.102 +++ dlls/x11drv/x11drv_main.c 16 Apr 2005 16:59:03 -0000 @@ -362,6 +362,9 @@ static BOOL process_attach(void) using_wine_desktop = 1; } + /* initialize DIB handling */ + AddVectoredExceptionHandler( TRUE, X11DRV_DIB_FaultHandler ); + /* initialize GDI */ X11DRV_GDI_Initialize( display ); @@ -422,6 +425,9 @@ static void process_detach(void) /* cleanup GDI */ X11DRV_GDI_Finalize(); + /* cleanup DIB handling */ + RemoveVectoredExceptionHandler( X11DRV_DIB_FaultHandler ); + DeleteCriticalSection( &X11DRV_CritSection ); } -- Dimi.
Dimitrie O. Paun wrote:
On Thu, Apr 14, 2005 at 03:05:36PM +0200, Alexandre Julliard wrote:
Actually it should be possible to handle the fault using a vectored handler, without requiring internal functions at all.
Completely untested (what do people use to test DIB handling?), but it compiles. Is something like this what you had it mind?
+/*********************************************************************** + * mem_area_add + * + * Adds a memory area to the list. + * + * PARAMS + * base [I] Beginning address of the area + * size [I] Size of the area + * data [I] Data associated with the area + * + */ +static struct mem_area *mem_area_add( const void *base, UINT size, void *data ) +{ + struct mem_area *area; + + EnterCriticalSection( &csMemAreas ); + + area = HeapAlloc(GetProcessHeap(), 0, sizeof(*area));
You can put the HeapAlloc outside of the critical section.
+ if (area) + { + area->base = base; + area->size = size; + area->data = data; + list_add_head( &mem_areas_list, &area->entry ); + } + + LeaveCriticalSection( &csMemAreas ); + + return area; +} + + +/*********************************************************************** + * mem_area_del + * + * Removes a memory area from the list. + * + * PARAMS + * addr [I] Address + * + * RETURNS + * Success: TRUE + * Failure: FALSE + */ +static BOOL mem_area_del( const void *addr ) +{ + struct mem_area *area; + + EnterCriticalSection( &csMemAreas ); + area = mem_area_find( addr ); + if (area) list_remove( &area->entry ); + LeaveCriticalSection( &csMemAreas );
Why don't you free area here?
+ + return area != NULL; +} + + /* Some of the following helper functions are duplicated in dlls/gdi/dib.c
Rob
On Sun, Apr 17, 2005 at 08:44:57AM -0500, Rob Shearman wrote:
Why don't you free area here?
Details, details. But if you insist ... :) Index: dlls/ntdll/ntdll.spec =================================================================== RCS file: /var/cvs/wine/dlls/ntdll/ntdll.spec,v retrieving revision 1.175 diff -u -p -r1.175 ntdll.spec --- dlls/ntdll/ntdll.spec 30 Mar 2005 10:22:51 -0000 1.175 +++ dlls/ntdll/ntdll.spec 17 Apr 2005 04:18:44 -0000 @@ -1035,4 +1035,3 @@ @ cdecl MODULE_DllThreadAttach(ptr) @ cdecl MODULE_GetLoadOrderW(ptr wstr wstr) @ cdecl VERSION_Init(wstr) -@ cdecl VIRTUAL_SetFaultHandler(ptr ptr ptr) Index: dlls/ntdll/virtual.c =================================================================== RCS file: /var/cvs/wine/dlls/ntdll/virtual.c,v retrieving revision 1.46 diff -u -p -r1.46 virtual.c --- dlls/ntdll/virtual.c 22 Feb 2005 19:33:50 -0000 1.46 +++ dlls/ntdll/virtual.c 17 Apr 2005 04:20:14 -0000 @@ -1098,25 +1104,6 @@ void virtual_init(void) /*********************************************************************** - * VIRTUAL_SetFaultHandler - */ -BOOL VIRTUAL_SetFaultHandler( LPCVOID addr, HANDLERPROC proc, LPVOID arg ) -{ - FILE_VIEW *view; - BOOL ret = FALSE; - - RtlEnterCriticalSection( &csVirtual ); - if ((view = VIRTUAL_FindView( addr ))) - { - view->handlerProc = proc; - view->handlerArg = arg; - ret = TRUE; - } - RtlLeaveCriticalSection( &csVirtual ); - return ret; -} - -/*********************************************************************** * VIRTUAL_HandleFault */ DWORD VIRTUAL_HandleFault( LPCVOID addr ) Index: dlls/x11drv/dib.c =================================================================== RCS file: /var/cvs/wine/dlls/x11drv/dib.c,v retrieving revision 1.35 diff -u -p -r1.35 dib.c --- dlls/x11drv/dib.c 14 Apr 2005 12:48:31 -0000 1.35 +++ dlls/x11drv/dib.c 17 Apr 2005 15:10:03 -0000 @@ -38,11 +38,32 @@ #include "winbase.h" #include "wingdi.h" #include "x11drv.h" +#include "excpt.h" +#include "wine/list.h" #include "wine/debug.h" WINE_DEFAULT_DEBUG_CHANNEL(bitmap); WINE_DECLARE_DEBUG_CHANNEL(x11drv); +struct mem_area +{ + struct list entry; /* Entry in global mem area list */ + const void *base; /* Base address */ + UINT size; /* Size in bytes */ + void *data; /* Data associated with this area */ +}; + +static struct list mem_areas_list = LIST_INIT(mem_areas_list); + +static CRITICAL_SECTION csMemAreas; +static CRITICAL_SECTION_DEBUG csMemAreas_debug = +{ + 0, 0, &csMemAreas, + { &csMemAreas_debug.ProcessLocksList, &csMemAreas_debug.ProcessLocksList }, + 0, 0, { 0, (DWORD)(__FILE__ ": csMemAreas") } +}; +static CRITICAL_SECTION csMemAreas = { &csMemAreas_debug, -1, 0, 0, 0, 0 }; + static int ximageDepthTable[32]; /* This structure holds the arguments for DIB_SetImageBits() */ @@ -88,6 +109,89 @@ static INT X11DRV_DIB_Coerce(X_PHYSBITMA static INT X11DRV_DIB_Lock(X_PHYSBITMAP *,INT,BOOL); static void X11DRV_DIB_Unlock(X_PHYSBITMAP *,BOOL); +/*********************************************************************** + * mem_area_find + * + * Find the memory area containing a given address. + * The csMemAreas section must be held by caller. + * + * PARAMS + * addr [I] Address + * + * RETURNS + * Success: area + * Failure: NULL + */ +static struct mem_area *mem_area_find( const void *addr ) +{ + struct list *ptr; + + LIST_FOR_EACH( ptr, &mem_areas_list ) + { + struct mem_area *area = LIST_ENTRY( ptr, struct mem_area, entry ); + if (area->base > addr) break; + if ((const char*)addr < (const char*)area->base + area->size) return area; + } + return NULL; +} + +/*********************************************************************** + * mem_area_add + * + * Adds a memory area to the list. + * + * PARAMS + * base [I] Beginning address of the area + * size [I] Size of the area + * data [I] Data associated with the area + * + */ +static struct mem_area *mem_area_add( const void *base, UINT size, void *data ) +{ + struct mem_area *area; + + area = HeapAlloc(GetProcessHeap(), 0, sizeof(*area)); + if (!area) return NULL; + + area->base = base; + area->size = size; + area->data = data; + + EnterCriticalSection( &csMemAreas ); + list_add_head( &mem_areas_list, &area->entry ); + LeaveCriticalSection( &csMemAreas ); + + return area; +} + + +/*********************************************************************** + * mem_area_del + * + * Removes a memory area from the list. + * + * PARAMS + * addr [I] Address + * + * RETURNS + * Success: TRUE + * Failure: FALSE + */ +static BOOL mem_area_del( const void *addr ) +{ + struct mem_area *area; + + EnterCriticalSection( &csMemAreas ); + area = mem_area_find( addr ); + if (area) list_remove( &area->entry ); + LeaveCriticalSection( &csMemAreas ); + + HeapFree(GetProcessHeap(), 0, area); + + return area != NULL; +} + + /* Some of the following helper functions are duplicated in dlls/gdi/dib.c @@ -4268,22 +4372,34 @@ static void X11DRV_DIB_DoUpdateDIBSectio /*********************************************************************** * X11DRV_DIB_FaultHandler */ -static BOOL X11DRV_DIB_FaultHandler( LPVOID res, LPCVOID addr ) +LONG CALLBACK X11DRV_DIB_FaultHandler( PEXCEPTION_POINTERS ep ) { - X_PHYSBITMAP *physBitmap = res; - INT state = X11DRV_DIB_Lock( physBitmap, DIB_Status_None, FALSE ); + struct mem_area *area; + X_PHYSBITMAP *physBitmap; + INT state; - if (state != DIB_Status_InSync) { - /* no way to tell whether app needs read or write yet, - * try read first */ - X11DRV_DIB_Coerce( physBitmap, DIB_Status_InSync, FALSE ); - } else { - /* hm, apparently the app must have write access */ - X11DRV_DIB_Coerce( physBitmap, DIB_Status_AppMod, FALSE ); - } - X11DRV_DIB_Unlock( physBitmap, TRUE ); + if (ep->ExceptionRecord->ExceptionCode != EXCEPTION_ACCESS_VIOLATION) + return EXCEPTION_CONTINUE_SEARCH; - return TRUE; + EnterCriticalSection(&csMemAreas); + area = mem_area_find(ep->ExceptionRecord->ExceptionAddress); + if (area) physBitmap = (X_PHYSBITMAP *)area->data; + else physBitmap = NULL; + LeaveCriticalSection(&csMemAreas); + + if (!physBitmap) return EXCEPTION_CONTINUE_SEARCH; + + state = X11DRV_DIB_Lock( physBitmap, DIB_Status_None, FALSE ); + if (state != DIB_Status_InSync) { + /* no way to tell whether app needs read or write yet, try read first */ + X11DRV_DIB_Coerce( physBitmap, DIB_Status_InSync, FALSE ); + } else { + /* hm, apparently the app must have write access */ + X11DRV_DIB_Coerce( physBitmap, DIB_Status_AppMod, FALSE ); + } + X11DRV_DIB_Unlock( physBitmap, TRUE ); + + return EXCEPTION_CONTINUE_EXECUTION; } /*********************************************************************** @@ -4566,7 +4682,6 @@ static XImage *X11DRV_XShmCreateImage( i HBITMAP X11DRV_CreateDIBSection( X11DRV_PDEVICE *physDev, HBITMAP hbitmap, const BITMAPINFO *bmi, UINT usage ) { - extern BOOL VIRTUAL_SetFaultHandler(LPCVOID addr, BOOL (*proc)(LPVOID, LPCVOID), LPVOID arg); X_PHYSBITMAP *physBitmap; DIBSECTION dib; @@ -4602,7 +4717,7 @@ HBITMAP X11DRV_CreateDIBSection( X11DRV_ /* install fault handler */ InitializeCriticalSection( &physBitmap->lock ); - if (VIRTUAL_SetFaultHandler(dib.dsBm.bmBits, X11DRV_DIB_FaultHandler, physBitmap)) + if (mem_area_add(dib.dsBm.bmBits, dib.dsBmih.biSize, physBitmap)) { X11DRV_DIB_DoProtectDIBSection( physBitmap, PAGE_READWRITE ); physBitmap->status = DIB_Status_AppMod; @@ -4616,6 +4731,8 @@ HBITMAP X11DRV_CreateDIBSection( X11DRV_ */ void X11DRV_DIB_DeleteDIBSection(X_PHYSBITMAP *physBitmap, DIBSECTION *dib) { + mem_area_del(dib->dsBm.bmBits); + if (dib->dshSection) X11DRV_DIB_Coerce(physBitmap, DIB_Status_InSync, FALSE); Index: dlls/x11drv/x11drv.h =================================================================== RCS file: /var/cvs/wine/dlls/x11drv/x11drv.h,v retrieving revision 1.69 diff -u -p -r1.69 x11drv.h --- dlls/x11drv/x11drv.h 14 Apr 2005 12:48:11 -0000 1.69 +++ dlls/x11drv/x11drv.h 17 Apr 2005 03:42:32 -0000 @@ -234,6 +234,7 @@ extern BOOL X11DRV_SwapBuffers(X11DRV_PD extern void X11DRV_BITMAP_Init(void); extern void X11DRV_FONT_Init( int log_pixels_x, int log_pixels_y ); +extern LONG CALLBACK X11DRV_DIB_FaultHandler( PEXCEPTION_POINTERS ep ); extern int X11DRV_DIB_BitmapInfoSize( const BITMAPINFO * info, WORD coloruse ); extern XImage *X11DRV_DIB_CreateXImage( int width, int height, int depth ); extern HGLOBAL X11DRV_DIB_CreateDIBFromBitmap(HDC hdc, HBITMAP hBmp); Index: dlls/x11drv/x11drv_main.c =================================================================== RCS file: /var/cvs/wine/dlls/x11drv/x11drv_main.c,v retrieving revision 1.102 diff -u -p -r1.102 x11drv_main.c --- dlls/x11drv/x11drv_main.c 14 Apr 2005 12:48:11 -0000 1.102 +++ dlls/x11drv/x11drv_main.c 16 Apr 2005 16:59:03 -0000 @@ -362,6 +362,9 @@ static BOOL process_attach(void) using_wine_desktop = 1; } + /* initialize DIB handling */ + AddVectoredExceptionHandler( TRUE, X11DRV_DIB_FaultHandler ); + /* initialize GDI */ X11DRV_GDI_Initialize( display ); @@ -422,6 +425,9 @@ static void process_detach(void) /* cleanup GDI */ X11DRV_GDI_Finalize(); + /* cleanup DIB handling */ + RemoveVectoredExceptionHandler( X11DRV_DIB_FaultHandler ); + DeleteCriticalSection( &X11DRV_CritSection ); } -- Dimi.
"Dimitrie O. Paun" <dpaun(a)rogers.com> wrote:
+struct mem_area +{ + struct list entry; /* Entry in global mem area list */ + const void *base; /* Base address */ + UINT size; /* Size in bytes */ + void *data; /* Data associated with this area */ +};
One more nitpick: it's better to change the size type to SIZE_T from UINT to allow 64-bit size memory allocations on (future) Win64. -- Dmitry.
On Mon, Apr 18, 2005 at 12:29:07AM +0900, Dmitry Timoshkov wrote:
"Dimitrie O. Paun" <dpaun(a)rogers.com> wrote:
+struct mem_area +{ + struct list entry; /* Entry in global mem area list */ + const void *base; /* Base address */ + UINT size; /* Size in bytes */ + void *data; /* Data associated with this area */ +};
One more nitpick: it's better to change the size type to SIZE_T from UINT to allow 64-bit size memory allocations on (future) Win64.
Yeah, and on second thought even this "mem_area" is too general because they are build specifically for DIBs. I should rename them dib_area, and instead of "void *data", I should have "X_PHYSBITMAP *physBitmap". But I'd like to hear from AJ is this is OK first :) -- Dimi.
"Dimitrie O. Paun" <dpaun(a)rogers.com> writes:
Yeah, and on second thought even this "mem_area" is too general because they are build specifically for DIBs. I should rename them dib_area, and instead of "void *data", I should have "X_PHYSBITMAP *physBitmap".
But I'd like to hear from AJ is this is OK first :)
Sure, the general idea is fine. You actually don't need a mem_area structure at all, you can store that directly in the phys bitmap. Also it would be nice to only set the handler when a DIB is allocated, not at startup. -- Alexandre Julliard julliard(a)winehq.org
On Mon, Apr 18, 2005 at 01:06:23PM +0200, Alexandre Julliard wrote:
Sure, the general idea is fine. You actually don't need a mem_area structure at all, you can store that directly in the phys bitmap. Also it would be nice to only set the handler when a DIB is allocated, not at startup.
Right, I realized that after sending the email about making the data field a pointer to physBitmap. Compiles, but not tested, as I don't have a DIB using app. ChangeLog Remove the VIRTUAL_SetFaultHandler(), use vectored exceptions instead. Index: dlls/ntdll/ntdll.spec =================================================================== RCS file: /var/cvs/wine/dlls/ntdll/ntdll.spec,v retrieving revision 1.177 diff -u -p -r1.177 ntdll.spec --- dlls/ntdll/ntdll.spec 16 Apr 2005 11:19:27 -0000 1.177 +++ dlls/ntdll/ntdll.spec 18 Apr 2005 11:42:07 -0000 @@ -1036,4 +1036,3 @@ @ cdecl MODULE_DllThreadAttach(ptr) @ cdecl MODULE_GetLoadOrderW(ptr wstr wstr) @ cdecl VERSION_Init(wstr) -@ cdecl VIRTUAL_SetFaultHandler(ptr ptr ptr) Index: dlls/ntdll/virtual.c =================================================================== RCS file: /var/cvs/wine/dlls/ntdll/virtual.c,v retrieving revision 1.46 diff -u -p -r1.46 virtual.c --- dlls/ntdll/virtual.c 22 Feb 2005 19:33:50 -0000 1.46 +++ dlls/ntdll/virtual.c 17 Apr 2005 04:20:14 -0000 @@ -1098,25 +1104,6 @@ void virtual_init(void) /*********************************************************************** - * VIRTUAL_SetFaultHandler - */ -BOOL VIRTUAL_SetFaultHandler( LPCVOID addr, HANDLERPROC proc, LPVOID arg ) -{ - FILE_VIEW *view; - BOOL ret = FALSE; - - RtlEnterCriticalSection( &csVirtual ); - if ((view = VIRTUAL_FindView( addr ))) - { - view->handlerProc = proc; - view->handlerArg = arg; - ret = TRUE; - } - RtlLeaveCriticalSection( &csVirtual ); - return ret; -} - -/*********************************************************************** * VIRTUAL_HandleFault */ DWORD VIRTUAL_HandleFault( LPCVOID addr ) Index: dlls/x11drv/dib.c =================================================================== RCS file: /var/cvs/wine/dlls/x11drv/dib.c,v retrieving revision 1.35 diff -u -p -r1.35 dib.c --- dlls/x11drv/dib.c 14 Apr 2005 12:48:31 -0000 1.35 +++ dlls/x11drv/dib.c 18 Apr 2005 12:14:34 -0000 @@ -38,11 +38,25 @@ #include "winbase.h" #include "wingdi.h" #include "x11drv.h" +#include "excpt.h" #include "wine/debug.h" WINE_DEFAULT_DEBUG_CHANNEL(bitmap); WINE_DECLARE_DEBUG_CHANNEL(x11drv); +static struct list dibs_list = LIST_INIT(dibs_list); + +static CRITICAL_SECTION dibs_cs; +static CRITICAL_SECTION_DEBUG dibs_cs_debug = +{ + 0, 0, &dibs_cs, + { &dibs_cs_debug.ProcessLocksList, &dibs_cs_debug.ProcessLocksList }, + 0, 0, { 0, (DWORD)(__FILE__ ": dibs_cs") } +}; +static CRITICAL_SECTION dibs_cs = { &dibs_cs_debug, -1, 0, 0, 0, 0 }; + +PVOID dibs_handler = 0; + static int ximageDepthTable[32]; /* This structure holds the arguments for DIB_SetImageBits() */ @@ -4268,22 +4282,43 @@ static void X11DRV_DIB_DoUpdateDIBSectio /*********************************************************************** * X11DRV_DIB_FaultHandler */ -static BOOL X11DRV_DIB_FaultHandler( LPVOID res, LPCVOID addr ) +static LONG CALLBACK X11DRV_DIB_FaultHandler( PEXCEPTION_POINTERS ep ) { - X_PHYSBITMAP *physBitmap = res; - INT state = X11DRV_DIB_Lock( physBitmap, DIB_Status_None, FALSE ); + X_PHYSBITMAP *physBitmap = NULL; + BOOL found = FALSE; + struct list *ptr; + INT state; + + if (ep->ExceptionRecord->ExceptionCode != EXCEPTION_ACCESS_VIOLATION) + return EXCEPTION_CONTINUE_SEARCH; + + EnterCriticalSection(&dibs_cs); + LIST_FOR_EACH( ptr, &dibs_list ) + { + physBitmap = LIST_ENTRY( ptr, X_PHYSBITMAP, entry ); + if (physBitmap->base > ep->ExceptionRecord->ExceptionAddress) break; + if ((const char*)ep->ExceptionRecord->ExceptionAddress < + (const char*)physBitmap->base + physBitmap->size) + { + found = TRUE; + break; + } + } + LeaveCriticalSection(&dibs_cs); - if (state != DIB_Status_InSync) { - /* no way to tell whether app needs read or write yet, - * try read first */ - X11DRV_DIB_Coerce( physBitmap, DIB_Status_InSync, FALSE ); - } else { - /* hm, apparently the app must have write access */ - X11DRV_DIB_Coerce( physBitmap, DIB_Status_AppMod, FALSE ); - } - X11DRV_DIB_Unlock( physBitmap, TRUE ); + if (!found) return EXCEPTION_CONTINUE_SEARCH; - return TRUE; + state = X11DRV_DIB_Lock( physBitmap, DIB_Status_None, FALSE ); + if (state != DIB_Status_InSync) { + /* no way to tell whether app needs read or write yet, try read first */ + X11DRV_DIB_Coerce( physBitmap, DIB_Status_InSync, FALSE ); + } else { + /* hm, apparently the app must have write access */ + X11DRV_DIB_Coerce( physBitmap, DIB_Status_AppMod, FALSE ); + } + X11DRV_DIB_Unlock( physBitmap, TRUE ); + + return EXCEPTION_CONTINUE_EXECUTION; } /*********************************************************************** @@ -4566,7 +4601,6 @@ static XImage *X11DRV_XShmCreateImage( i HBITMAP X11DRV_CreateDIBSection( X11DRV_PDEVICE *physDev, HBITMAP hbitmap, const BITMAPINFO *bmi, UINT usage ) { - extern BOOL VIRTUAL_SetFaultHandler(LPCVOID addr, BOOL (*proc)(LPVOID, LPCVOID), LPVOID arg); X_PHYSBITMAP *physBitmap; DIBSECTION dib; @@ -4602,11 +4636,18 @@ HBITMAP X11DRV_CreateDIBSection( X11DRV_ /* install fault handler */ InitializeCriticalSection( &physBitmap->lock ); - if (VIRTUAL_SetFaultHandler(dib.dsBm.bmBits, X11DRV_DIB_FaultHandler, physBitmap)) - { - X11DRV_DIB_DoProtectDIBSection( physBitmap, PAGE_READWRITE ); - physBitmap->status = DIB_Status_AppMod; - } + + physBitmap->base = dib.dsBm.bmBits; + physBitmap->size = dib.dsBmih.biSize; + physBitmap->status = DIB_Status_AppMod; + + EnterCriticalSection( &dibs_cs ); + list_add_head( &dibs_list, &physBitmap->entry ); + if (!dibs_handler) + dibs_handler = AddVectoredExceptionHandler( TRUE, X11DRV_DIB_FaultHandler ); + LeaveCriticalSection( &dibs_cs ); + + X11DRV_DIB_DoProtectDIBSection( physBitmap, PAGE_READWRITE ); return hbitmap; } @@ -4616,6 +4657,10 @@ HBITMAP X11DRV_CreateDIBSection( X11DRV_ */ void X11DRV_DIB_DeleteDIBSection(X_PHYSBITMAP *physBitmap, DIBSECTION *dib) { + EnterCriticalSection( &dibs_cs ); + list_remove( &physBitmap->entry ); + LeaveCriticalSection( &dibs_cs ); + if (dib->dshSection) X11DRV_DIB_Coerce(physBitmap, DIB_Status_InSync, FALSE); Index: dlls/x11drv/x11drv.h =================================================================== RCS file: /var/cvs/wine/dlls/x11drv/x11drv.h,v retrieving revision 1.69 diff -u -p -r1.69 x11drv.h --- dlls/x11drv/x11drv.h 14 Apr 2005 12:48:11 -0000 1.69 +++ dlls/x11drv/x11drv.h 18 Apr 2005 12:13:43 -0000 @@ -64,6 +64,7 @@ typedef int Status; #include "winuser.h" #include "ddrawi.h" #include "thread.h" +#include "wine/list.h" #define MAX_PIXELFORMATS 8 @@ -111,6 +112,9 @@ typedef struct #ifdef HAVE_LIBXXSHM XShmSegmentInfo shminfo; /* shared memory segment info */ #endif + struct list entry; /* Entry in global DIB list */ + const void *base; /* Base address */ + SIZE_T size; /* Size in bytes */ } X_PHYSBITMAP; /* X physical font */ @@ -147,6 +151,7 @@ typedef struct /* GCs used for B&W and color bitmap operations */ extern GC BITMAP_monoGC, BITMAP_colorGC; extern X_PHYSBITMAP BITMAP_stock_phys_bitmap; /* phys bitmap for the default stock bitmap */ +extern PVOID dibs_handler; #define BITMAP_GC(physBitmap) (((physBitmap)->pixmap_depth == 1) ? BITMAP_monoGC : BITMAP_colorGC) Index: dlls/x11drv/x11drv_main.c =================================================================== RCS file: /var/cvs/wine/dlls/x11drv/x11drv_main.c,v retrieving revision 1.102 diff -u -p -r1.102 x11drv_main.c --- dlls/x11drv/x11drv_main.c 14 Apr 2005 12:48:11 -0000 1.102 +++ dlls/x11drv/x11drv_main.c 18 Apr 2005 12:06:15 -0000 @@ -422,6 +422,9 @@ static void process_detach(void) /* cleanup GDI */ X11DRV_GDI_Finalize(); + /* cleanup DIB handling */ + if (dibs_handler) RemoveVectoredExceptionHandler( dibs_handler ); + DeleteCriticalSection( &X11DRV_CritSection ); } -- Dimi.
"Dimitrie O. Paun" <dpaun(a)rogers.com> wrote:
Right, I realized that after sending the email about making the data field a pointer to physBitmap. Compiles, but not tested, as I don't have a DIB using app.
Here is a largely simplified source ripped from one of my very old projects which loads a TGA file and shows it using a DIB section. A sample TGA file is included. -- Dmitry.
On Mon, Apr 18, 2005 at 10:46:10PM +0900, Dmitry Timoshkov wrote:
Here is a largely simplified source ripped from one of my very old projects which loads a TGA file and shows it using a DIB section. A sample TGA file is included.
Thank you Dmitry. It didn't test the fault handler directly, but it was easy to modify it to do so. Now with working code... (found one small, but essential bug: the ExceptionAddress is the address of the code, not of the data, that's in ExceptionInformation. Oh well :)). ChangeLog Use vectored exception handling to get rid of VIRTUAL_SetFaultHandler(). Index: dlls/ntdll/ntdll.spec =================================================================== RCS file: /var/cvs/wine/dlls/ntdll/ntdll.spec,v retrieving revision 1.177 diff -u -p -r1.177 ntdll.spec --- dlls/ntdll/ntdll.spec 16 Apr 2005 11:19:27 -0000 1.177 +++ dlls/ntdll/ntdll.spec 18 Apr 2005 11:42:07 -0000 @@ -1036,4 +1036,3 @@ @ cdecl MODULE_DllThreadAttach(ptr) @ cdecl MODULE_GetLoadOrderW(ptr wstr wstr) @ cdecl VERSION_Init(wstr) -@ cdecl VIRTUAL_SetFaultHandler(ptr ptr ptr) Index: dlls/ntdll/virtual.c =================================================================== RCS file: /var/cvs/wine/dlls/ntdll/virtual.c,v retrieving revision 1.46 diff -u -p -r1.46 virtual.c --- dlls/ntdll/virtual.c 22 Feb 2005 19:33:50 -0000 1.46 +++ dlls/ntdll/virtual.c 17 Apr 2005 04:20:14 -0000 @@ -1098,25 +1104,6 @@ void virtual_init(void) /*********************************************************************** - * VIRTUAL_SetFaultHandler - */ -BOOL VIRTUAL_SetFaultHandler( LPCVOID addr, HANDLERPROC proc, LPVOID arg ) -{ - FILE_VIEW *view; - BOOL ret = FALSE; - - RtlEnterCriticalSection( &csVirtual ); - if ((view = VIRTUAL_FindView( addr ))) - { - view->handlerProc = proc; - view->handlerArg = arg; - ret = TRUE; - } - RtlLeaveCriticalSection( &csVirtual ); - return ret; -} - -/*********************************************************************** * VIRTUAL_HandleFault */ DWORD VIRTUAL_HandleFault( LPCVOID addr ) Index: dlls/x11drv/dib.c =================================================================== RCS file: /var/cvs/wine/dlls/x11drv/dib.c,v retrieving revision 1.35 diff -u -p -r1.35 dib.c --- dlls/x11drv/dib.c 14 Apr 2005 12:48:31 -0000 1.35 +++ dlls/x11drv/dib.c 19 Apr 2005 01:45:59 -0000 @@ -38,11 +38,25 @@ #include "winbase.h" #include "wingdi.h" #include "x11drv.h" +#include "excpt.h" #include "wine/debug.h" WINE_DEFAULT_DEBUG_CHANNEL(bitmap); WINE_DECLARE_DEBUG_CHANNEL(x11drv); +static struct list dibs_list = LIST_INIT(dibs_list); + +static CRITICAL_SECTION dibs_cs; +static CRITICAL_SECTION_DEBUG dibs_cs_debug = +{ + 0, 0, &dibs_cs, + { &dibs_cs_debug.ProcessLocksList, &dibs_cs_debug.ProcessLocksList }, + 0, 0, { 0, (DWORD)(__FILE__ ": dibs_cs") } +}; +static CRITICAL_SECTION dibs_cs = { &dibs_cs_debug, -1, 0, 0, 0, 0 }; + +PVOID dibs_handler = 0; + static int ximageDepthTable[32]; /* This structure holds the arguments for DIB_SetImageBits() */ @@ -4268,22 +4282,45 @@ static void X11DRV_DIB_DoUpdateDIBSectio /*********************************************************************** * X11DRV_DIB_FaultHandler */ -static BOOL X11DRV_DIB_FaultHandler( LPVOID res, LPCVOID addr ) +static LONG CALLBACK X11DRV_DIB_FaultHandler( PEXCEPTION_POINTERS ep ) { - X_PHYSBITMAP *physBitmap = res; - INT state = X11DRV_DIB_Lock( physBitmap, DIB_Status_None, FALSE ); + X_PHYSBITMAP *physBitmap = NULL; + BOOL found = FALSE; + const BYTE *addr; + struct list *ptr; + INT state; - if (state != DIB_Status_InSync) { - /* no way to tell whether app needs read or write yet, - * try read first */ - X11DRV_DIB_Coerce( physBitmap, DIB_Status_InSync, FALSE ); - } else { - /* hm, apparently the app must have write access */ - X11DRV_DIB_Coerce( physBitmap, DIB_Status_AppMod, FALSE ); - } - X11DRV_DIB_Unlock( physBitmap, TRUE ); + if (ep->ExceptionRecord->ExceptionCode != EXCEPTION_ACCESS_VIOLATION) + return EXCEPTION_CONTINUE_SEARCH; + + addr = (const BYTE*)ep->ExceptionRecord->ExceptionInformation[1]; + + EnterCriticalSection(&dibs_cs); + LIST_FOR_EACH( ptr, &dibs_list ) + { + physBitmap = LIST_ENTRY( ptr, X_PHYSBITMAP, entry ); + if (physBitmap->base > addr) break; + if (addr < physBitmap->base + physBitmap->size) + { + found = TRUE; + break; + } + } + LeaveCriticalSection(&dibs_cs); + + if (!found) return EXCEPTION_CONTINUE_SEARCH; + + state = X11DRV_DIB_Lock( physBitmap, DIB_Status_None, FALSE ); + if (state != DIB_Status_InSync) { + /* no way to tell whether app needs read or write yet, try read first */ + X11DRV_DIB_Coerce( physBitmap, DIB_Status_InSync, FALSE ); + } else { + /* hm, apparently the app must have write access */ + X11DRV_DIB_Coerce( physBitmap, DIB_Status_AppMod, FALSE ); + } + X11DRV_DIB_Unlock( physBitmap, TRUE ); - return TRUE; + return EXCEPTION_CONTINUE_EXECUTION; } /*********************************************************************** @@ -4566,7 +4603,6 @@ static XImage *X11DRV_XShmCreateImage( i HBITMAP X11DRV_CreateDIBSection( X11DRV_PDEVICE *physDev, HBITMAP hbitmap, const BITMAPINFO *bmi, UINT usage ) { - extern BOOL VIRTUAL_SetFaultHandler(LPCVOID addr, BOOL (*proc)(LPVOID, LPCVOID), LPVOID arg); X_PHYSBITMAP *physBitmap; DIBSECTION dib; @@ -4602,11 +4638,18 @@ HBITMAP X11DRV_CreateDIBSection( X11DRV_ /* install fault handler */ InitializeCriticalSection( &physBitmap->lock ); - if (VIRTUAL_SetFaultHandler(dib.dsBm.bmBits, X11DRV_DIB_FaultHandler, physBitmap)) - { - X11DRV_DIB_DoProtectDIBSection( physBitmap, PAGE_READWRITE ); - physBitmap->status = DIB_Status_AppMod; - } + + physBitmap->base = dib.dsBm.bmBits; + physBitmap->size = dib.dsBmih.biSizeImage; + physBitmap->status = DIB_Status_AppMod; + + EnterCriticalSection( &dibs_cs ); + list_add_head( &dibs_list, &physBitmap->entry ); + if (!dibs_handler) + dibs_handler = AddVectoredExceptionHandler( TRUE, X11DRV_DIB_FaultHandler ); + LeaveCriticalSection( &dibs_cs ); + + X11DRV_DIB_DoProtectDIBSection( physBitmap, PAGE_READWRITE ); return hbitmap; } @@ -4616,6 +4659,10 @@ HBITMAP X11DRV_CreateDIBSection( X11DRV_ */ void X11DRV_DIB_DeleteDIBSection(X_PHYSBITMAP *physBitmap, DIBSECTION *dib) { + EnterCriticalSection( &dibs_cs ); + list_remove( &physBitmap->entry ); + LeaveCriticalSection( &dibs_cs ); + if (dib->dshSection) X11DRV_DIB_Coerce(physBitmap, DIB_Status_InSync, FALSE); Index: dlls/x11drv/x11drv.h =================================================================== RCS file: /var/cvs/wine/dlls/x11drv/x11drv.h,v retrieving revision 1.69 diff -u -p -r1.69 x11drv.h --- dlls/x11drv/x11drv.h 14 Apr 2005 12:48:11 -0000 1.69 +++ dlls/x11drv/x11drv.h 19 Apr 2005 01:44:27 -0000 @@ -64,6 +64,7 @@ typedef int Status; #include "winuser.h" #include "ddrawi.h" #include "thread.h" +#include "wine/list.h" #define MAX_PIXELFORMATS 8 @@ -111,6 +112,9 @@ typedef struct #ifdef HAVE_LIBXXSHM XShmSegmentInfo shminfo; /* shared memory segment info */ #endif + struct list entry; /* Entry in global DIB list */ + const BYTE *base; /* Base address */ + SIZE_T size; /* Size in bytes */ } X_PHYSBITMAP; /* X physical font */ @@ -147,6 +151,7 @@ typedef struct /* GCs used for B&W and color bitmap operations */ extern GC BITMAP_monoGC, BITMAP_colorGC; extern X_PHYSBITMAP BITMAP_stock_phys_bitmap; /* phys bitmap for the default stock bitmap */ +extern PVOID dibs_handler; #define BITMAP_GC(physBitmap) (((physBitmap)->pixmap_depth == 1) ? BITMAP_monoGC : BITMAP_colorGC) Index: dlls/x11drv/x11drv_main.c =================================================================== RCS file: /var/cvs/wine/dlls/x11drv/x11drv_main.c,v retrieving revision 1.102 diff -u -p -r1.102 x11drv_main.c --- dlls/x11drv/x11drv_main.c 14 Apr 2005 12:48:11 -0000 1.102 +++ dlls/x11drv/x11drv_main.c 18 Apr 2005 12:06:15 -0000 @@ -422,6 +422,9 @@ static void process_detach(void) /* cleanup GDI */ X11DRV_GDI_Finalize(); + /* cleanup DIB handling */ + if (dibs_handler) RemoveVectoredExceptionHandler( dibs_handler ); + DeleteCriticalSection( &X11DRV_CritSection ); } -- Dimi.
"Dimitrie O. Paun" <dpaun(a)rogers.com> writes:
+ EnterCriticalSection(&dibs_cs); + LIST_FOR_EACH( ptr, &dibs_list ) + { + physBitmap = LIST_ENTRY( ptr, X_PHYSBITMAP, entry ); + if (physBitmap->base > addr) break;
If you want to short-circuit the search this way you need to sort the list by addresses.
+ state = X11DRV_DIB_Lock( physBitmap, DIB_Status_None, FALSE ); + if (state != DIB_Status_InSync) { + /* no way to tell whether app needs read or write yet, try read first */
Actually we can tell now that we have the full exception information.
+ EnterCriticalSection( &dibs_cs ); + list_add_head( &dibs_list, &physBitmap->entry ); + if (!dibs_handler) + dibs_handler = AddVectoredExceptionHandler( TRUE, X11DRV_DIB_FaultHandler ); + LeaveCriticalSection( &dibs_cs );
You can't hold the critsection when adding the handler since this will grab the vectored_handlers critsection, and thus acquire the sections in the reverse order of what happens when the handler is called. -- Alexandre Julliard julliard(a)winehq.org
On Tue, Apr 19, 2005 at 01:13:12PM +0200, Alexandre Julliard wrote:
You can't hold the critsection when adding the handler since this will grab the vectored_handlers critsection, and thus acquire the sections in the reverse order of what happens when the handler is called.
Good point, I haven't even considered it. Thanks for catching this nasty. As for the list, I'm not sure it's worth sorting, that optimizes the slow path anyway. Maybe it's more advantageous to keep the most recently used DIB at the front of the list. If this is a problem, we can have a more complicated struture to do a more clever search, but this is a different patch. ChangeLog Use vectored exceptions to get rid of VIRTUAL_SetFaultHandler(). Index: dlls/ntdll/ntdll.spec =================================================================== RCS file: /var/cvs/wine/dlls/ntdll/ntdll.spec,v retrieving revision 1.177 diff -u -p -r1.177 ntdll.spec --- dlls/ntdll/ntdll.spec 16 Apr 2005 11:19:27 -0000 1.177 +++ dlls/ntdll/ntdll.spec 18 Apr 2005 11:42:07 -0000 @@ -1036,4 +1036,3 @@ @ cdecl MODULE_DllThreadAttach(ptr) @ cdecl MODULE_GetLoadOrderW(ptr wstr wstr) @ cdecl VERSION_Init(wstr) -@ cdecl VIRTUAL_SetFaultHandler(ptr ptr ptr) Index: dlls/ntdll/virtual.c =================================================================== RCS file: /var/cvs/wine/dlls/ntdll/virtual.c,v retrieving revision 1.46 diff -u -p -r1.46 virtual.c --- dlls/ntdll/virtual.c 22 Feb 2005 19:33:50 -0000 1.46 +++ dlls/ntdll/virtual.c 17 Apr 2005 04:20:14 -0000 @@ -1098,25 +1104,6 @@ void virtual_init(void) /*********************************************************************** - * VIRTUAL_SetFaultHandler - */ -BOOL VIRTUAL_SetFaultHandler( LPCVOID addr, HANDLERPROC proc, LPVOID arg ) -{ - FILE_VIEW *view; - BOOL ret = FALSE; - - RtlEnterCriticalSection( &csVirtual ); - if ((view = VIRTUAL_FindView( addr ))) - { - view->handlerProc = proc; - view->handlerArg = arg; - ret = TRUE; - } - RtlLeaveCriticalSection( &csVirtual ); - return ret; -} - -/*********************************************************************** * VIRTUAL_HandleFault */ DWORD VIRTUAL_HandleFault( LPCVOID addr ) Index: dlls/x11drv/dib.c =================================================================== RCS file: /var/cvs/wine/dlls/x11drv/dib.c,v retrieving revision 1.35 diff -u -p -r1.35 dib.c --- dlls/x11drv/dib.c 14 Apr 2005 12:48:31 -0000 1.35 +++ dlls/x11drv/dib.c 19 Apr 2005 12:26:23 -0000 @@ -38,11 +38,25 @@ #include "winbase.h" #include "wingdi.h" #include "x11drv.h" +#include "excpt.h" #include "wine/debug.h" WINE_DEFAULT_DEBUG_CHANNEL(bitmap); WINE_DECLARE_DEBUG_CHANNEL(x11drv); +static struct list dibs_list = LIST_INIT(dibs_list); + +static CRITICAL_SECTION dibs_cs; +static CRITICAL_SECTION_DEBUG dibs_cs_debug = +{ + 0, 0, &dibs_cs, + { &dibs_cs_debug.ProcessLocksList, &dibs_cs_debug.ProcessLocksList }, + 0, 0, { 0, (DWORD)(__FILE__ ": dibs_cs") } +}; +static CRITICAL_SECTION dibs_cs = { &dibs_cs_debug, -1, 0, 0, 0, 0 }; + +PVOID dibs_handler = 0; + static int ximageDepthTable[32]; /* This structure holds the arguments for DIB_SetImageBits() */ @@ -4268,22 +4282,43 @@ static void X11DRV_DIB_DoUpdateDIBSectio /*********************************************************************** * X11DRV_DIB_FaultHandler */ -static BOOL X11DRV_DIB_FaultHandler( LPVOID res, LPCVOID addr ) +static LONG CALLBACK X11DRV_DIB_FaultHandler( PEXCEPTION_POINTERS ep ) { - X_PHYSBITMAP *physBitmap = res; - INT state = X11DRV_DIB_Lock( physBitmap, DIB_Status_None, FALSE ); + X_PHYSBITMAP *physBitmap = NULL; + BOOL found = FALSE; + const BYTE *addr; + struct list *ptr; - if (state != DIB_Status_InSync) { - /* no way to tell whether app needs read or write yet, - * try read first */ - X11DRV_DIB_Coerce( physBitmap, DIB_Status_InSync, FALSE ); - } else { - /* hm, apparently the app must have write access */ - X11DRV_DIB_Coerce( physBitmap, DIB_Status_AppMod, FALSE ); - } - X11DRV_DIB_Unlock( physBitmap, TRUE ); + if (ep->ExceptionRecord->ExceptionCode != EXCEPTION_ACCESS_VIOLATION) + return EXCEPTION_CONTINUE_SEARCH; + + addr = (const BYTE*)ep->ExceptionRecord->ExceptionInformation[1]; + + EnterCriticalSection(&dibs_cs); + LIST_FOR_EACH( ptr, &dibs_list ) + { + physBitmap = LIST_ENTRY( ptr, X_PHYSBITMAP, entry ); + if ((physBitmap->base <= addr) && (addr < physBitmap->base + physBitmap->size)) + { + found = TRUE; + break; + } + } + LeaveCriticalSection(&dibs_cs); + + if (!found) return EXCEPTION_CONTINUE_SEARCH; + + X11DRV_DIB_Lock( physBitmap, DIB_Status_None, FALSE ); + if (ep->ExceptionRecord->ExceptionInformation[0]) { + /* the app tried to write the DIB bits */ + X11DRV_DIB_Coerce( physBitmap, DIB_Status_AppMod, FALSE ); + } else { + /* the app tried to read the DIB bits */ + X11DRV_DIB_Coerce( physBitmap, DIB_Status_InSync, FALSE ); + } + X11DRV_DIB_Unlock( physBitmap, TRUE ); - return TRUE; + return EXCEPTION_CONTINUE_EXECUTION; } /*********************************************************************** @@ -4566,7 +4601,6 @@ static XImage *X11DRV_XShmCreateImage( i HBITMAP X11DRV_CreateDIBSection( X11DRV_PDEVICE *physDev, HBITMAP hbitmap, const BITMAPINFO *bmi, UINT usage ) { - extern BOOL VIRTUAL_SetFaultHandler(LPCVOID addr, BOOL (*proc)(LPVOID, LPCVOID), LPVOID arg); X_PHYSBITMAP *physBitmap; DIBSECTION dib; @@ -4602,11 +4636,18 @@ HBITMAP X11DRV_CreateDIBSection( X11DRV_ /* install fault handler */ InitializeCriticalSection( &physBitmap->lock ); - if (VIRTUAL_SetFaultHandler(dib.dsBm.bmBits, X11DRV_DIB_FaultHandler, physBitmap)) - { - X11DRV_DIB_DoProtectDIBSection( physBitmap, PAGE_READWRITE ); - physBitmap->status = DIB_Status_AppMod; - } + + physBitmap->base = dib.dsBm.bmBits; + physBitmap->size = dib.dsBmih.biSizeImage; + physBitmap->status = DIB_Status_AppMod; + + if (!dibs_handler) + dibs_handler = AddVectoredExceptionHandler( TRUE, X11DRV_DIB_FaultHandler ); + EnterCriticalSection( &dibs_cs ); + list_add_head( &dibs_list, &physBitmap->entry ); + LeaveCriticalSection( &dibs_cs ); + + X11DRV_DIB_DoProtectDIBSection( physBitmap, PAGE_READWRITE ); return hbitmap; } @@ -4616,6 +4657,10 @@ HBITMAP X11DRV_CreateDIBSection( X11DRV_ */ void X11DRV_DIB_DeleteDIBSection(X_PHYSBITMAP *physBitmap, DIBSECTION *dib) { + EnterCriticalSection( &dibs_cs ); + list_remove( &physBitmap->entry ); + LeaveCriticalSection( &dibs_cs ); + if (dib->dshSection) X11DRV_DIB_Coerce(physBitmap, DIB_Status_InSync, FALSE); Index: dlls/x11drv/x11drv.h =================================================================== RCS file: /var/cvs/wine/dlls/x11drv/x11drv.h,v retrieving revision 1.69 diff -u -p -r1.69 x11drv.h --- dlls/x11drv/x11drv.h 14 Apr 2005 12:48:11 -0000 1.69 +++ dlls/x11drv/x11drv.h 19 Apr 2005 01:44:27 -0000 @@ -64,6 +64,7 @@ typedef int Status; #include "winuser.h" #include "ddrawi.h" #include "thread.h" +#include "wine/list.h" #define MAX_PIXELFORMATS 8 @@ -111,6 +112,9 @@ typedef struct #ifdef HAVE_LIBXXSHM XShmSegmentInfo shminfo; /* shared memory segment info */ #endif + struct list entry; /* Entry in global DIB list */ + const BYTE *base; /* Base address */ + SIZE_T size; /* Size in bytes */ } X_PHYSBITMAP; /* X physical font */ @@ -147,6 +151,7 @@ typedef struct /* GCs used for B&W and color bitmap operations */ extern GC BITMAP_monoGC, BITMAP_colorGC; extern X_PHYSBITMAP BITMAP_stock_phys_bitmap; /* phys bitmap for the default stock bitmap */ +extern PVOID dibs_handler; #define BITMAP_GC(physBitmap) (((physBitmap)->pixmap_depth == 1) ? BITMAP_monoGC : BITMAP_colorGC) Index: dlls/x11drv/x11drv_main.c =================================================================== RCS file: /var/cvs/wine/dlls/x11drv/x11drv_main.c,v retrieving revision 1.102 diff -u -p -r1.102 x11drv_main.c --- dlls/x11drv/x11drv_main.c 14 Apr 2005 12:48:11 -0000 1.102 +++ dlls/x11drv/x11drv_main.c 18 Apr 2005 12:06:15 -0000 @@ -422,6 +422,9 @@ static void process_detach(void) /* cleanup GDI */ X11DRV_GDI_Finalize(); + /* cleanup DIB handling */ + if (dibs_handler) RemoveVectoredExceptionHandler( dibs_handler ); + DeleteCriticalSection( &X11DRV_CritSection ); } -- Dimi.
"Dimitrie O. Paun" <dpaun(a)rogers.com> writes:
On Tue, Apr 19, 2005 at 01:13:12PM +0200, Alexandre Julliard wrote:
You can't hold the critsection when adding the handler since this will grab the vectored_handlers critsection, and thus acquire the sections in the reverse order of what happens when the handler is called.
Good point, I haven't even considered it. Thanks for catching this nasty.
Actually you have the same problem with the GDI lock, this is going to be more tricky to solve... -- Alexandre Julliard julliard(a)winehq.org
On Tue, Apr 19, 2005 at 08:37:30PM +0200, Alexandre Julliard wrote:
Actually you have the same problem with the GDI lock, this is going to be more tricky to solve...
Duh! One way to do it is to not hold the lock while we call the handler. Which I think we need to do anyway, as app handlers are not under our control :) But to do so, we need to see what semantics we must have for the loop calling the handlers. If we drop the lock, the list can be changed while we iterate through it. A simple solution would be to detect such a change, and if it occurs, restart from the beginning? But I'm not sure that's going to cut it :) Or notice that we are handling a fault in Add/Remove and simply store the handler in another list that will be processed by the main handler after it has finished processing the current fault. -- Dimi.
participants (5)
-
Alexandre Julliard -
Dimitrie O. Paun -
Dmitry Timoshkov -
Eric Pouech -
Rob Shearman