vulkan: don't keep a buffer-chain node whose buffer was not mapped

.github/workflows/Linux-vulkan-validation.yml

@@ -253,6 +253,10 @@ jobs:
           # a window into it take the other upload path (the one
           # beetle-psx takes).
           HARNESS_CORE_XRGB8888=1 SAN=none VK_DEBUG=1 GFX_INSTRUMENT=1 CYCLES=1 make -C samples/gfx/threaded_video check-vulkan
+          # And with the first map of a buffer-chain block refused on
+          # each device: the driver must not suballocate from a buffer
+          # it could not map.
+          SAN=none VK_DEBUG=1 GFX_INSTRUMENT=1 CYCLES=1 make -C samples/gfx/threaded_video check-vulkan-mapfail
           # And with the harness core as a Vulkan hardware core, which
           # runs its own lanes alone: the core's image must not be drawn
           # from once the core has destroyed it - at a content close,

gfx/drivers/vulkan.c

@@ -854,6 +854,8 @@ static struct vk_buffer vulkan_create_buffer(
    return buffer;
 }
 
+static void vulkan_destroy_buffer(VkDevice device, struct vk_buffer *buffer);
+
 static struct vk_buffer_node *vulkan_buffer_chain_alloc_node(
       const struct vulkan_context *context,
       size_t len, VkBufferUsageFlags usage)
@@ -864,6 +866,14 @@ static struct vk_buffer_node *vulkan_buffer_chain_alloc_node(
       return NULL;
    node->buffer = vulkan_create_buffer(
          context, len, usage);
+   /* A buffer that could not be created or mapped has nothing to
+    * suballocate from, so do not hand it to the chain. */
+   if (!node->buffer.mapped)
+   {
+      vulkan_destroy_buffer(context->device, &node->buffer);
+      free(node);
+      return NULL;
+   }
    node->next   = NULL;
    return node;
 }
