vulkan/overlay: deal with unknown pNext structures
authorLionel Landwerlin <lionel.g.landwerlin@intel.com>
Sat, 12 Nov 2022 22:24:11 +0000 (00:24 +0200)
committerLionel Landwerlin <lionel.g.landwerlin@intel.com>
Mon, 17 Apr 2023 12:41:58 +0000 (15:41 +0300)
To implement some of the features of the layer, we need to enable some
of the feature bits at device/command_buffer creation. To do so, we
need to edit some of the structures coming from the application. Most
of those are const so we need to clone them before edition.

This change disables some of the layer features if we run into a
situation where one of the structure we need to clone is unknown such
that we can't make a copy of it (since we don't know its size).

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/7677
Cc: mesa-stable
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/19897>

src/vulkan/overlay-layer/overlay.cpp

index 105b023..3d02ade 100644 (file)
@@ -89,6 +89,8 @@ struct device_data {
    struct queue_data **queues;
    uint32_t n_queues;
 
+   bool pipeline_statistics_enabled;
+
    /* For a single frame */
    struct frame_stat frame_stats;
 };
@@ -290,6 +292,16 @@ static VkLayerDeviceCreateInfo *get_device_chain_info(const VkDeviceCreateInfo *
    return NULL;
 }
 
+static void
+free_chain(struct VkBaseOutStructure *chain)
+{
+   while (chain) {
+      void *node = chain;
+      chain = chain->pNext;
+      free(node);
+   }
+}
+
 static struct VkBaseOutStructure *
 clone_chain(const struct VkBaseInStructure *chain)
 {
@@ -297,6 +309,11 @@ clone_chain(const struct VkBaseInStructure *chain)
 
    vk_foreach_struct_const(item, chain) {
       size_t item_size = vk_structure_type_size(item);
+      if (item_size == 0) {
+         free_chain(head);
+         return NULL;
+      }
+
       struct VkBaseOutStructure *new_item =
          (struct VkBaseOutStructure *)malloc(item_size);;
 
@@ -312,16 +329,6 @@ clone_chain(const struct VkBaseInStructure *chain)
    return head;
 }
 
-static void
-free_chain(struct VkBaseOutStructure *chain)
-{
-   while (chain) {
-      void *node = chain;
-      chain = chain->pNext;
-      free(node);
-   }
-}
-
 /**/
 
 static struct instance_data *new_instance_data(VkInstance instance)
@@ -2203,34 +2210,44 @@ static VkResult overlay_BeginCommandBuffer(
     * we have the right inheritance.
     */
    if (cmd_buffer_data->level == VK_COMMAND_BUFFER_LEVEL_SECONDARY) {
-      VkCommandBufferBeginInfo *begin_info = (VkCommandBufferBeginInfo *)
-         clone_chain((const struct VkBaseInStructure *)pBeginInfo);
-      VkCommandBufferInheritanceInfo *parent_inhe_info = (VkCommandBufferInheritanceInfo *)
-         vk_find_struct(begin_info, COMMAND_BUFFER_INHERITANCE_INFO);
-      VkCommandBufferInheritanceInfo inhe_info = {
-         VK_STRUCTURE_TYPE_COMMAND_BUFFER_INHERITANCE_INFO,
-         NULL,
-         VK_NULL_HANDLE,
-         0,
-         VK_NULL_HANDLE,
-         VK_FALSE,
-         0,
-         overlay_query_flags,
-      };
+      VkCommandBufferBeginInfo begin_info = *pBeginInfo;
 
-      if (parent_inhe_info)
-         parent_inhe_info->pipelineStatistics = overlay_query_flags;
-      else {
-         inhe_info.pNext = begin_info->pNext;
-         begin_info->pNext = &inhe_info;
-      }
+      struct VkBaseOutStructure *new_pnext =
+         clone_chain((const struct VkBaseInStructure *)pBeginInfo->pNext);
+      VkCommandBufferInheritanceInfo inhe_info;
 
-      VkResult result = device_data->vtable.BeginCommandBuffer(commandBuffer, pBeginInfo);
+      /* If there was no pNext chain given or we managed to copy it, we can
+       * add our stuff in there.
+       *
+       * Otherwise, keep the old pointer. We failed to copy the pNext chain,
+       * meaning there is an unknown extension somewhere in there.
+       */
+      if (new_pnext || pBeginInfo->pNext == NULL) {
+         begin_info.pNext = new_pnext;
+
+         VkCommandBufferInheritanceInfo *parent_inhe_info = (VkCommandBufferInheritanceInfo *)
+            vk_find_struct(new_pnext, COMMAND_BUFFER_INHERITANCE_INFO);
+         inhe_info = (VkCommandBufferInheritanceInfo) {
+            VK_STRUCTURE_TYPE_COMMAND_BUFFER_INHERITANCE_INFO,
+            NULL,
+            VK_NULL_HANDLE,
+            0,
+            VK_NULL_HANDLE,
+            VK_FALSE,
+            0,
+            overlay_query_flags,
+         };
+
+         if (parent_inhe_info)
+            parent_inhe_info->pipelineStatistics = overlay_query_flags;
+         else
+            __vk_append_struct(&begin_info, &inhe_info);
+      }
 
-      if (!parent_inhe_info)
-         begin_info->pNext = inhe_info.pNext;
+      VkResult result = device_data->vtable.BeginCommandBuffer(
+         commandBuffer, &begin_info);
 
-      free_chain((struct VkBaseOutStructure *)begin_info);
+      free_chain(new_pnext);
 
       return result;
    }
@@ -2334,7 +2351,7 @@ static VkResult overlay_AllocateCommandBuffers(
 
    VkQueryPool pipeline_query_pool = VK_NULL_HANDLE;
    VkQueryPool timestamp_query_pool = VK_NULL_HANDLE;
-   if (device_data->instance->pipeline_statistics_enabled &&
+   if (device_data->pipeline_statistics_enabled &&
        pAllocateInfo->level == VK_COMMAND_BUFFER_LEVEL_PRIMARY) {
       VkQueryPoolCreateInfo pool_info = {
          VK_STRUCTURE_TYPE_QUERY_POOL_CREATE_INFO,
@@ -2517,29 +2534,33 @@ static VkResult overlay_CreateDevice(
    VkPhysicalDeviceFeatures device_features = {};
    VkPhysicalDeviceFeatures *device_features_ptr = NULL;
 
-   VkDeviceCreateInfo *device_info = (VkDeviceCreateInfo *)
-      clone_chain((const struct VkBaseInStructure *)pCreateInfo);
+   VkDeviceCreateInfo create_info = *pCreateInfo;
 
-   VkPhysicalDeviceFeatures2 *device_features2 = (VkPhysicalDeviceFeatures2 *)
-      vk_find_struct(device_info, PHYSICAL_DEVICE_FEATURES_2);
-   if (device_features2) {
-      /* Can't use device_info->pEnabledFeatures when VkPhysicalDeviceFeatures2 is present */
-      device_features_ptr = &device_features2->features;
-   } else {
-      if (device_info->pEnabledFeatures)
-         device_features = *(device_info->pEnabledFeatures);
-      device_features_ptr = &device_features;
-      device_info->pEnabledFeatures = &device_features;
-   }
+   struct VkBaseOutStructure *new_pnext =
+      clone_chain((const struct VkBaseInStructure *) pCreateInfo->pNext);
+   if (new_pnext != NULL) {
+      create_info.pNext = new_pnext;
 
-   if (instance_data->pipeline_statistics_enabled) {
-      device_features_ptr->inheritedQueries = true;
-      device_features_ptr->pipelineStatisticsQuery = true;
-   }
+      VkPhysicalDeviceFeatures2 *device_features2 = (VkPhysicalDeviceFeatures2 *)
+         vk_find_struct(new_pnext, PHYSICAL_DEVICE_FEATURES_2);
+      if (device_features2) {
+         /* Can't use device_info->pEnabledFeatures when VkPhysicalDeviceFeatures2 is present */
+         device_features_ptr = &device_features2->features;
+      } else {
+         if (create_info.pEnabledFeatures)
+            device_features = *(create_info.pEnabledFeatures);
+         device_features_ptr = &device_features;
+         create_info.pEnabledFeatures = &device_features;
+      }
 
+      if (instance_data->pipeline_statistics_enabled) {
+         device_features_ptr->inheritedQueries = true;
+         device_features_ptr->pipelineStatisticsQuery = true;
+      }
+   }
 
-   VkResult result = fpCreateDevice(physicalDevice, device_info, pAllocator, pDevice);
-   free_chain((struct VkBaseOutStructure *)device_info);
+   VkResult result = fpCreateDevice(physicalDevice, &create_info, pAllocator, pDevice);
+   free_chain(new_pnext);
    if (result != VK_SUCCESS) return result;
 
    struct device_data *device_data = new_device_data(*pDevice, instance_data);
@@ -2556,6 +2577,10 @@ static VkResult overlay_CreateDevice(
 
    device_map_queues(device_data, pCreateInfo);
 
+   device_data->pipeline_statistics_enabled =
+      new_pnext != NULL &&
+      instance_data->pipeline_statistics_enabled;
+
    return result;
 }