radv/pipeline: Don't dereference NULL dynamic state pointers

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.
Acked-by: Edward O'Callaghan <funfunctor@folklore1984.net>
Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>

diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
index bb975311c077d5506c3924415050463c18ead8fe..404e840901575c6e5ac8cd94a83b656dbc91481c 100644 (file)
--- a/src/amd/vulkan/radv_pipeline.c
+++ b/src/amd/vulkan/radv_pipeline.c
@@ -722,6 +722,10 @@ radv_pipeline_init_blend_state(struct radv_pipeline *pipeline,
        bool blend_mrt0_is_dual_src = false;
        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;
@@ -1076,18 +1080,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)) {
@@ -1105,7 +1118,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);
@@ -1117,14 +1144,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 =
@@ -1132,7 +1162,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 =
@@ -1140,7 +1169,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 =
@@ -1148,7 +1176,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 =