@@ -9201,12 +9211,17 @@ static void vulkan_run_hdr_pipeline(VkPipeline pipeline, VkRenderPass render_pas
    {
       VkWriteDescriptorSet write;
       VkDescriptorImageInfo image_info;
-      VkDescriptorSet set = vulkan_descriptor_manager_alloc(
+      VkDescriptorSet set;
+      /* NULL when the uniform buffer could not be mapped at init */
+      vulkan_hdr_uniform_t* mapped_ubo = (vulkan_hdr_uniform_t*)ubo->mapped;
+
+      if (!mapped_ubo)
+         return;
+
+      set = vulkan_descriptor_manager_alloc(
             vk->context->device,
             &vk->chain->descriptor_manager);
 
-      vulkan_hdr_uniform_t* mapped_ubo = (vulkan_hdr_uniform_t*)ubo->mapped;
-
       if (set == VK_NULL_HANDLE)
          return;
 

samples/gfx/threaded_video/Makefile

@@ -118,6 +118,22 @@ check-vulkan-rebar: all $(FR_LIB) fake_rebar_layer.json
 	    fi; \
 	 done; echo "check-vulkan-rebar: passed"
 
+# check-vulkan through the same layer with the first map of a 64 KiB
+# buffer on each device refused (FAKE_REBAR_FAIL_MAP_SIZE): the size of
+# a buffer-chain block, so a chain is handed a buffer it cannot write.
+# Every lane must still pass, and at least one map must have been
+# refused.
+check-vulkan-mapfail: all $(FR_LIB) fake_rebar_layer.json
+	@VK_ADD_LAYER_PATH=. VK_INSTANCE_LAYERS=VK_LAYER_RA_fake_rebar \
+	 FAKE_REBAR_FAIL_MAP_SIZE=65536 \
+	 $(MAKE) -s --no-print-directory check-vulkan > mapfail.log 2>&1; rc=$$?; \
+	 grep -E 'all lanes|FAIL|Validation|check-vulkan|fake-rebar-map' mapfail.log; \
+	 if [ $$rc -ne 0 ]; then cat mapfail.log; rm -f mapfail.log; exit $$rc; fi; \
+	 refused=$$(awk '/fake-rebar-map:/ { n += $$NF } END { print n + 0 }' mapfail.log); \
+	 rm -f mapfail.log; \
+	 if [ "$$refused" = 0 ]; then echo "check-vulkan-mapfail: no map was refused"; exit 1; fi; \
+	 echo "check-vulkan-mapfail: $$refused buffer maps refused, every lane passed"
+
 # The same lanes on any real driver named in DRIVER (gl, glcore, ...),
 # through whatever context the display offers - in CI, llvmpipe under
 # Xvfb with LIBGL_ALWAYS_SOFTWARE=1. No validation layer here: a
@@ -150,6 +166,6 @@ check-driver: all
 clean:
 	rm -f $(TARGET) check.log harness_core.so retroarch_nomain.o retroarch_nomain.d \
 	      harness_main.o harness_main.d harness_cocoa.o harness_cocoa.d default.metallib \
-	      rebar.log libfake_rebar_layer.so fake_rebar_layer.dll fake_rebar_layer.json
+	      rebar.log mapfail.log libfake_rebar_layer.so fake_rebar_layer.dll fake_rebar_layer.json
 
-.PHONY: all check check-vulkan check-vulkan-rebar check-driver clean
+.PHONY: all check check-vulkan check-vulkan-rebar check-vulkan-mapfail check-driver clean

samples/gfx/threaded_video/fake_rebar_layer.c

@@ -38,6 +38,11 @@
  * that every type-2 allocation refused falls back and the run still
  * passes.
  *
+ * With FAKE_REBAR_FAIL_MAP_SIZE=n, the first vkMapMemory of n bytes on
+ * each device fails, as a map can on a real driver out of address
+ * space: check-vulkan-mapfail runs the lanes so, and the driver is to
+ * get past the buffer it could not map.
+ *
  * One instance and one device at a time, which is how the harness runs
  * the driver; the next layer's entry points are kept in globals. */
 
@@ -67,6 +72,7 @@ static unsigned long             s_allocs[FR_TYPES];
 static unsigned long             s_refused;
 static unsigned long             s_images_bar;
 static VkDeviceMemory            s_live_bar[FR_LIVE];
+static unsigned long             s_map_refused;
 
 static PFN_vkVoidFunction fr_next_instance(const char *name)
 {
@@ -80,6 +86,13 @@ static int fr_refuse(void)
    return v && *v && strcmp(v, "0");
 }
 
+/* FAKE_REBAR_FAIL_MAP_SIZE, or 0 */
+static VkDeviceSize fr_fail_map_size(void)
+{
+   const char *v = getenv("FAKE_REBAR_FAIL_MAP_SIZE");
+   return v ? (VkDeviceSize)strtoul(v, NULL, 10) : 0;
+}
+
 static void fr_fill(VkPhysicalDeviceMemoryProperties *p)
 {
    memset(p, 0, sizeof(*p));
@@ -203,6 +216,22 @@ static VKAPI_ATTR void VKAPI_CALL fr_FreeMemory(VkDevice device,
    next(device, memory, cb);
 }
 
+static VKAPI_ATTR VkResult VKAPI_CALL fr_MapMemory(VkDevice device,
+      VkDeviceMemory memory, VkDeviceSize offset, VkDeviceSize size,
+      VkMemoryMapFlags flags, void **data)
+{
+   PFN_vkMapMemory next = (PFN_vkMapMemory)
+      s_next_gdpa(device, "vkMapMemory");
+   VkDeviceSize fail    = fr_fail_map_size();
+   if (fail && size == fail && !s_map_refused)
+   {
+      s_map_refused++;
+      *data = NULL;
+      return VK_ERROR_MEMORY_MAP_FAILED;
+   }
+   return next(device, memory, offset, size, flags, data);
+}
+
 static void fr_count_image(VkDeviceMemory memory)
 {
    unsigned i;
@@ -241,7 +270,10 @@ static VKAPI_ATTR void VKAPI_CALL fr_DestroyDevice(VkDevice device,
       s_next_gdpa(device, "vkDestroyDevice");
    printf("[info] fake-rebar: allocations in type 0 %lu, type 1 %lu, type 2 %lu; type 2 refused %lu; images in type 2 %lu\n",
          s_allocs[0], s_allocs[1], s_allocs[2], s_refused, s_images_bar);
+   if (fr_fail_map_size())
+      printf("[info] fake-rebar-map: maps refused %lu\n", s_map_refused);
    fflush(stdout);
+   s_map_refused = 0;
    next(device, cb);
 }
 
@@ -299,6 +331,7 @@ static PFN_vkVoidFunction fr_device_hook(const char *name)
    FR_HOOK(GetBufferMemoryRequirements2);
    FR_HOOK(GetImageMemoryRequirements2);
    FR_HOOK(AllocateMemory);
+   FR_HOOK(MapMemory);
    FR_HOOK(FreeMemory);
    FR_HOOK(BindImageMemory);
    FR_HOOK(BindImageMemory2);