Thanks for cleaning this up. Series is Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> Cc: "12.0" <<a href="mailto:mesa-stable@lists.freedesktop.org">mesa-stable@lists.freedesktop.org</a>> <div class="gmail_quote">On Jun 10, 2016 8:02 PM, "Nanley Chery" <<a href="mailto:nanleychery@gmail.com">nanleychery@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">From: Nanley Chery <<a href="mailto:nanley.g.chery@intel.com">nanley.g.chery@intel.com</a>> 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 <<a href="mailto:nanley.g.chery@intel.com">nanley.g.chery@intel.com</a>> Cc: "12.0" <<a href="mailto:mesa-stable@lists.freedesktop.org">mesa-stable@lists.freedesktop.org</a>> --- 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 <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div>