Thanks for cleaning this up. Series is
Reviewed-by: Jason Ekstrand <ja...@jlekstrand.net>
Cc: "12.0" <mesa-sta...@lists.freedesktop.org>
On Jun 10, 2016 8:02 PM, "Nanley Chery" <nanleych...@gmail.com> wrote:
> From: Nanley Chery <nanley.g.ch...@intel.com>
>
> Add guards to prevent dereferencing NULL dynamic pipeline state. Asserts
> of pCreateInfo members are moved to the earliest points at which they
> should not be NULL.
>
> This fixes a segfault seen in the McNopper demo, VKTS_Example09.
>
> v3 (Jason Ekstrand):
> - Fix disabled rasterization check
> - Revert opaque detection of color attachment usage
>
> Signed-off-by: Nanley Chery <nanley.g.ch...@intel.com>
> Cc: "12.0" <mesa-sta...@lists.freedesktop.org>
> ---
> src/intel/vulkan/anv_pipeline.c | 70
> ++++++++++++++++++++++++++++-------------
> 1 file changed, 48 insertions(+), 22 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_pipeline.c
> b/src/intel/vulkan/anv_pipeline.c
> index ae03787..60b7c6b 100644
> --- a/src/intel/vulkan/anv_pipeline.c
> +++ b/src/intel/vulkan/anv_pipeline.c
> @@ -979,18 +979,27 @@ copy_non_dynamic_state(struct anv_pipeline *pipeline,
>
> struct anv_dynamic_state *dynamic = &pipeline->dynamic_state;
>
> - dynamic->viewport.count = pCreateInfo->pViewportState->viewportCount;
> - if (states & (1 << VK_DYNAMIC_STATE_VIEWPORT)) {
> - typed_memcpy(dynamic->viewport.viewports,
> - pCreateInfo->pViewportState->pViewports,
> - pCreateInfo->pViewportState->viewportCount);
> - }
> + /* Section 9.2 of the Vulkan 1.0.15 spec says:
> + *
> + * pViewportState is [...] NULL if the pipeline
> + * has rasterization disabled.
> + */
> + if (!pCreateInfo->pRasterizationState->rasterizerDiscardEnable) {
> + assert(pCreateInfo->pViewportState);
> +
> + dynamic->viewport.count =
> pCreateInfo->pViewportState->viewportCount;
> + if (states & (1 << VK_DYNAMIC_STATE_VIEWPORT)) {
> + typed_memcpy(dynamic->viewport.viewports,
> + pCreateInfo->pViewportState->pViewports,
> + pCreateInfo->pViewportState->viewportCount);
> + }
>
> - dynamic->scissor.count = pCreateInfo->pViewportState->scissorCount;
> - if (states & (1 << VK_DYNAMIC_STATE_SCISSOR)) {
> - typed_memcpy(dynamic->scissor.scissors,
> - pCreateInfo->pViewportState->pScissors,
> - pCreateInfo->pViewportState->scissorCount);
> + dynamic->scissor.count = pCreateInfo->pViewportState->scissorCount;
> + if (states & (1 << VK_DYNAMIC_STATE_SCISSOR)) {
> + typed_memcpy(dynamic->scissor.scissors,
> + pCreateInfo->pViewportState->pScissors,
> + pCreateInfo->pViewportState->scissorCount);
> + }
> }
>
> if (states & (1 << VK_DYNAMIC_STATE_LINE_WIDTH)) {
> @@ -1008,10 +1017,27 @@ copy_non_dynamic_state(struct anv_pipeline
> *pipeline,
> pCreateInfo->pRasterizationState->depthBiasSlopeFactor;
> }
>
> - if (states & (1 << VK_DYNAMIC_STATE_BLEND_CONSTANTS)) {
> + /* Section 9.2 of the Vulkan 1.0.15 spec says:
> + *
> + * pColorBlendState is [...] NULL if the pipeline has rasterization
> + * disabled or if the subpass of the render pass the pipeline is
> + * created against does not use any color attachments.
> + */
> + bool uses_color_att = false;
> + for (unsigned i = 0; i < subpass->color_count; ++i) {
> + if (subpass->color_attachments[i] != VK_ATTACHMENT_UNUSED) {
> + uses_color_att = true;
> + break;
> + }
> + }
> +
> + if (uses_color_att &&
> + !pCreateInfo->pRasterizationState->rasterizerDiscardEnable) {
> assert(pCreateInfo->pColorBlendState);
> - typed_memcpy(dynamic->blend_constants,
> - pCreateInfo->pColorBlendState->blendConstants, 4);
> +
> + if (states & (1 << VK_DYNAMIC_STATE_BLEND_CONSTANTS))
> + typed_memcpy(dynamic->blend_constants,
> + pCreateInfo->pColorBlendState->blendConstants, 4);
> }
>
> /* If there is no depthstencil attachment, then don't read
> @@ -1020,14 +1046,17 @@ copy_non_dynamic_state(struct anv_pipeline
> *pipeline,
> * no need to override the depthstencil defaults in
> * anv_pipeline::dynamic_state when there is no depthstencil
> attachment.
> *
> - * From the Vulkan spec (20 Oct 2015, git-aa308cb):
> + * Section 9.2 of the Vulkan 1.0.15 spec says:
> *
> - * pDepthStencilState [...] may only be NULL if renderPass and
> subpass
> - * specify a subpass that has no depth/stencil attachment.
> + * pDepthStencilState is [...] NULL if the pipeline has
> rasterization
> + * disabled or if the subpass of the render pass the pipeline is
> created
> + * against does not use a depth/stencil attachment.
> */
> - if (subpass->depth_stencil_attachment != VK_ATTACHMENT_UNUSED) {
> + if (!pCreateInfo->pRasterizationState->rasterizerDiscardEnable &&
> + subpass->depth_stencil_attachment != VK_ATTACHMENT_UNUSED) {
> + assert(pCreateInfo->pDepthStencilState);
> +
> if (states & (1 << VK_DYNAMIC_STATE_DEPTH_BOUNDS)) {
> - assert(pCreateInfo->pDepthStencilState);
> dynamic->depth_bounds.min =
> pCreateInfo->pDepthStencilState->minDepthBounds;
> dynamic->depth_bounds.max =
> @@ -1035,7 +1064,6 @@ copy_non_dynamic_state(struct anv_pipeline *pipeline,
> }
>
> if (states & (1 << VK_DYNAMIC_STATE_STENCIL_COMPARE_MASK)) {
> - assert(pCreateInfo->pDepthStencilState);
> dynamic->stencil_compare_mask.front =
> pCreateInfo->pDepthStencilState->front.compareMask;
> dynamic->stencil_compare_mask.back =
> @@ -1043,7 +1071,6 @@ copy_non_dynamic_state(struct anv_pipeline *pipeline,
> }
>
> if (states & (1 << VK_DYNAMIC_STATE_STENCIL_WRITE_MASK)) {
> - assert(pCreateInfo->pDepthStencilState);
> dynamic->stencil_write_mask.front =
> pCreateInfo->pDepthStencilState->front.writeMask;
> dynamic->stencil_write_mask.back =
> @@ -1051,7 +1078,6 @@ copy_non_dynamic_state(struct anv_pipeline *pipeline,
> }
>
> if (states & (1 << VK_DYNAMIC_STATE_STENCIL_REFERENCE)) {
> - assert(pCreateInfo->pDepthStencilState);
> dynamic->stencil_reference.front =
> pCreateInfo->pDepthStencilState->front.reference;
> dynamic->stencil_reference.back =
> --
> 2.8.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
