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 <jarred.davies@imgtec.com>

Reviewed-by: Frank Binns <frank.binns@imgtec.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/21690>
This commit is contained in:
Jarred Davies 2023-01-31 19:24:30 +00:00 committed by Marge Bot
parent 7176e0c160
commit 1115a29025
5 changed files with 41 additions and 46 deletions

View file

@ -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;
}

View file

@ -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++;
}

View file

@ -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. */

View file

@ -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){

View file

@ -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;