From 6f64ec0f51ee1f854a086d0cead51dcdc51bed95 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 11 Aug 2026 09:12:49 -0400 Subject: [PATCH] [Fix, Test] (MG_Impl): validate indirect-dispatch and indirect-count arguments before the backend-availability check --- .../MG_Impl/GLImpl/Drawing/GL_Drawing.cpp | 46 +++++++++++-------- .../MG_Test/State/NegativeApiErrorsTest.cpp | 12 +++-- 2 files changed, 35 insertions(+), 23 deletions(-) diff --git a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp index 347fa9ac..369339f4 100644 --- a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp +++ b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp @@ -493,15 +493,12 @@ namespace MobileGL::MG_Impl::GLImpl { } void DispatchComputeIndirect(GLintptr indirect) { - auto dispatchComputeIndirect = MG_Backend::gBackendFunctionsTable.GL.DispatchComputeIndirect; - if (!dispatchComputeIndirect) { - MG_State::pGLContext->RecordError( - ErrorCode::InvalidOperation, - MakeUnique("MG_Impl/GLImpl", __func__, - "Backend does not support indirect compute dispatch.")); - return; - } - if (!ValidateCurrentProgramForCompute(__func__)) return; + // Argument and binding validation runs FIRST. Both are properties of the call and of GL + // state, so a context whose backend cannot dispatch at all must still report the + // argument error the spec names rather than masking every one of them with + // "unsupported" - which is what put GL_INVALID_OPERATION where + // KHR-GL43.compute_shader.api-indirect expects GL_INVALID_VALUE. + // // GL 4.6 core 19: `indirect` is a byte offset into GL_DISPATCH_INDIRECT_BUFFER - // negative or misaligned is INVALID_VALUE, nothing bound is INVALID_OPERATION. if (indirect < 0 || (indirect % 4) != 0) { @@ -534,6 +531,15 @@ namespace MobileGL::MG_Impl::GLImpl { indirect, indirectBuffer->GetSize()))); return; } + auto dispatchComputeIndirect = MG_Backend::gBackendFunctionsTable.GL.DispatchComputeIndirect; + if (!dispatchComputeIndirect) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidOperation, + MakeUnique("MG_Impl/GLImpl", __func__, + "Backend does not support indirect compute dispatch.")); + return; + } + if (!ValidateCurrentProgramForCompute(__func__)) return; dispatchComputeIndirect(indirect); } @@ -651,6 +657,12 @@ namespace MobileGL::MG_Impl::GLImpl { void MultiDrawElementsIndirectCount(GLenum mode, GLenum type, const void* indirect, GLintptr drawcount, GLsizei maxdrawcount, GLsizei stride) { + // Argument validation before the backend-availability check: see DispatchComputeIndirect. + // DrawElementsIndirectCommand: count, instanceCount, firstIndex, baseVertex, baseInstance. + if (!ValidateIndirectCountDraw(reinterpret_cast(indirect), drawcount, maxdrawcount, stride, + 5 * sizeof(Uint32), __func__)) { + return; + } auto multiDrawElementsIndirectCount = MG_Backend::gBackendFunctionsTable.GL.MultiDrawElementsIndirectCount; if (!multiDrawElementsIndirectCount) { MG_State::pGLContext->RecordError( @@ -659,16 +671,17 @@ namespace MobileGL::MG_Impl::GLImpl { "Backend does not support indirect-parameter indexed draws.")); return; } - // DrawElementsIndirectCommand: count, instanceCount, firstIndex, baseVertex, baseInstance. - if (!ValidateIndirectCountDraw(reinterpret_cast(indirect), drawcount, maxdrawcount, stride, - 5 * sizeof(Uint32), __func__)) { - return; - } MultiDrawElementsIndirectCount_Backend(mode, type, indirect, drawcount, maxdrawcount, stride); } void MultiDrawArraysIndirectCount(GLenum mode, const void* indirect, GLintptr drawcount, GLsizei maxdrawcount, GLsizei stride) { + // Argument validation before the backend-availability check: see DispatchComputeIndirect. + // DrawArraysIndirectCommand: count, instanceCount, first, baseInstance. + if (!ValidateIndirectCountDraw(reinterpret_cast(indirect), drawcount, maxdrawcount, stride, + 4 * sizeof(Uint32), __func__)) { + return; + } auto multiDrawArraysIndirectCount = MG_Backend::gBackendFunctionsTable.GL.MultiDrawArraysIndirectCount; if (!multiDrawArraysIndirectCount) { MG_State::pGLContext->RecordError( @@ -677,11 +690,6 @@ namespace MobileGL::MG_Impl::GLImpl { "Backend does not support indirect-parameter array draws.")); return; } - // DrawArraysIndirectCommand: count, instanceCount, first, baseInstance. - if (!ValidateIndirectCountDraw(reinterpret_cast(indirect), drawcount, maxdrawcount, stride, - 4 * sizeof(Uint32), __func__)) { - return; - } MultiDrawArraysIndirectCount_Backend(mode, indirect, drawcount, maxdrawcount, stride); } diff --git a/MobileGL/MG_Test/State/NegativeApiErrorsTest.cpp b/MobileGL/MG_Test/State/NegativeApiErrorsTest.cpp index 267808e7..732c6482 100644 --- a/MobileGL/MG_Test/State/NegativeApiErrorsTest.cpp +++ b/MobileGL/MG_Test/State/NegativeApiErrorsTest.cpp @@ -95,12 +95,16 @@ namespace { ASSERT_NE(reservedOnly, 0u); ASSERT_EQ(IsBuffer(reservedOnly), GL_FALSE); - GLuint samplerReservedOnly = 0; - GenSamplers(1, &samplerReservedOnly); + // glGenSamplers, unlike glGenBuffers, creates the objects outright, so a sampler name is + // only "not an existing object" once it has been deleted. + GLuint deadSampler = 0; + GenSamplers(1, &deadSampler); + ASSERT_NE(deadSampler, 0u); + DeleteSamplers(1, &deadSampler); DrainErrors(); const GLuint mixedBuffers[2] = {buffer, reservedOnly}; - const GLuint samplers[1] = {samplerReservedOnly}; + const GLuint samplers[1] = {deadSampler}; const GLintptr offsets[2] = {0, 0}; const GLsizeiptr sizes[2] = {256, 256}; @@ -110,7 +114,7 @@ namespace { {"glBindBuffersRange with a reserved-but-uncreated name", [&] { BindBuffersRange(GL_UNIFORM_BUFFER, 0, 2, mixedBuffers, offsets, sizes); }, GL_INVALID_OPERATION}, - {"glBindSamplers with a reserved-but-uncreated name", [&] { BindSamplers(0, 1, samplers); }, + {"glBindSamplers with a deleted sampler name", [&] { BindSamplers(0, 1, samplers); }, GL_INVALID_OPERATION}, });