From 509c8a60c47f203320c102007ba2963f0da4ff73 Mon Sep 17 00:00:00 2001 From: Iago Toral Quiroga Date: Tue, 2 Jun 2020 13:13:43 +0200 Subject: [PATCH] v3dv: don't leak attachment state We were assuming that if the command buffer state doesn't have any attachments (as per the attachment count) the attachment state array should not be allocated, however, during meta operations it is possible that the attachment state grows (since meta operations can emit render passes of their own). In that case, we would grow the state for the meta operation but then pop the previous attachment count and we would leak the state. An example of that is a secondary command buffer which has no attachment state by default since it doesn't execute a render pass begin, but that executes one in a meta operation (for vkCmdClearAttachments for example). Fix this by making the attachment count an allocation count instead and not popping it once we finish a meta operation. Also, always free the state so long as there is a valid pointer, and assert that the allocated count is not zero in that case. Part-of: --- src/broadcom/vulkan/v3dv_cmd_buffer.c | 30 +++++++++++++++------------ src/broadcom/vulkan/v3dv_private.h | 2 +- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/broadcom/vulkan/v3dv_cmd_buffer.c b/src/broadcom/vulkan/v3dv_cmd_buffer.c index 4da56856df8..e56fb9edae5 100644 --- a/src/broadcom/vulkan/v3dv_cmd_buffer.c +++ b/src/broadcom/vulkan/v3dv_cmd_buffer.c @@ -251,10 +251,8 @@ cmd_buffer_free_resources(struct v3dv_cmd_buffer *cmd_buffer) if (cmd_buffer->state.job) v3dv_job_destroy(cmd_buffer->state.job); - if (cmd_buffer->state.attachments) { - assert(cmd_buffer->state.attachment_count > 0); + if (cmd_buffer->state.attachments) vk_free(&cmd_buffer->pool->alloc, cmd_buffer->state.attachments); - } if (cmd_buffer->state.query.end.alloc_count > 0) vk_free(&cmd_buffer->device->alloc, cmd_buffer->state.query.end.states); @@ -272,6 +270,11 @@ cmd_buffer_free_resources(struct v3dv_cmd_buffer *cmd_buffer) cmd_buffer->meta.blit.dspool, &cmd_buffer->device->alloc); } + + if (cmd_buffer->state.meta.attachments) { + assert(cmd_buffer->state.meta.attachment_alloc_count > 0); + vk_free(&cmd_buffer->device->alloc, cmd_buffer->state.meta.attachments); + } } static void @@ -1154,9 +1157,11 @@ cmd_buffer_ensure_render_pass_attachment_state(struct v3dv_cmd_buffer *cmd_buffe struct v3dv_cmd_buffer_state *state = &cmd_buffer->state; const struct v3dv_render_pass *pass = state->pass; - if (state->attachment_count < pass->attachment_count) { - if (state->attachment_count > 0) + if (state->attachment_alloc_count < pass->attachment_count) { + if (state->attachments > 0) { + assert(state->attachment_alloc_count > 0); vk_free(&cmd_buffer->device->alloc, state->attachments); + } uint32_t size = sizeof(struct v3dv_cmd_buffer_attachment_state) * pass->attachment_count; @@ -1166,10 +1171,10 @@ cmd_buffer_ensure_render_pass_attachment_state(struct v3dv_cmd_buffer *cmd_buffe v3dv_flag_oom(cmd_buffer, NULL); return; } - state->attachment_count = pass->attachment_count; + state->attachment_alloc_count = pass->attachment_count; } - assert(state->attachment_count >= pass->attachment_count); + assert(state->attachment_alloc_count >= pass->attachment_count); } void @@ -3377,8 +3382,8 @@ v3dv_cmd_buffer_meta_state_push(struct v3dv_cmd_buffer *cmd_buffer, const uint32_t attachment_state_item_size = sizeof(struct v3dv_cmd_buffer_attachment_state); const uint32_t attachment_state_total_size = - attachment_state_item_size * state->attachment_count; - if (state->meta.attachment_alloc_count < state->attachment_count) { + attachment_state_item_size * state->attachment_alloc_count; + if (state->meta.attachment_alloc_count < state->attachment_alloc_count) { if (state->meta.attachment_alloc_count > 0) vk_free(&cmd_buffer->device->alloc, state->meta.attachments); @@ -3389,9 +3394,9 @@ v3dv_cmd_buffer_meta_state_push(struct v3dv_cmd_buffer *cmd_buffer, v3dv_flag_oom(cmd_buffer, NULL); return; } - state->meta.attachment_alloc_count = state->attachment_count; + state->meta.attachment_alloc_count = state->attachment_alloc_count; } - state->meta.attachment_count = state->attachment_count; + state->meta.attachment_count = state->attachment_alloc_count; memcpy(state->meta.attachments, state->attachments, attachment_state_total_size); @@ -3428,12 +3433,11 @@ v3dv_cmd_buffer_meta_state_pop(struct v3dv_cmd_buffer *cmd_buffer, state->pass = v3dv_render_pass_from_handle(state->meta.pass); state->framebuffer = v3dv_framebuffer_from_handle(state->meta.framebuffer); - assert(state->meta.attachment_count <= state->attachment_count); + assert(state->meta.attachment_count <= state->attachment_alloc_count); const uint32_t attachment_state_item_size = sizeof(struct v3dv_cmd_buffer_attachment_state); const uint32_t attachment_state_total_size = attachment_state_item_size * state->meta.attachment_count; - state->attachment_count = state->meta.attachment_count; memcpy(state->attachments, state->meta.attachments, attachment_state_total_size); diff --git a/src/broadcom/vulkan/v3dv_private.h b/src/broadcom/vulkan/v3dv_private.h index baa8e071fb4..b59a62cc6b7 100644 --- a/src/broadcom/vulkan/v3dv_private.h +++ b/src/broadcom/vulkan/v3dv_private.h @@ -834,7 +834,7 @@ struct v3dv_cmd_buffer_state { */ bool tile_aligned_render_area; - uint32_t attachment_count; + uint32_t attachment_alloc_count; struct v3dv_cmd_buffer_attachment_state *attachments; struct v3dv_vertex_binding vertex_bindings[MAX_VBS];