This is a port of commit a4a59172482d50318a5ae7f99021bcf0125e0f53:

   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, related to pColorBlendState, seen in Talos Principle
which I've observed after startup is completed and when exiting the menus,
depending on when Vulkan rendering is selected.

v2: moved the NULL check in radv_pipeline_init_blend_state to after the
declarations.
---
 src/amd/vulkan/radv_pipeline.c | 67 +++++++++++++++++++++++++++++-------------
 1 file changed, 47 insertions(+), 20 deletions(-)

diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
index eb64b69..d992a10 100644
--- a/src/amd/vulkan/radv_pipeline.c
+++ b/src/amd/vulkan/radv_pipeline.c
@@ -717,6 +717,10 @@ radv_pipeline_init_blend_state(struct radv_pipeline 
*pipeline,
        uint32_t blend_enable = 0, blend_need_alpha = 0;
        int i;
        bool single_cb_enable = false;
+
+       if (!vkblend)
+               return;
+
        if (extra && extra->custom_blend_mode) {
                single_cb_enable = true;
                mode = extra->custom_blend_mode;
@@ -1069,18 +1073,27 @@ radv_pipeline_init_dynamic_state(struct radv_pipeline 
*pipeline,
 
        struct radv_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)) {
@@ -1098,7 +1111,21 @@ radv_pipeline_init_dynamic_state(struct radv_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].attachment != 
VK_ATTACHMENT_UNUSED) {
+                       uses_color_att = true;
+                       break;
+               }
+       }
+
+       if (uses_color_att && states & (1 << VK_DYNAMIC_STATE_BLEND_CONSTANTS)) 
{
                assert(pCreateInfo->pColorBlendState);
                typed_memcpy(dynamic->blend_constants,
                             pCreateInfo->pColorBlendState->blendConstants, 4);
@@ -1110,14 +1137,17 @@ radv_pipeline_init_dynamic_state(struct radv_pipeline 
*pipeline,
         * no need to override the depthstencil defaults in
         * radv_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.attachment != 
VK_ATTACHMENT_UNUSED) {
+       if (!pCreateInfo->pRasterizationState->rasterizerDiscardEnable &&
+           subpass->depth_stencil_attachment.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 =
@@ -1125,7 +1155,6 @@ radv_pipeline_init_dynamic_state(struct radv_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 =
@@ -1133,7 +1162,6 @@ radv_pipeline_init_dynamic_state(struct radv_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 =
@@ -1141,7 +1169,6 @@ radv_pipeline_init_dynamic_state(struct radv_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.1.4

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to