Skip to content
Snippets Groups Projects
Commit ad6a036a authored by Paulo Zanoni's avatar Paulo Zanoni Committed by Marge Bot
Browse files

anv: don't leave undefined values in exec->syncobj_values


In anv_execbuf_add_syncobj(), we try to not create or use
exec->syncobj_values if we don't need to. But when we figure we're
going to need it (i.e., when timeline_value is not zero), then we
create exec->syncobj_values with vk_zalloc, which means every previous
value is set to zero, as it should be. This is all correct.

The problem starts when we add a 16th element. In this case we double
exec->syncobj_array_length and realloc the buffer by using vk_alloc
and copying the old array to the new one. After that, we write the
timeline_value to the array only if it's not zero, and that's the
problem: since we just used vkalloc and memcpy, we don't have any
guarantees that the new array will be zero after the 16th element, and
if timeline_value is zero we write nothing to that position.

Once we start using exec->syncobj_values we have to commit to using
it, so the "if (timeline_value)" check near the end of the function
has to be changed to "if (exec->syncobj_values)", so we actually set
elements after the 16th to zero when they need to be zero.  Another
approach to fix this would be to memset the new elements once we
double syncobj_array_length.

In practice, I couldn't find any application or deqp test that used
more than 3 elements in exec->syncobj_array_length, and we need more
than 16 elements in order to be able to reproduce the bug, so I'm not
aware of any real-world bug that goes away with this patch. This issue
was found while reading code.

If we craft a little Vulkan program that submits a ton of timeline and
binary semaphores on vkQueueSubmit, then waits for them, we get the
following error without this patch:

MESA: error: ../../src/intel/vulkan/anv_batch_chain.c:1910: execbuf2 failed: Invalid argument (VK_ERROR_DEVICE_LOST)

v2: Rebase.

Cc: mesa-stable
Reviewed-by: default avatarIvan Briano <ivan.briano@intel.com>
Reviewed-by: default avatarLionel Landwerlin <lionel.g.landwerlin@intel.com>
Signed-off-by: default avatarPaulo Zanoni <paulo.r.zanoni@intel.com>
Part-of: <mesa/mesa!20703>
parent fd3e8047
No related branches found
No related tags found
No related merge requests found
......@@ -241,7 +241,7 @@ anv_execbuf_add_syncobj(struct anv_device *device,
.handle = syncobj,
.flags = flags,
};
if (timeline_value)
if (exec->syncobj_values)
exec->syncobj_values[exec->syncobj_count] = timeline_value;
exec->syncobj_count++;
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment