<div dir="ltr"> <div><div class="gmail_extra"> <div class="gmail_quote">On Fri, Jun 10, 2016 at 4:12 PM, Nanley Chery <<a href="mailto:nanleychery@gmail.com" target="_blank">nanleychery@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);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. This fixes a segfault seen in the McNopper demo, VKTS_Example09. 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 | 51 ++++++++++++++++++++++++++++++----------- 1 file changed, 37 insertions(+), 14 deletions(-) diff --git a/src/intel/vulkan/anv_pipeline.c b/src/intel/vulkan/anv_pipeline.c index d5a3a21..4ce13eb 100644 --- a/src/intel/vulkan/anv_pipeline.c +++ b/src/intel/vulkan/anv_pipeline.c @@ -979,11 +979,28 @@ 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); + bool fs_used = pipeline->active_stages & VK_SHADER_STAGE_FRAGMENT_BIT; + bool color_att_used = false; + for (unsigned i = 0; i < subpass->color_count; ++i) { + if (subpass->color_attachments[i] != VK_ATTACHMENT_UNUSED) { + color_att_used = true; + break; + } + } + + /* Section 9.2 of the Vulkan 1.0.15 spec says: + * + * pViewportState is [...] NULL if the pipeline + * has rasterization disabled. + */ + if (fs_used) { </blockquote><div> </div><div>fs_used is not equivalent to rasterization being disabled. The only think I know of that can actually cause rasterization to be disabled is if you set rasterizerDiscardEnable. If you don't have a fragment shader they still get rastierized, but it's depth-only. </div><div>--Jason </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + 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; @@ -1008,7 +1025,14 @@ 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. + */ + if (fs_used && color_att_used && + states & (1 << VK_DYNAMIC_STATE_BLEND_CONSTANTS)) { assert(pCreateInfo->pColorBlendState); typed_memcpy(dynamic->blend_constants, pCreateInfo->pColorBlendState->blendConstants, 4); @@ -1020,14 +1044,16 @@ 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 (fs_used && 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 +1061,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 +1068,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 +1075,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 </blockquote></div> </div></div></div>