On Fri, Jun 10, 2016 at 4:12 PM, Nanley Chery <[email protected]> wrote:
> From: Nanley Chery <[email protected]> > > 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 <[email protected]> > Cc: "12.0" <[email protected]> > --- > 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) { > 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. --Jason > + 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 > >
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
