[PATCH v2 0/1] MR1760: opengl32: Avoid calling functions that have not been queried yet.
The glMapBuffer and al. functions are all duplicated between EXT/ARB and core functions, and we reduced code duplication by redirecting the EXT to the core functions. The internal function table is only filled as the functions are queried, and it causes crahes when games are using the EXT functions without querying the core function first. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53990 -- v2: opengl32: Avoid calling functions that have not been queried yet. https://gitlab.winehq.org/wine/wine/-/merge_requests/1760
From: Rémi Bernon <rbernon(a)codeweavers.com> The glMapBuffer and al. functions are all duplicated between EXT/ARB and core functions, and we reduced code duplication by redirecting the EXT to the core functions. The internal function table is only filled as the functions are queried, and it causes crahes when games are using the EXT functions without querying the core function first. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53990 --- dlls/opengl32/unix_wgl.c | 105 ++++++++++++++++++++++++++++----------- dlls/opengl32/wgl.c | 61 ++++++++++++++++------- 2 files changed, 119 insertions(+), 47 deletions(-) diff --git a/dlls/opengl32/unix_wgl.c b/dlls/opengl32/unix_wgl.c index 6aa6ed9f708..24bd904b068 100644 --- a/dlls/opengl32/unix_wgl.c +++ b/dlls/opengl32/unix_wgl.c @@ -1743,6 +1743,22 @@ static void *get_named_buffer_pointer( TEB *teb, GLint buffer ) return ptr; } +static void unmap_buffer( TEB *teb, GLenum target ) +{ + const struct opengl_funcs *funcs = teb->glTable; + typeof(*funcs->ext.p_glUnmapBuffer) *func; + if (!(func = funcs->ext.p_glUnmapBuffer)) func = (void *)funcs->wgl.p_wglGetProcAddress( "glUnmapBuffer" ); + if (func) func( target ); +} + +static void unmap_named_buffer( TEB *teb, GLint buffer ) +{ + const struct opengl_funcs *funcs = teb->glTable; + typeof(*funcs->ext.p_glUnmapNamedBuffer) *func; + if (!(func = funcs->ext.p_glUnmapNamedBuffer)) func = (void *)funcs->wgl.p_wglGetProcAddress( "glUnmapNamedBuffer" ); + if (func) func( buffer ); +} + static NTSTATUS wow64_map_buffer( TEB *teb, GLint buffer, GLenum target, void *ptr, SIZE_T size, GLbitfield access, PTR32 *ret ) { @@ -1794,7 +1810,7 @@ static NTSTATUS wow64_unmap_buffer( void *ptr, SIZE_T size, GLbitfield access ) return STATUS_INVALID_ADDRESS; } -NTSTATUS wow64_ext_glGetBufferPointerv( void *args ) +static NTSTATUS wow64_gl_get_buffer_pointer_v( void *args, NTSTATUS (*get_buffer_pointer_v64)(void *) ) { PTR32 *ptr; /* pointer to the buffer data, where we saved the wow64 pointer */ struct @@ -1814,19 +1830,24 @@ NTSTATUS wow64_ext_glGetBufferPointerv( void *args ) PTR32 *wow_ptr = UlongToPtr(params32->params); NTSTATUS status; - if ((status = ext_glGetBufferPointerv( ¶ms ))) return status; + if ((status = get_buffer_pointer_v64( ¶ms ))) return status; if (params.pname != GL_BUFFER_MAP_POINTER) return STATUS_NOT_IMPLEMENTED; if (ULongToPtr(*wow_ptr = PtrToUlong(ptr)) == ptr) return STATUS_SUCCESS; /* we're lucky */ *wow_ptr = ptr[0]; return STATUS_SUCCESS; } +NTSTATUS wow64_ext_glGetBufferPointerv( void *args ) +{ + return wow64_gl_get_buffer_pointer_v( args, ext_glGetBufferPointerv ); +} + NTSTATUS wow64_ext_glGetBufferPointervARB( void *args ) { - return wow64_ext_glGetBufferPointerv( args ); + return wow64_gl_get_buffer_pointer_v( args, ext_glGetBufferPointervARB ); } -NTSTATUS wow64_ext_glGetNamedBufferPointerv( void *args ) +static NTSTATUS wow64_gl_get_named_buffer_pointer_v( void *args, NTSTATUS (*gl_get_named_buffer_pointer_v64)(void *) ) { PTR32 *ptr; /* pointer to the buffer data, where we saved the wow64 pointer */ struct @@ -1846,19 +1867,24 @@ NTSTATUS wow64_ext_glGetNamedBufferPointerv( void *args ) PTR32 *wow_ptr = UlongToPtr(params32->params); NTSTATUS status; - if ((status = ext_glGetNamedBufferPointerv( ¶ms ))) return status; + if ((status = gl_get_named_buffer_pointer_v64( ¶ms ))) return status; if (params.pname != GL_BUFFER_MAP_POINTER) return STATUS_NOT_IMPLEMENTED; if (ULongToPtr(*wow_ptr = PtrToUlong(ptr)) == ptr) return STATUS_SUCCESS; /* we're lucky */ *wow_ptr = ptr[0]; return STATUS_SUCCESS; } +NTSTATUS wow64_ext_glGetNamedBufferPointerv( void *args ) +{ + return wow64_gl_get_named_buffer_pointer_v( args, ext_glGetNamedBufferPointerv ); +} + NTSTATUS wow64_ext_glGetNamedBufferPointervEXT( void *args ) { - return wow64_ext_glGetNamedBufferPointerv( args ); + return wow64_gl_get_named_buffer_pointer_v( args, ext_glGetNamedBufferPointervEXT ); } -NTSTATUS wow64_ext_glMapBuffer( void *args ) +static NTSTATUS wow64_gl_map_buffer( void *args, NTSTATUS (*gl_map_buffer64)(void *) ) { struct { @@ -1873,23 +1899,27 @@ NTSTATUS wow64_ext_glMapBuffer( void *args ) .target = params32->target, .access = params32->access, }; - struct glUnmapBuffer_params unmap_params = { .teb = params.teb, .target = params.target }; NTSTATUS status; /* already mapped, we're being called again with a wow64 pointer */ if (params32->ret) params.ret = get_buffer_pointer( params.teb, params.target ); - else if ((status = ext_glMapBuffer( ¶ms ))) return status; + else if ((status = gl_map_buffer64( ¶ms ))) return status; status = wow64_map_buffer( params.teb, 0, params.target, params.ret, 0, params.access, ¶ms32->ret ); if (!status || status == STATUS_INVALID_ADDRESS) return status; - ext_glUnmapBuffer( &unmap_params ); + unmap_buffer( params.teb, params.target ); return status; } +NTSTATUS wow64_ext_glMapBuffer( void *args ) +{ + return wow64_gl_map_buffer( args, ext_glMapBuffer ); +} + NTSTATUS wow64_ext_glMapBufferARB( void *args ) { - return wow64_ext_glMapBuffer( args ); + return wow64_gl_map_buffer( args, ext_glMapBufferARB ); } NTSTATUS wow64_ext_glMapBufferRange( void *args ) @@ -1911,7 +1941,6 @@ NTSTATUS wow64_ext_glMapBufferRange( void *args ) .length = (GLsizeiptr)ULongToPtr(params32->length), .access = params32->access, }; - struct glUnmapBuffer_params unmap_params = { .teb = params.teb, .target = params.target }; NTSTATUS status; /* already mapped, we're being called again with a wow64 pointer */ @@ -1921,11 +1950,11 @@ NTSTATUS wow64_ext_glMapBufferRange( void *args ) status = wow64_map_buffer( params.teb, 0, params.target, params.ret, params.length, params.access, ¶ms32->ret ); if (!status || status == STATUS_INVALID_ADDRESS) return status; - ext_glUnmapBuffer( &unmap_params ); + unmap_buffer( params.teb, params.target ); return status; } -NTSTATUS wow64_ext_glMapNamedBuffer( void *args ) +static NTSTATUS wow64_gl_map_named_buffer( void *args, NTSTATUS (*gl_map_named_buffer64)(void *) ) { struct { @@ -1940,26 +1969,30 @@ NTSTATUS wow64_ext_glMapNamedBuffer( void *args ) .buffer = params32->buffer, .access = params32->access, }; - struct glUnmapNamedBuffer_params unmap_params = { .teb = params.teb, .buffer = params.buffer }; NTSTATUS status; /* already mapped, we're being called again with a wow64 pointer */ if (params32->ret) params.ret = get_named_buffer_pointer( params.teb, params.buffer ); - else if ((status = ext_glMapNamedBuffer( ¶ms ))) return status; + else if ((status = gl_map_named_buffer64( ¶ms ))) return status; status = wow64_map_buffer( params.teb, params.buffer, 0, params.ret, 0, params.access, ¶ms32->ret ); if (!status || status == STATUS_INVALID_ADDRESS) return status; - ext_glUnmapNamedBuffer( &unmap_params ); + unmap_named_buffer( params.teb, params.buffer ); return status; } +NTSTATUS wow64_ext_glMapNamedBuffer( void *args ) +{ + return wow64_gl_map_named_buffer( args, ext_glMapNamedBuffer ); +} + NTSTATUS wow64_ext_glMapNamedBufferEXT( void *args ) { - return wow64_ext_glMapNamedBuffer( args ); + return wow64_gl_map_named_buffer( args, ext_glMapNamedBufferEXT ); } -NTSTATUS wow64_ext_glMapNamedBufferRange( void *args ) +static NTSTATUS wow64_gl_map_named_buffer_range( void *args, NTSTATUS (*gl_map_named_buffer_range64)(void *) ) { struct { @@ -1978,26 +2011,30 @@ NTSTATUS wow64_ext_glMapNamedBufferRange( void *args ) .length = (GLsizeiptr)ULongToPtr(params32->length), .access = params32->access, }; - struct glUnmapNamedBuffer_params unmap_params = { .teb = params.teb, .buffer = params.buffer }; NTSTATUS status; /* already mapped, we're being called again with a wow64 pointer */ if (params32->ret) params.ret = get_named_buffer_pointer( params.teb, params.buffer ); - else if ((status = ext_glMapNamedBufferRange( ¶ms ))) return status; + else if ((status = gl_map_named_buffer_range64( ¶ms ))) return status; status = wow64_map_buffer( params.teb, params.buffer, 0, params.ret, params.length, params.access, ¶ms32->ret ); if (!status || status == STATUS_INVALID_ADDRESS) return status; - ext_glUnmapNamedBuffer( &unmap_params ); + unmap_named_buffer( params.teb, params.buffer ); return status; } +NTSTATUS wow64_ext_glMapNamedBufferRange( void *args ) +{ + return wow64_gl_map_named_buffer_range( args, ext_glMapNamedBufferRange ); +} + NTSTATUS wow64_ext_glMapNamedBufferRangeEXT( void *args ) { - return wow64_ext_glMapNamedBufferRange( args ); + return wow64_gl_map_named_buffer_range( args, ext_glMapNamedBufferRangeEXT ); } -NTSTATUS wow64_ext_glUnmapBuffer( void *args ) +static NTSTATUS wow64_gl_unmap_buffer( void *args, NTSTATUS (*gl_unmap_buffer64)(void *) ) { PTR32 *ptr; struct @@ -2013,17 +2050,22 @@ NTSTATUS wow64_ext_glUnmapBuffer( void *args ) status = wow64_unmap_buffer( ptr, get_buffer_param( teb, params32->target, GL_BUFFER_MAP_LENGTH ), get_buffer_param( teb, params32->target, GL_BUFFER_ACCESS_FLAGS ) ); - ext_glUnmapBuffer( args ); + gl_unmap_buffer64( args ); return status; } +NTSTATUS wow64_ext_glUnmapBuffer( void *args ) +{ + return wow64_gl_unmap_buffer( args, ext_glUnmapBuffer ); +} + NTSTATUS wow64_ext_glUnmapBufferARB( void *args ) { - return wow64_ext_glUnmapBuffer( args ); + return wow64_gl_unmap_buffer( args, ext_glUnmapBufferARB ); } -NTSTATUS wow64_ext_glUnmapNamedBuffer( void *args ) +static NTSTATUS wow64_gl_unmap_named_buffer( void *args, NTSTATUS (*gl_unmap_named_buffer64)(void *) ) { PTR32 *ptr; struct @@ -2039,14 +2081,19 @@ NTSTATUS wow64_ext_glUnmapNamedBuffer( void *args ) status = wow64_unmap_buffer( ptr, get_named_buffer_param( teb, params32->buffer, GL_BUFFER_MAP_LENGTH ), get_named_buffer_param( teb, params32->buffer, GL_BUFFER_ACCESS_FLAGS ) ); - ext_glUnmapNamedBuffer( args ); + gl_unmap_named_buffer64( args ); return status; } +NTSTATUS wow64_ext_glUnmapNamedBuffer( void *args ) +{ + return wow64_gl_unmap_named_buffer( args, ext_glUnmapNamedBuffer ); +} + NTSTATUS wow64_ext_glUnmapNamedBufferEXT( void *args ) { - return wow64_ext_glUnmapNamedBuffer( args ); + return wow64_gl_unmap_named_buffer( args, ext_glUnmapNamedBufferEXT ); } NTSTATUS wow64_thread_attach( void *args ) diff --git a/dlls/opengl32/wgl.c b/dlls/opengl32/wgl.c index f2ef3580f0f..f57f7703409 100644 --- a/dlls/opengl32/wgl.c +++ b/dlls/opengl32/wgl.c @@ -1058,7 +1058,7 @@ static void *get_named_buffer_pointer( GLint buffer ) } #endif -void * WINAPI glMapBuffer( GLenum target, GLenum access ) +static void *gl_map_buffer( enum unix_funcs code, GLenum target, GLenum access ) { struct glMapBuffer_params args = { @@ -1070,13 +1070,13 @@ void * WINAPI glMapBuffer( GLenum target, GLenum access ) TRACE( "target %d, access %d\n", target, access ); - if (!(status = UNIX_CALL( glMapBuffer, &args ))) return args.ret; + if (!(status = WINE_UNIX_CALL( code, &args ))) return args.ret; #ifndef _WIN64 if (status == STATUS_INVALID_ADDRESS) { TRACE( "Unable to map wow64 buffer directly, using copy buffer!\n" ); if (!(args.ret = _aligned_malloc( (size_t)args.ret, 16 ))) status = STATUS_NO_MEMORY; - else if (!(status = UNIX_CALL( glMapBuffer, &args ))) return args.ret; + else if (!(status = WINE_UNIX_CALL( code, &args ))) return args.ret; _aligned_free( args.ret ); } #endif @@ -1084,9 +1084,14 @@ void * WINAPI glMapBuffer( GLenum target, GLenum access ) return args.ret; } +void * WINAPI glMapBuffer( GLenum target, GLenum access ) +{ + return gl_map_buffer( unix_glMapBuffer, target, access ); +} + void * WINAPI glMapBufferARB( GLenum target, GLenum access ) { - return glMapBuffer( target, access ); + return gl_map_buffer( unix_glMapBufferARB, target, access ); } void * WINAPI glMapBufferRange( GLenum target, GLintptr offset, GLsizeiptr length, GLbitfield access ) @@ -1117,7 +1122,7 @@ void * WINAPI glMapBufferRange( GLenum target, GLintptr offset, GLsizeiptr lengt return args.ret; } -void * WINAPI glMapNamedBuffer( GLuint buffer, GLenum access ) +static void *gl_map_named_buffer( enum unix_funcs code, GLuint buffer, GLenum access ) { struct glMapNamedBuffer_params args = { @@ -1129,13 +1134,13 @@ void * WINAPI glMapNamedBuffer( GLuint buffer, GLenum access ) TRACE( "(%d, %d)\n", buffer, access ); - if (!(status = UNIX_CALL( glMapNamedBuffer, &args ))) return args.ret; + if (!(status = WINE_UNIX_CALL( code, &args ))) return args.ret; #ifndef _WIN64 if (status == STATUS_INVALID_ADDRESS) { TRACE( "Unable to map wow64 buffer directly, using copy buffer!\n" ); if (!(args.ret = _aligned_malloc( (size_t)args.ret, 16 ))) status = STATUS_NO_MEMORY; - else if (!(status = UNIX_CALL( glMapNamedBuffer, &args ))) return args.ret; + else if (!(status = WINE_UNIX_CALL( code, &args ))) return args.ret; _aligned_free( args.ret ); } #endif @@ -1143,12 +1148,17 @@ void * WINAPI glMapNamedBuffer( GLuint buffer, GLenum access ) return args.ret; } +void * WINAPI glMapNamedBuffer( GLuint buffer, GLenum access ) +{ + return gl_map_named_buffer( unix_glMapNamedBuffer, buffer, access ); +} + void * WINAPI glMapNamedBufferEXT( GLuint buffer, GLenum access ) { - return glMapNamedBuffer( buffer, access ); + return gl_map_named_buffer( unix_glMapNamedBufferEXT, buffer, access ); } -void * WINAPI glMapNamedBufferRange( GLuint buffer, GLintptr offset, GLsizeiptr length, GLbitfield access ) +static void *gl_map_named_buffer_range( enum unix_funcs code, GLuint buffer, GLintptr offset, GLsizeiptr length, GLbitfield access ) { struct glMapNamedBufferRange_params args = { @@ -1162,13 +1172,13 @@ void * WINAPI glMapNamedBufferRange( GLuint buffer, GLintptr offset, GLsizeiptr TRACE( "buffer %d, offset %Id, length %Id, access %d\n", buffer, offset, length, access ); - if (!(status = UNIX_CALL( glMapNamedBufferRange, &args ))) return args.ret; + if (!(status = WINE_UNIX_CALL( code, &args ))) return args.ret; #ifndef _WIN64 if (status == STATUS_INVALID_ADDRESS) { TRACE( "Unable to map wow64 buffer directly, using copy buffer!\n" ); if (!(args.ret = _aligned_malloc( length, 16 ))) status = STATUS_NO_MEMORY; - else if (!(status = UNIX_CALL( glMapNamedBufferRange, &args ))) return args.ret; + else if (!(status = WINE_UNIX_CALL( code, &args ))) return args.ret; _aligned_free( args.ret ); } #endif @@ -1176,12 +1186,17 @@ void * WINAPI glMapNamedBufferRange( GLuint buffer, GLintptr offset, GLsizeiptr return args.ret; } +void * WINAPI glMapNamedBufferRange( GLuint buffer, GLintptr offset, GLsizeiptr length, GLbitfield access ) +{ + return gl_map_named_buffer_range( unix_glMapNamedBufferRange, buffer, offset, length, access ); +} + void * WINAPI glMapNamedBufferRangeEXT( GLuint buffer, GLintptr offset, GLsizeiptr length, GLbitfield access ) { - return glMapNamedBufferRange( buffer, offset, length, access ); + return gl_map_named_buffer_range( unix_glMapNamedBufferRangeEXT, buffer, offset, length, access ); } -GLboolean WINAPI glUnmapBuffer( GLenum target ) +static GLboolean gl_unmap_buffer( enum unix_funcs code, GLenum target ) { struct glUnmapBuffer_params args = { @@ -1195,7 +1210,7 @@ GLboolean WINAPI glUnmapBuffer( GLenum target ) TRACE( "target %d\n", target ); - if (!(status = UNIX_CALL( glUnmapBuffer, &args ))) return args.ret; + if (!(status = WINE_UNIX_CALL( code, &args ))) return args.ret; #ifndef _WIN64 if (status == STATUS_INVALID_ADDRESS) { @@ -1208,12 +1223,17 @@ GLboolean WINAPI glUnmapBuffer( GLenum target ) return args.ret; } +GLboolean WINAPI glUnmapBuffer( GLenum target ) +{ + return gl_unmap_buffer( unix_glUnmapBuffer, target ); +} + GLboolean WINAPI glUnmapBufferARB( GLenum target ) { - return glUnmapBuffer( target ); + return gl_unmap_buffer( unix_glUnmapBufferARB, target ); } -GLboolean WINAPI glUnmapNamedBuffer( GLuint buffer ) +static GLboolean gl_unmap_named_buffer( enum unix_funcs code, GLuint buffer ) { struct glUnmapNamedBuffer_params args = { @@ -1227,7 +1247,7 @@ GLboolean WINAPI glUnmapNamedBuffer( GLuint buffer ) TRACE( "buffer %d\n", buffer ); - if (!(status = UNIX_CALL( glUnmapNamedBuffer, &args ))) return args.ret; + if (!(status = WINE_UNIX_CALL( code, &args ))) return args.ret; #ifndef _WIN64 if (status == STATUS_INVALID_ADDRESS) { @@ -1240,9 +1260,14 @@ GLboolean WINAPI glUnmapNamedBuffer( GLuint buffer ) return args.ret; } +GLboolean WINAPI glUnmapNamedBuffer( GLuint buffer ) +{ + return gl_unmap_named_buffer( unix_glUnmapNamedBuffer, buffer ); +} + GLboolean WINAPI glUnmapNamedBufferEXT( GLuint buffer ) { - return glUnmapNamedBuffer( buffer ); + return gl_unmap_named_buffer( unix_glUnmapNamedBufferEXT, buffer ); } static BOOL WINAPI call_opengl_debug_message_callback( struct wine_gl_debug_message_params *params, ULONG size ) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1760
v2: Remove unnecessary `void *` return from `unmap_buffer` / `unmap_named_buffer` helpers. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1760#note_19317
participants (1)
-
Rémi Bernon