[MG_Backend, MG_IntegrationTest] (P7): an unaligned counter range declines its binding instead of killing the session

`unaligned-atomic-buffer-range@P7`, `unaligned-storage-buffer-range@P7` and
`unaligned-texel-buffer-range@P7` become named declines (rule I (a)). The two
offset-overflow guards next to them keep their named Fatal under their own words:
those are arithmetic that cannot happen, not shapes an application can bind.

The atomic-counter one is the only member of this cluster an application can
actually reach, and it is a real live gap, not a hypothetical. GL 4.6 core 6.1.1
gives GL_ATOMIC_COUNTER_BUFFER no queryable offset alignment, so
ValidateBufferRangeOffsetAndSize only enforces offset % 4 on it - while glslang
lowers every atomic_uint onto a synthesized storage block whose descriptor offset
must be a multiple of minStorageBufferOffsetAlignment (16 on lavapipe, 64 on
Adreno). `glBindBufferRange(GL_ATOMIC_COUNTER_BUFFER, 0, buf, 4, 4)` is a legal
GL bind that this backend cannot express, and it used to take the session with
it. The plain SSBO and texel halves are unreachable by comparison:
GL_SHADER_STORAGE_BUFFER_OFFSET_ALIGNMENT and GL_TEXTURE_BUFFER_OFFSET_ALIGNMENT
are published from the same Vulkan limits, so the frontend already answers
GL_INVALID_VALUE.

DECLINED RATHER THAN COPIED, and this is the package's one real design choice.
Copying the window into an aligned transient slice reads correctly and then
silently DROPS every shader write. This site cannot tell a writable storage block
from a read-only one - there is no readonly bit in the storage-block reflection,
an atomic counter is written by definition, and MarkWireBufferGpuWritten right
below already assumes any binding here may be written. A correct copy needs a
post-dispatch copy-back from the transient slice to the unaligned source, ordered
with a barrier, and the place to record that is the draw/dispatch tail in
VulkanRenderer.cpp / WireDraw.inc - wave-2 packages B and C, not this file.
Returning stale counters with no error is worse than losing the dispatch with a
named line in the role log. notes/p7/magma-a.md carries the recipe for whoever
holds those files.

The texel half keeps the draw where it safely can: a read-only buffer texture
falls back to the same placeholder view an unbound one gets (zeros, which is what
a missing view gives in monolith), while a writable imageBuffer declines - the
placeholder is shared by every unbound binding, so absorbing real stores into it
would alias other bindings as well as lose the writes. Copying is not available
there either: the transient arena carries no {UNIFORM,STORAGE}_TEXEL_BUFFER
usage, so no VkBufferView can be made over it, and widening a shared per-frame
allocation to buy an unreachable path is the wrong trade.

red-once (R-16), a reachable case this time, all three Magma two-process arms:

    DirectVulkan.{Split,Spawn,Tcp}.Buffers.F1WireScenario.UnalignedAtomicCounterRangeDeclinesAndTheSessionLives
      before  Subprocess aborted - Fatal{UnmigratedVerb, "Magma:unaligned-atomic-buffer-range@P7"}
      after   3/3 Passed, and the role log carries
              "block 'gl_AtomicCounterBlock_0' is bound at offset 4, which is not a
               multiple of this device's minStorageBufferOffsetAlignment (16);
               atomic-counter ranges cannot be expressed as a descriptor ... DECLINED
               (unaligned-atomic-buffer-range)"

The case pins the decline's whole observable: the bind raises no GL error, the
dispatch does not run, every byte of the counter buffer is unchanged, and the
context still serves ordinary work afterwards. A future copy-back turns one
EXPECT into seed[1] + 1 and leaves the rest standing.

