Add device dispatch table magic value check

The device dispatch table has a magic value to ensure consistency, but
there was no message logged in case the magic value was corrupted. This
commit adds that check, fixes the message being printed for the instance
magic value check, and adds tests to ensure they are contining to be
checked in the future.

docs/LoaderLayerInterface.md

@@ -2500,6 +2500,22 @@ Android Vulkan documentation</a>.
     <td>Yes</td>
     <td><small>N/A</small></td>
   </tr>
+  <tr>
+  <td><small><b>LLP_LAYER_22</b></small></td>
+    <td>During <i>vkCreateDevice</i>, a layer <b>must not</b> modify the
+        <i>pDevice</i> pointer during prior to calling down to the lower
+        layers.<br/>
+        This is because the loader passes information in this pointer that is
+        necessary for the initialization code in the loader's terminator
+        function.<br/>
+        Instead, if the layer is overriding the <i>pDevice</i> pointer, it
+        <b>must</b> do so only after the call to the lower layers returns.
+    </td>
+    <td>The loader will likely crash.</td>
+    <td>No</td>
+    <td>Yes</td>
+    <td><small>N/A</small></td>
+  </tr>
 </table>
 
 

loader/generated/vk_loader_extensions.c

@@ -316,7 +316,7 @@ VKAPI_ATTR bool VKAPI_CALL loader_icd_init_entries(struct loader_icd_term *icd_t
 VKAPI_ATTR void VKAPI_CALL loader_init_device_dispatch_table(struct loader_dev_dispatch_table *dev_table, PFN_vkGetDeviceProcAddr gpa,
                                                              VkDevice dev) {
     VkLayerDispatchTable *table = &dev_table->core_dispatch;
-    assert(table->magic == DEVICE_DISP_TABLE_MAGIC_NUMBER);
+    if (table->magic != DEVICE_DISP_TABLE_MAGIC_NUMBER) { abort(); }
     for (uint32_t i = 0; i < MAX_NUM_UNKNOWN_EXTS; i++) dev_table->ext_dispatch[i] = (PFN_vkDevExt)vkDevExtError;
 
     // ---- Core 1_0 commands

loader/loader.c

@@ -5181,7 +5181,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateInstance(const VkInstanceCreateI
         loader_log(ptr_instance, VULKAN_LOADER_WARN_BIT, 0,
                    "terminator_CreateInstance: Instance pointer (%p) has invalid MAGIC value 0x%08x. Instance value possibly "
                    "corrupted by active layer (Policy #LLP_LAYER_21).  ",
-                   ptr_instance->magic);
+                   ptr_instance, ptr_instance->magic);
     }
 
     // Save the application version if it has been modified - layers sometimes needs features in newer API versions than
@@ -5564,6 +5564,17 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateDevice(VkPhysicalDevice physical
     VkBaseOutStructure *caller_dgci_container = NULL;
     VkDeviceGroupDeviceCreateInfoKHR *caller_dgci = NULL;
 
+    if (NULL == dev) {
+        loader_log(icd_term->this_instance, VULKAN_LOADER_WARN_BIT, 0,
+                   "terminator_CreateDevice: Loader device pointer null encountered.  Possibly set by active layer. (Policy "
+                   "#LLP_LAYER_22)");
+    } else if (LOADER_MAGIC_NUMBER != dev->loader_dispatch.core_dispatch.magic) {
+        loader_log(icd_term->this_instance, VULKAN_LOADER_WARN_BIT, 0,
+                   "terminator_CreateDevice: Device pointer (%p) has invalid MAGIC value 0x%08x. Device value possibly "
+                   "corrupted by active layer (Policy #LLP_LAYER_22).  ",
+                   dev, dev->loader_dispatch.core_dispatch.magic);
+    }
+
     dev->phys_dev_term = phys_dev_term;
 
     icd_exts.list = NULL;

loader/trampoline.c

@@ -97,10 +97,8 @@ LOADER_EXPORT VKAPI_ATTR PFN_vkVoidFunction VKAPI_CALL vkGetInstanceProcAddr(VkI
         struct loader_instance *ptr_instance = loader_get_instance(instance);
         // If we've gotten here and the pointer is NULL, it's invalid
         if (ptr_instance == NULL) {
-            loader_log(NULL,
-                       VULKAN_LOADER_FATAL_ERROR_BIT | VULKAN_LOADER_FATAL_ERROR_BIT | VULKAN_LOADER_ERROR_BIT |
-                           VULKAN_LOADER_VALIDATION_BIT,
-                       0, "vkGetInstanceProcAddr: Invalid instance [VUID-vkGetInstanceProcAddr-instance-parameter]");
+            loader_log(NULL, VULKAN_LOADER_FATAL_ERROR_BIT | VULKAN_LOADER_ERROR_BIT | VULKAN_LOADER_VALIDATION_BIT, 0,
+                       "vkGetInstanceProcAddr: Invalid instance [VUID-vkGetInstanceProcAddr-instance-parameter]");
             abort(); /* Intentionally fail so user can correct issue. */
         }
         // Return trampoline code for non-global entrypoints including any extensions.
@@ -770,10 +768,8 @@ LOADER_EXPORT VKAPI_ATTR void VKAPI_CALL vkDestroyInstance(VkInstance instance,
 
     ptr_instance = loader_get_instance(instance);
     if (ptr_instance == NULL) {
-        loader_log(
-            NULL,
-            VULKAN_LOADER_FATAL_ERROR_BIT | VULKAN_LOADER_FATAL_ERROR_BIT | VULKAN_LOADER_ERROR_BIT | VULKAN_LOADER_VALIDATION_BIT,
-            0, "vkDestroyInstance: Invalid instance [VUID-vkDestroyInstance-instance-parameter]");
+        loader_log(NULL, VULKAN_LOADER_FATAL_ERROR_BIT | VULKAN_LOADER_ERROR_BIT | VULKAN_LOADER_VALIDATION_BIT, 0,
+                   "vkDestroyInstance: Invalid instance [VUID-vkDestroyInstance-instance-parameter]");
         loader_platform_thread_unlock_mutex(&loader_lock);
         abort(); /* Intentionally fail so user can correct issue. */
     }
@@ -829,20 +825,15 @@ LOADER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkEnumeratePhysicalDevices(VkInstan
 
     inst = loader_get_instance(instance);
     if (NULL == inst) {
-        loader_log(
-            NULL,
-            VULKAN_LOADER_FATAL_ERROR_BIT | VULKAN_LOADER_FATAL_ERROR_BIT | VULKAN_LOADER_ERROR_BIT | VULKAN_LOADER_VALIDATION_BIT,
-            0, "vkEnumeratePhysicalDevices: Invalid instance [VUID-vkEnumeratePhysicalDevices-instance-parameter]");
+        loader_log(NULL, VULKAN_LOADER_FATAL_ERROR_BIT | VULKAN_LOADER_ERROR_BIT | VULKAN_LOADER_VALIDATION_BIT, 0,
+                   "vkEnumeratePhysicalDevices: Invalid instance [VUID-vkEnumeratePhysicalDevices-instance-parameter]");
         abort(); /* Intentionally fail so user can correct issue. */
     }
 
     if (NULL == pPhysicalDeviceCount) {
-        loader_log(
-            inst,
-            VULKAN_LOADER_FATAL_ERROR_BIT | VULKAN_LOADER_FATAL_ERROR_BIT | VULKAN_LOADER_ERROR_BIT | VULKAN_LOADER_VALIDATION_BIT,
-            0,
-            "vkEnumeratePhysicalDevices: Received NULL pointer for physical device count return value. "
-            "[VUID-vkEnumeratePhysicalDevices-pPhysicalDeviceCount-parameter]");
+        loader_log(inst, VULKAN_LOADER_FATAL_ERROR_BIT | VULKAN_LOADER_ERROR_BIT | VULKAN_LOADER_VALIDATION_BIT, 0,
+                   "vkEnumeratePhysicalDevices: Received NULL pointer for physical device count return value. "
+                   "[VUID-vkEnumeratePhysicalDevices-pPhysicalDeviceCount-parameter]");
         res = VK_ERROR_INITIALIZATION_FAILED;
         goto out;
     }
@@ -870,9 +861,8 @@ LOADER_EXPORT VKAPI_ATTR void VKAPI_CALL vkGetPhysicalDeviceFeatures(VkPhysicalD
     VkPhysicalDevice unwrapped_phys_dev = loader_unwrap_physical_device(physicalDevice);
     if (VK_NULL_HANDLE == unwrapped_phys_dev) {
         loader_log(
-            NULL,
-            VULKAN_LOADER_FATAL_ERROR_BIT | VULKAN_LOADER_FATAL_ERROR_BIT | VULKAN_LOADER_ERROR_BIT | VULKAN_LOADER_VALIDATION_BIT,
-            0, "vkGetPhysicalDeviceFeatures: Invalid physicalDevice [VUID-vkGetPhysicalDeviceFeatures-physicalDevice-parameter]");
+            NULL, VULKAN_LOADER_FATAL_ERROR_BIT | VULKAN_LOADER_ERROR_BIT | VULKAN_LOADER_VALIDATION_BIT, 0,
+            "vkGetPhysicalDeviceFeatures: Invalid physicalDevice [VUID-vkGetPhysicalDeviceFeatures-physicalDevice-parameter]");
         abort(); /* Intentionally fail so user can correct issue. */
     }
     disp = loader_get_instance_layer_dispatch(physicalDevice);

scripts/loader_extension_generator.py

@@ -780,7 +780,7 @@ def OutputLoaderDispatchTables(self):
                 tables += 'VKAPI_ATTR void VKAPI_CALL loader_init_device_dispatch_table(struct loader_dev_dispatch_table *dev_table, PFN_vkGetDeviceProcAddr gpa,\n'
                 tables += '                                                             VkDevice dev) {\n'
                 tables += '    VkLayerDispatchTable *table = &dev_table->core_dispatch;\n'
-                tables += '    assert(table->magic == DEVICE_DISP_TABLE_MAGIC_NUMBER);\n'
+                tables += '    if (table->magic != DEVICE_DISP_TABLE_MAGIC_NUMBER) { abort(); }\n'
                 tables += '    for (uint32_t i = 0; i < MAX_NUM_UNKNOWN_EXTS; i++) dev_table->ext_dispatch[i] = (PFN_vkDevExt)vkDevExtError;\n'
 
             elif x == 1:

tests/framework/layer/test_layer.cpp

@@ -179,6 +179,10 @@ VKAPI_ATTR VkResult VKAPI_CALL test_vkCreateInstance(const VkInstanceCreateInfo*
     }
     const VkInstanceCreateInfo* create_info_pointer = use_modified_create_info ? &instance_create_info : pCreateInfo;
 
+    if (layer.clobber_pInstance) {
+        memset(*pInstance, 0, 128);
+    }
+
     // Continue call down the chain
     VkResult result = fpCreateInstance(create_info_pointer, pAllocator, pInstance);
     if (result != VK_SUCCESS) {
@@ -299,6 +303,10 @@ VKAPI_ATTR VkResult VKAPI_CALL test_vkCreateDevice(VkPhysicalDevice physicalDevi
     // Advance the link info for the next element on the chain
     chain_info->u.pLayerInfo = chain_info->u.pLayerInfo->pNext;
 
+    if (layer.clobber_pDevice) {
+        memset(*pDevice, 0, 128);
+    }
+
     VkResult result = fpCreateDevice(physicalDevice, pCreateInfo, pAllocator, pDevice);
     if (result != VK_SUCCESS) {
         return result;

tests/framework/layer/test_layer.h

@@ -183,6 +183,11 @@ struct TestLayer {
 
     BUILDER_VALUE(TestLayer, bool, check_if_EnumDevExtProps_is_same_as_queried_function, false)
 
+    // Clober the data pointed to by pInstance to overwrite the magic value
+    BUILDER_VALUE(TestLayer, bool, clobber_pInstance, false)
+    // Clober the data pointed to by pDevice to overwrite the magic value
+    BUILDER_VALUE(TestLayer, bool, clobber_pDevice, false)
+
     PFN_vkGetInstanceProcAddr next_vkGetInstanceProcAddr = VK_NULL_HANDLE;
     PFN_GetPhysicalDeviceProcAddr next_GetPhysicalDeviceProcAddr = VK_NULL_HANDLE;
     PFN_vkGetDeviceProcAddr next_vkGetDeviceProcAddr = VK_NULL_HANDLE;

tests/loader_regression_tests.cpp

@@ -4069,3 +4069,80 @@ TEST(Layer, pfnNextGetInstanceProcAddr_should_not_return_layers_own_functions) {
     DeviceWrapper dev{inst};
     dev.CheckCreate(phys_devs.at(0));
 }
+
+TEST(Layer, LLP_LAYER_21) {
+    FrameworkEnvironment env{};
+    env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2)).add_physical_device({});
+
+    env.add_implicit_layer(ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{}
+                                                         .set_name("implicit_layer_name")
+                                                         .set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2)
+                                                         .set_disable_environment("DISABLE_ME")),
+                           "implicit_test_layer.json");
+    env.get_test_layer(0).set_clobber_pInstance(true);
+
+    InstWrapper inst{env.vulkan_functions};
+    FillDebugUtilsCreateDetails(inst.create_info, env.debug_log);
+#if defined(WIN32)
+#if defined(_WIN64)
+    ASSERT_DEATH(
+        inst.CheckCreate(),
+        testing::ContainsRegex(
+            R"(terminator_CreateInstance: Instance pointer \(................\) has invalid MAGIC value 0x00000000. Instance value )"
+            R"(possibly corrupted by active layer \(Policy #LLP_LAYER_21\))"));
+#else
+    ASSERT_DEATH(
+        inst.CheckCreate(),
+        testing::ContainsRegex(
+            R"(terminator_CreateInstance: Instance pointer \(........\) has invalid MAGIC value 0x00000000. Instance value )"
+            R"(possibly corrupted by active layer \(Policy #LLP_LAYER_21\))"));
+#endif
+#else
+    ASSERT_DEATH(
+        inst.CheckCreate(),
+        testing::ContainsRegex(
+            R"(terminator_CreateInstance: Instance pointer \(0x[0-9A-Fa-f]+\) has invalid MAGIC value 0x00000000. Instance value )"
+            R"(possibly corrupted by active layer \(Policy #LLP_LAYER_21\))"));
+#endif
+}
+
+TEST(Layer, LLP_LAYER_22) {
+    FrameworkEnvironment env{};
+    env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2)).add_physical_device({});
+
+    env.add_implicit_layer(ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{}
+                                                         .set_name("implicit_layer_name")
+                                                         .set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2)
+                                                         .set_disable_environment("DISABLE_ME")),
+                           "implicit_test_layer.json");
+    env.get_test_layer(0).set_clobber_pDevice(true);
+
+    InstWrapper inst{env.vulkan_functions};
+    inst.create_info.add_extension("VK_EXT_debug_utils");
+    inst.CheckCreate();
+
+    DebugUtilsWrapper log{inst};
+    CreateDebugUtilsMessenger(log);
+
+    DeviceWrapper dev{inst};
+#if defined(WIN32)
+#if defined(_WIN64)
+    ASSERT_DEATH(
+        dev.CheckCreate(inst.GetPhysDev()),
+        testing::ContainsRegex(
+            R"(terminator_CreateDevice: Device pointer \(................\) has invalid MAGIC value 0x00000000. Device value )"
+            R"(possibly corrupted by active layer \(Policy #LLP_LAYER_22\))"));
+#else
+    ASSERT_DEATH(dev.CheckCreate(inst.GetPhysDev()),
+                 testing::ContainsRegex(
+                     R"(terminator_CreateDevice: Device pointer \(........\) has invalid MAGIC value 0x00000000. Device value )"
+                     R"(possibly corrupted by active layer \(Policy #LLP_LAYER_22\))"));
+#endif
+#else
+    ASSERT_DEATH(
+        dev.CheckCreate(inst.GetPhysDev()),
+        testing::ContainsRegex(
+            R"(terminator_CreateDevice: Device pointer \(0x[0-9A-Fa-f]+\) has invalid MAGIC value 0x00000000. Device value )"
+            R"(possibly corrupted by active layer \(Policy #LLP_LAYER_22\))"));
+#endif
+}