From: Jarred Davies Date: Tue, 31 Jan 2023 19:24:30 +0000 (+0000) Subject: pvr: Fix segfaults when pDepthStencilAttachment is NULL X-Git-Tag: upstream/23.3.3~11663 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=1115a2902593e8fc379a4781aebde54cf32e1bd5;p=platform%2Fupstream%2Fmesa.git pvr: Fix segfaults when pDepthStencilAttachment is NULL depth_stencil_attachment has been changed from a pointer to the attachment idx to just the attachment idx, as this avoids the driver having to check for NULL when comparing attachments indexes with depth_stencil_attachment. Anyplace that relies on depth_stencil_attachment being a valid index must already check that depth_stencil_attachment is not VK_ATTACHMENT_UNUSED, so this change avoids having to check both the pointer and the index for the same information. Noticed when running dEQP-VK.api.smoke.triangle Signed-off-by: Jarred Davies Reviewed-by: Frank Binns Part-of: --- diff --git a/src/imagination/vulkan/pvr_cmd_buffer.c b/src/imagination/vulkan/pvr_cmd_buffer.c index 8c2bee2..4282a6d 100644 --- a/src/imagination/vulkan/pvr_cmd_buffer.c +++ b/src/imagination/vulkan/pvr_cmd_buffer.c @@ -764,11 +764,11 @@ pvr_load_op_constants_create_and_upload(struct pvr_cmd_buffer *cmd_buffer, const struct pvr_render_pass_attachment *attachment; const struct pvr_image_view *image_view; - assert(*load_op->subpass->depth_stencil_attachment != + assert(load_op->subpass->depth_stencil_attachment != VK_ATTACHMENT_UNUSED); assert(!load_op->is_hw_object); attachment = - &pass->attachments[*load_op->subpass->depth_stencil_attachment]; + &pass->attachments[load_op->subpass->depth_stencil_attachment]; image_view = render_pass_info->attachments[attachment->index]; @@ -783,10 +783,10 @@ pvr_load_op_constants_create_and_upload(struct pvr_cmd_buffer *cmd_buffer, const struct pvr_render_pass_attachment *attachment; VkClearValue clear_value; - assert(*load_op->subpass->depth_stencil_attachment != + assert(load_op->subpass->depth_stencil_attachment != VK_ATTACHMENT_UNUSED); attachment = - &pass->attachments[*load_op->subpass->depth_stencil_attachment]; + &pass->attachments[load_op->subpass->depth_stencil_attachment]; clear_value = render_pass_info->clear_values[attachment->index]; @@ -4364,11 +4364,11 @@ pvr_setup_isp_faces_and_control(struct pvr_cmd_buffer *const cmd_buffer, const bool rasterizer_discard = dynamic_state->rs.rasterizer_discard_enable; const uint32_t subpass_idx = pass_info->subpass_idx; - const uint32_t *depth_stencil_attachment_idx = + const uint32_t depth_stencil_attachment_idx = pass_info->pass->subpasses[subpass_idx].depth_stencil_attachment; const struct pvr_image_view *const attachment = - depth_stencil_attachment_idx - ? pass_info->attachments[*depth_stencil_attachment_idx] + depth_stencil_attachment_idx != VK_ATTACHMENT_UNUSED + ? pass_info->attachments[depth_stencil_attachment_idx] : NULL; const enum PVRX(TA_OBJTYPE) @@ -7032,12 +7032,15 @@ pvr_stencil_has_self_dependency(const struct pvr_cmd_buffer_state *const state) pvr_get_current_subpass(state); const uint32_t *const input_attachments = current_subpass->input_attachments; + if (current_subpass->depth_stencil_attachment == VK_ATTACHMENT_UNUSED) + return false; + /* We only need to check the current software subpass as we don't support * merging to/from a subpass with self-dep stencil. */ for (uint32_t i = 0; i < current_subpass->input_count; i++) { - if (input_attachments[i] == *current_subpass->depth_stencil_attachment) + if (input_attachments[i] == current_subpass->depth_stencil_attachment) return true; } diff --git a/src/imagination/vulkan/pvr_hw_pass.c b/src/imagination/vulkan/pvr_hw_pass.c index ee6dc66..dccb9d5 100644 --- a/src/imagination/vulkan/pvr_hw_pass.c +++ b/src/imagination/vulkan/pvr_hw_pass.c @@ -541,12 +541,12 @@ pvr_subpass_setup_render_init(struct pvr_renderpass_context *ctx) hw_subpass->stencil_clear)) { struct pvr_render_int_attachment *int_ds_attach; - assert(*input_subpass->depth_stencil_attachment != + assert(input_subpass->depth_stencil_attachment != VK_ATTACHMENT_UNUSED); - assert(*input_subpass->depth_stencil_attachment < + assert(input_subpass->depth_stencil_attachment < ctx->pass->attachment_count); int_ds_attach = - &ctx->int_attach[*input_subpass->depth_stencil_attachment]; + &ctx->int_attach[input_subpass->depth_stencil_attachment]; assert(hw_render->ds_attach_idx == VK_ATTACHMENT_UNUSED || hw_render->ds_attach_idx == int_ds_attach->attachment->index); @@ -561,7 +561,7 @@ pvr_subpass_setup_render_init(struct pvr_renderpass_context *ctx) } } - if (*input_subpass->depth_stencil_attachment != VK_ATTACHMENT_UNUSED) + if (input_subpass->depth_stencil_attachment != VK_ATTACHMENT_UNUSED) first_ds = false; for (uint32_t j = 0U; j < input_subpass->color_count; j++) { @@ -937,11 +937,11 @@ pvr_copy_z_replicate_details(struct pvr_renderpass_context *ctx, uint32_t z_replicate; bool found = false; - assert(*input_subpass->depth_stencil_attachment >= 0U && - *input_subpass->depth_stencil_attachment < + assert(input_subpass->depth_stencil_attachment >= 0U && + input_subpass->depth_stencil_attachment < (int32_t)ctx->pass->attachment_count); - int_ds_attach = &ctx->int_attach[*input_subpass->depth_stencil_attachment]; + int_ds_attach = &ctx->int_attach[input_subpass->depth_stencil_attachment]; assert(hw_subpass->z_replicate == -1); @@ -1475,7 +1475,7 @@ pvr_enable_z_replicate(struct pvr_renderpass_context *ctx, struct pvr_renderpass_subpass *subpass = &ctx->subpasses[i]; struct pvr_render_subpass *input_subpass = subpass->input_subpass; - if (*input_subpass->depth_stencil_attachment == replicate_attach_idx) { + if (input_subpass->depth_stencil_attachment == replicate_attach_idx) { first_use = i; break; } @@ -1488,8 +1488,7 @@ pvr_enable_z_replicate(struct pvr_renderpass_context *ctx, struct pvr_render_subpass *input_subpass = subpass->input_subpass; /* If the subpass writes to the attachment then enable z replication. */ - if (input_subpass->depth_stencil_attachment && - *input_subpass->depth_stencil_attachment == replicate_attach_idx && + if (input_subpass->depth_stencil_attachment == replicate_attach_idx && !subpass->z_replicate) { subpass->z_replicate = true; @@ -1693,7 +1692,7 @@ pvr_is_z_replicate_space_available(const struct pvr_device_info *dev_info, struct pvr_renderpass_subpass *subpass = &ctx->subpasses[i]; struct pvr_render_subpass *input_subpass = subpass->input_subpass; - if (*input_subpass->depth_stencil_attachment == (int32_t)attach_idx) { + if (input_subpass->depth_stencil_attachment == (int32_t)attach_idx) { first_use = i; break; } @@ -1820,12 +1819,12 @@ pvr_is_subpass_space_available(const struct pvr_device_info *dev_info, } if (sp_depth->incoming_ds_is_input) { - if (sp_depth->existing_ds_attach != *subpass->depth_stencil_attachment) { + if (sp_depth->existing_ds_attach != subpass->depth_stencil_attachment) { result = pvr_is_z_replicate_space_available( dev_info, ctx, alloc, - *subpass->depth_stencil_attachment, + subpass->depth_stencil_attachment, &sp_dsts->incoming_zrep); if (result != VK_SUCCESS) goto err_free_alloc; @@ -1970,12 +1969,10 @@ pvr_merge_subpass(const struct pvr_device *device, bool ret; /* Depth attachment for the incoming subpass. */ - if (*input_subpass->depth_stencil_attachment != VK_ATTACHMENT_UNUSED) { - int_ds_attach = - &ctx->int_attach[*input_subpass->depth_stencil_attachment]; - } else { + if (input_subpass->depth_stencil_attachment != VK_ATTACHMENT_UNUSED) + int_ds_attach = &ctx->int_attach[input_subpass->depth_stencil_attachment]; + else int_ds_attach = NULL; - } /* Attachment ID for the existing depth attachment. */ if (ctx->int_ds_attach) @@ -1986,7 +1983,7 @@ pvr_merge_subpass(const struct pvr_device *device, /* Is the incoming depth attachment used as an input to the incoming subpass? */ sp_depth.incoming_ds_is_input = - pvr_is_input(input_subpass, *input_subpass->depth_stencil_attachment); + pvr_is_input(input_subpass, input_subpass->depth_stencil_attachment); /* Is the current depth attachment used as an input to the incoming subpass? */ @@ -2216,12 +2213,12 @@ pvr_merge_subpass(const struct pvr_device *device, } if (sp_depth.incoming_ds_is_input) { - if (*input_subpass->depth_stencil_attachment != + if (input_subpass->depth_stencil_attachment != sp_depth.existing_ds_attach) { result = pvr_enable_z_replicate(ctx, hw_render, - *input_subpass->depth_stencil_attachment, + input_subpass->depth_stencil_attachment, &sp_dsts.incoming_zrep); if (result != VK_SUCCESS) goto end_merge_subpass; @@ -2324,9 +2321,9 @@ static VkResult pvr_schedule_subpass(const struct pvr_device *device, subpass_num, subpass->input_attachments, subpass->input_count); - if (*subpass->depth_stencil_attachment != VK_ATTACHMENT_UNUSED) { + if (subpass->depth_stencil_attachment != VK_ATTACHMENT_UNUSED) { struct pvr_render_int_attachment *int_depth_attach = - &ctx->int_attach[*subpass->depth_stencil_attachment]; + &ctx->int_attach[subpass->depth_stencil_attachment]; assert(int_depth_attach->remaining_count > 0U); int_depth_attach->remaining_count--; @@ -2573,7 +2570,7 @@ VkResult pvr_create_renderpass_hwsetup( int_attach->remaining_count += color_output_uses + input_attachment_uses; - if ((uint32_t)*subpass->depth_stencil_attachment == i) + if ((uint32_t)subpass->depth_stencil_attachment == i) int_attach->remaining_count++; } diff --git a/src/imagination/vulkan/pvr_pass.c b/src/imagination/vulkan/pvr_pass.c index 23f751b..49abb3a 100644 --- a/src/imagination/vulkan/pvr_pass.c +++ b/src/imagination/vulkan/pvr_pass.c @@ -72,14 +72,6 @@ static inline bool pvr_subpass_has_msaa_input_attachment( return false; } -static inline size_t -pvr_num_subpass_attachments(const VkSubpassDescription2 *desc) -{ - return desc->inputAttachmentCount + desc->colorAttachmentCount + - (desc->pResolveAttachments ? desc->colorAttachmentCount : 0) + - (desc->pDepthStencilAttachment != NULL); -} - static bool pvr_is_subpass_initops_flush_needed( const struct pvr_render_pass *pass, const struct pvr_renderpass_hwsetup_render *hw_render) @@ -453,8 +445,10 @@ VkResult pvr_CreateRenderPass2(VkDevice _device, subpass_attachment_count = 0; for (uint32_t i = 0; i < pCreateInfo->subpassCount; i++) { + const VkSubpassDescription2 *desc = &pCreateInfo->pSubpasses[i]; subpass_attachment_count += - pvr_num_subpass_attachments(&pCreateInfo->pSubpasses[i]); + desc->inputAttachmentCount + desc->colorAttachmentCount + + (desc->pResolveAttachments ? desc->colorAttachmentCount : 0); } vk_multialloc_add(&ma, @@ -579,9 +573,10 @@ VkResult pvr_CreateRenderPass2(VkDevice _device, } if (desc->pDepthStencilAttachment) { - subpass->depth_stencil_attachment = subpass_attachments++; - *subpass->depth_stencil_attachment = + subpass->depth_stencil_attachment = desc->pDepthStencilAttachment->attachment; + } else { + subpass->depth_stencil_attachment = VK_ATTACHMENT_UNUSED; } /* Give the dependencies a slice of the subpass_attachments array. */ diff --git a/src/imagination/vulkan/pvr_pipeline.c b/src/imagination/vulkan/pvr_pipeline.c index 9442cd7..89962d9 100644 --- a/src/imagination/vulkan/pvr_pipeline.c +++ b/src/imagination/vulkan/pvr_pipeline.c @@ -1941,9 +1941,9 @@ pvr_create_subpass_info(const VkGraphicsPipelineCreateInfo *const info) pass->attachments[subpass->color_attachments[i]].aspects; } - if (subpass->depth_stencil_attachment) { + if (subpass->depth_stencil_attachment != VK_ATTACHMENT_UNUSED) { attachment_aspects |= - pass->attachments[*subpass->depth_stencil_attachment].aspects; + pass->attachments[subpass->depth_stencil_attachment].aspects; } return (struct vk_subpass_info){ diff --git a/src/imagination/vulkan/pvr_private.h b/src/imagination/vulkan/pvr_private.h index ab96603..28a9262 100644 --- a/src/imagination/vulkan/pvr_private.h +++ b/src/imagination/vulkan/pvr_private.h @@ -1031,7 +1031,7 @@ struct pvr_render_subpass { uint32_t input_count; uint32_t *input_attachments; - uint32_t *depth_stencil_attachment; + uint32_t depth_stencil_attachment; /* Derived and other state. */ uint32_t dep_count;