Lanes: integration-magma-split 75/75, -spawn 54/54, -tcp 56/56,
integration-magma-buffers 21/21.
This commit is contained in:
2026-09-22 16:22:40 -04:00
parent de049f9ea0
commit ae57d01168
3 changed files with 123 additions and 7 deletions
@@ -1160,9 +1160,32 @@ namespace MobileGL::MG_Backend::DirectVulkan {
if (texelSize == 0 || !ResolveWireRange(record.Desc.BufOffset, record.Desc.BufSize, slice.size, start, size)) return false;
size = std::min(size / texelSize, static_cast<VkDeviceSize>(m_wireMaxTexelElements)) * texelSize;
if (size == 0) return false;
if (start > std::numeric_limits<VkDeviceSize>::max() - slice.offset ||
(slice.offset + start) % m_wireTexelOffsetAlignment != 0)
WireDescriptorFatal("unaligned-texel-buffer-range@P7");
if (start > std::numeric_limits<VkDeviceSize>::max() - slice.offset)
WireDescriptorFatal("texel-buffer-offset-overflow");
if ((slice.offset + start) % m_wireTexelOffsetAlignment != 0) {
// P7 A.4, a named decline (rule I (a)). Not reachable through the public API:
// glTexBufferRange already refuses an offset that is not a multiple of
// GL_TEXTURE_BUFFER_OFFSET_ALIGNMENT (GL_Texture.cpp), and that cap is published
// from this same minTexelBufferOffsetAlignment. It stays a decline rather than a
// copy for the reason below.
const Bool writable = storage && state.BoundShaderImages[unit].Access != kMGPipeImageAccessReadOnly;
MGLOG_E_ONCE("ResolveWireTexelBufferDescriptor: binding %u is bound at offset %llu, which is not a "
"multiple of this device's minTexelBufferOffsetAlignment (%llu); the %s view is DECLINED "
"(unaligned-texel-buffer-range)", binding,
static_cast<unsigned long long>(slice.offset + start),
static_cast<unsigned long long>(m_wireTexelOffsetAlignment),
writable ? "writable" : "read-only");
// A read-only fetch can still be served the way an unbound buffer texture is -
// zeros, which is what a missing view gives in monolith - and that keeps the draw.
// A writable imageBuffer must not: the placeholder is shared by every unbound
// binding, so absorbing real stores into it would alias other bindings AND lose
// the application's writes without a word. Copying into an aligned transient slice
// is not the answer either: the transient arena carries no
// {UNIFORM,STORAGE}_TEXEL_BUFFER usage (VkBufferManager::InitializeTransientArenas),
// so no VkBufferView can be made over it, and widening a shared per-frame
// allocation to buy an unreachable path is the wrong trade. See notes/p7/magma-a.md.
return writable ? false : placeholder();
}
VkBufferViewCreateInfo viewInfo{VK_STRUCTURE_TYPE_BUFFER_VIEW_CREATE_INFO};
viewInfo.buffer = slice.buffer;
viewInfo.format = format;
@@ -1550,10 +1573,43 @@ namespace MobileGL::MG_Backend::DirectVulkan {
if (!m_bufferManager->AcquireWireSlice(BufferKind::ShaderStorage, range.Res, slice) || !slice.IsValid()) return false;
VkDeviceSize start = 0, size = 0;
if (!ResolveWireRange(range.Offset, range.Size, slice.size, start, size)) return false;
if (start > std::numeric_limits<VkDeviceSize>::max() - slice.offset ||
(slice.offset + start) % m_wireStorageOffsetAlignment != 0)
WireDescriptorFatal(isAtomicCounterBlock ? "unaligned-atomic-buffer-range@P7"
: "unaligned-storage-buffer-range@P7");
if (start > std::numeric_limits<VkDeviceSize>::max() - slice.offset)
WireDescriptorFatal("storage-buffer-offset-overflow");
if ((slice.offset + start) % m_wireStorageOffsetAlignment != 0) {
// P7 A.4, a named decline (rule I (a)), and the one shape in this cluster an
// application can actually reach. GL has no queryable atomic-counter buffer
// offset alignment, so glBindBufferRange only enforces offset % 4 for
// GL_ATOMIC_COUNTER_BUFFER (GL 4.6 core 6.1.1, GL_Buffer.cpp
// ValidateBufferRangeOffsetAndSize); glslang lowers the counter block onto a
// storage buffer, whose descriptor offset must be a multiple of
// minStorageBufferOffsetAlignment - 16 on lavapipe, 64 on Adreno. Offset 4 is
// therefore a legal bind this backend cannot express. The plain SSBO half is
// unreachable by comparison: GL_SHADER_STORAGE_BUFFER_OFFSET_ALIGNMENT is
// published from that same Vulkan limit, so the frontend already refused it
// with GL_INVALID_VALUE.
//
// DECLINED RATHER THAN COPIED, deliberately. Copying the window into an aligned
// transient slice reads correctly and then silently DROPS every shader write,
// and this site cannot tell a writable block from a read-only one: there is no
// readonly bit in the storage-block reflection (ProgramFactory.h carries only
// name and index), an atomic counter is written by definition, and
// MarkWireBufferGpuWritten below already assumes any binding here may be
// written. A correct copy needs a post-dispatch copy-back from the transient
// slice to the unaligned source, ordered with a barrier - and the place to
// record that is the draw/dispatch tail in VulkanRenderer.cpp / WireDraw.inc,
// which belong to wave-2 packages B and C, not to this file. Losing the
// dispatch with a named line in the role log beats returning stale counters
// with no error at all. notes/p7/magma-a.md carries the recipe.
MGLOG_E_ONCE("ResolveStorageBufferDescriptor: block '%s' is bound at offset %llu, which is not a "
"multiple of this device's minStorageBufferOffsetAlignment (%llu); %s ranges cannot be "
"expressed as a descriptor and the shader's writes would be dropped by a read-only "
"copy, so the binding is DECLINED (unaligned-%s-buffer-range)",
blockName.c_str(), static_cast<unsigned long long>(slice.offset + start),
static_cast<unsigned long long>(m_wireStorageOffsetAlignment),
isAtomicCounterBlock ? "atomic-counter" : "shader-storage",
isAtomicCounterBlock ? "atomic" : "storage");
return false;
}
if (size > m_wireMaxStorageRange) {
// P7 A.3: clamp, do not refuse. The monolith arm binds whatever the GL range
// asked for and never consults maxStorageBufferRange at all (see the resident
@@ -2706,6 +2706,7 @@ if (MOBILEGL_BUILD_DISAGGREGATED)
F1WireScenario.UniformBufferRangeAndRebindPixels
F1WireScenario.ShortUniformBufferRangePadsMissingBytes
F1WireScenario.SubWordUniformBufferRangePadsMissingBytes
F1WireScenario.UnalignedAtomicCounterRangeDeclinesAndTheSessionLives
F1WireScenario.ComputeWrittenVertexAndIndexBuffersDrawAndReadBack
F1WireScenario.ColorBlitToDefaultIgnoresViewportAndPreservesOrientation
F1WireScenario.ColorBlitToDefaultHonorsScissorAndReversedRect
@@ -1055,6 +1055,65 @@ void main() { color = vec4(0, 1, 0, 1); }
glDeleteProgram(graphics);
}
TEST_F(F1WireScenario, UnalignedAtomicCounterRangeDeclinesAndTheSessionLives) {
if (!Ready()) return;
// The one shape in the P7 unaligned-range cluster an application can actually reach.
// GL 4.6 core 6.1.1 gives GL_ATOMIC_COUNTER_BUFFER no queryable offset alignment, so
// glBindBufferRange only enforces offset % 4 on it, while glslang lowers the counter
// block onto a storage buffer whose descriptor offset must be a multiple of
// GL_SHADER_STORAGE_BUFFER_OFFSET_ALIGNMENT (minStorageBufferOffsetAlignment: 16 on
// lavapipe, 64 on Adreno). Offset 4 is therefore a legal bind the backend cannot
// express. It used to take the session with it as a named Fatal; it is now a decline,
// and what this case pins is exactly that - the dispatch is lost, nothing else is.
if (Gl().BackendName() != "DirectVulkan") GTEST_SKIP() << "Magma descriptor-alignment decline";
GLint storageAlignment = 0;
glGetIntegerv(GL_SHADER_STORAGE_BUFFER_OFFSET_ALIGNMENT, &storageAlignment);
if (storageAlignment <= 4) GTEST_SKIP() << "this device can express a 4-byte-aligned counter range";
const char* compute = R"(#version 430 core
layout(local_size_x=1) in;
layout(binding=0, offset=0) uniform atomic_uint counter;
void main() { atomicCounterIncrement(counter); }
)";
const GLuint cs = BuildWireProgram({{GL_COMPUTE_SHADER, compute}});
ASSERT_NE(cs, 0u);
GLuint counters = 0;
glGenBuffers(1, &counters);
const std::array<GLuint, 4> seed{0x11111111u, 0x22222222u, 0x33333333u, 0x44444444u};
glBindBuffer(GL_ATOMIC_COUNTER_BUFFER, counters);
glBufferData(GL_ATOMIC_COUNTER_BUFFER, GLsizeiptr(seed.size() * sizeof(GLuint)), seed.data(), GL_DYNAMIC_COPY);
// Offset 4: a multiple of 4 and so a legal GL bind, but not a multiple of the storage
// alignment this device reports. The bind itself must NOT raise a GL error.
glBindBufferRange(GL_ATOMIC_COUNTER_BUFFER, 0, counters, GLintptr(4), GLsizeiptr(sizeof(GLuint)));
ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)) << "offset 4 is a legal GL_ATOMIC_COUNTER_BUFFER range";
glUseProgram(cs);
glDispatchCompute(1, 1, 1);
glMemoryBarrier(GL_ATOMIC_COUNTER_BARRIER_BIT | GL_BUFFER_UPDATE_BARRIER_BIT);
// The session is still here - this is the whole point - and it still answers GL.
std::array<GLuint, 4> after{};
glGetBufferSubData(GL_ATOMIC_COUNTER_BUFFER, 0, GLsizeiptr(after.size() * sizeof(GLuint)), after.data());
ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)) << "the declined dispatch must not poison the context";
// The dispatch was declined, so the counter is untouched; the bytes outside the bound
// range must be untouched either way. Both halves of that are worth pinning: a future
// copy-back implementation turns the first EXPECT into seed[1] + 1 and nothing else.
EXPECT_EQ(after[1], seed[1]) << "a declined counter dispatch must not have run";
EXPECT_EQ(after[0], seed[0]);
EXPECT_EQ(after[2], seed[2]);
EXPECT_EQ(after[3], seed[3]);
// And the context is still usable for ordinary work afterwards.
GLuint probe = 0;
glGenBuffers(1, &probe);
glBindBuffer(GL_ARRAY_BUFFER, probe);
glBufferData(GL_ARRAY_BUFFER, 16, seed.data(), GL_STATIC_DRAW);
EXPECT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)) << "the session survived the decline";
glBindBuffer(GL_ARRAY_BUFFER, 0);
glDeleteBuffers(1, &probe);
glBindBufferBase(GL_ATOMIC_COUNTER_BUFFER, 0, 0);
glBindBuffer(GL_ATOMIC_COUNTER_BUFFER, 0);
glUseProgram(0);
glDeleteBuffers(1, &counters);
glDeleteProgram(cs);
}
TEST_F(F1WireScenario, SrgbDrawTracksFramebufferConversion) {
if (!Ready()) return;
Attach(GL_SRGB8_ALPHA8);