loader: re-enable stricter clang-tidy checks, fix findings, add regre…

.clang-tidy

@@ -5,17 +5,20 @@ Checks: >
   bugprone-*,
   cert-*,
   clang-analyzer-*,
+  concurrency-*,
+  misc-*,
   performance-*,
   portability-*,
-  -bugprone-assignment-in-if-condition,
-  -bugprone-branch-clone,
-  -bugprone-easily-swappable-parameters,
-  -bugprone-invalid-enum-default-initialization,
-  -bugprone-multi-level-implicit-pointer-conversion,
-  -bugprone-unchecked-string-to-number-conversion,
-  -clang-analyzer-optin.core.EnumCastOutOfRange,
-  -clang-analyzer-optin.performance.Padding,
-  -performance-no-int-to-ptr,
+  readability-else-after-return,
+  readability-inconsistent-declaration-parameter-name,
+  readability-isolate-declaration,
+  readability-non-const-parameter,
+  readability-redundant-casting,
+  readability-redundant-declaration,
+  readability-redundant-parentheses,
+  readability-suspicious-call-argument,
+  -misc-include-cleaner,
+  -misc-use-internal-linkage,
   -portability-avoid-pragma-once
 WarningsAsErrors: '*'
 HeaderFilterRegex: '.*/loader/.*'

loader/allocation.h

@@ -39,11 +39,11 @@ void loader_instance_heap_free(const struct loader_instance *instance, void *pMe
 void *loader_instance_heap_realloc(const struct loader_instance *instance, void *pMemory, size_t orig_size, size_t size,
                                    VkSystemAllocationScope allocation_scope);
 
-void *loader_device_heap_alloc(const struct loader_device *device, size_t size, VkSystemAllocationScope allocationScope);
-void *loader_device_heap_calloc(const struct loader_device *device, size_t size, VkSystemAllocationScope allocationScope);
+void *loader_device_heap_alloc(const struct loader_device *device, size_t size, VkSystemAllocationScope allocation_scope);
+void *loader_device_heap_calloc(const struct loader_device *device, size_t size, VkSystemAllocationScope allocation_scope);
 void loader_device_heap_free(const struct loader_device *device, void *pMemory);
 void *loader_device_heap_realloc(const struct loader_device *device, void *pMemory, size_t orig_size, size_t size,
-                                 VkSystemAllocationScope alloc_scope);
+                                 VkSystemAllocationScope allocation_scope);
 
 // Wrappers around various memory functions. The loader will use the VkAllocationCallbacks functions if pAllocator is not NULL,
 // otherwise use the system functions

loader/asm_offset.c

@@ -170,11 +170,11 @@ int main(int argc, char **argv) {
                     values[i].comment);
         }
     }
-    // NOLINTEND(cert-err33-c)
     if (ferror(file)) {
         fclose(file);
         fprintf(stderr, "Error: failed to write gen_defines.asm\n");
         return 1;
     }
+    // NOLINTEND(cert-err33-c)
     return fclose(file);
 }

loader/debug_utils.c

@@ -135,6 +135,7 @@ VkResult util_CreateDebugUtilsMessengers(struct loader_instance *inst, const voi
         if (in_structure.sType == VK_STRUCTURE_TYPE_DEBUG_UTILS_MESSENGER_CREATE_INFO_EXT) {
             // Assign a unique handle to each messenger (just use the address of the VkDebugUtilsMessengerCreateInfoEXT)
             // This is only being used this way due to it being for an 'anonymous' callback during instance creation
+            // NOLINTNEXTLINE(performance-no-int-to-ptr) - non-dispatchable handle; round-trip via uintptr_t is required
             VkDebugUtilsMessengerEXT messenger_handle = (VkDebugUtilsMessengerEXT)(uintptr_t)pNext;
             VkResult ret = util_CreateDebugUtilsMessenger(inst, (const VkDebugUtilsMessengerCreateInfoEXT *)pNext, pAllocator,
                                                           messenger_handle);
@@ -231,6 +232,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateDebugUtilsMessengerEXT(VkInstanc
     new_dbg_func_node->pNext = inst->current_dbg_function_head;
     inst->current_dbg_function_head = new_dbg_func_node;
     *pNextIndex = next_index;
+    // NOLINTNEXTLINE(performance-no-int-to-ptr) - non-dispatchable handle; round-trip via uintptr_t is required
     *pMessenger = (VkDebugUtilsMessengerEXT)(uintptr_t)pNextIndex;
     new_dbg_func_node->messenger.messenger = *pMessenger;
 
@@ -267,6 +269,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateDebugUtilsMessengerEXT(VkInstanc
 VKAPI_ATTR void VKAPI_CALL terminator_DestroyDebugUtilsMessengerEXT(VkInstance instance, VkDebugUtilsMessengerEXT messenger,
                                                                     const VkAllocationCallbacks *pAllocator) {
     struct loader_instance *inst = (struct loader_instance *)instance;
+    // NOLINTNEXTLINE(performance-no-int-to-ptr) - non-dispatchable handle; round-trip via uintptr_t is required
     uint32_t *debug_messenger_index = (uint32_t *)(uintptr_t)messenger;
     // Make sure that messenger actually points to anything
     if (NULL == debug_messenger_index) {
@@ -418,6 +421,7 @@ VkResult util_CreateDebugReportCallbacks(struct loader_instance *inst, const voi
         if (in_structure.sType == VK_STRUCTURE_TYPE_DEBUG_REPORT_CREATE_INFO_EXT) {
             // Assign a unique handle to each callback (just use the address of the VkDebugReportCallbackCreateInfoEXT):
             // This is only being used this way due to it being for an 'anonymous' callback during instance creation
+            // NOLINTNEXTLINE(performance-no-int-to-ptr) - non-dispatchable handle; round-trip via uintptr_t is required
             VkDebugReportCallbackEXT report_handle = (VkDebugReportCallbackEXT)(uintptr_t)pNext;
             VkResult ret =
                 util_CreateDebugReportCallback(inst, (const VkDebugReportCallbackCreateInfoEXT *)pNext, pAllocator, report_handle);
@@ -515,6 +519,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateDebugReportCallbackEXT(VkInstanc
     new_dbg_func_node->pNext = inst->current_dbg_function_head;
     inst->current_dbg_function_head = new_dbg_func_node;
     *pNextIndex = next_index;
+    // NOLINTNEXTLINE(performance-no-int-to-ptr) - non-dispatchable handle; round-trip via uintptr_t is required
     *pCallback = (VkDebugReportCallbackEXT)(uintptr_t)pNextIndex;
     new_dbg_func_node->report.msgCallback = *pCallback;
 
@@ -551,6 +556,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateDebugReportCallbackEXT(VkInstanc
 VKAPI_ATTR void VKAPI_CALL terminator_DestroyDebugReportCallbackEXT(VkInstance instance, VkDebugReportCallbackEXT callback,
                                                                     const VkAllocationCallbacks *pAllocator) {
     struct loader_instance *inst = (struct loader_instance *)instance;
+    // NOLINTNEXTLINE(performance-no-int-to-ptr) - non-dispatchable handle; round-trip via uintptr_t is required
     uint32_t *debug_report_index = (uint32_t *)(uintptr_t)callback;
     // Make sure that callback actually points to anything
     if (NULL == debug_report_index) {
@@ -596,6 +602,7 @@ VKAPI_ATTR void VKAPI_CALL terminator_DebugReportMessageEXT(VkInstance instance,
     // Now that all ICDs have seen the message, call the necessary callbacks.  Ignoring "bail" return value
     // as there is nothing to bail from at this point.
 
+    // NOLINTNEXTLINE(readability-suspicious-call-argument) - objType/object correctly forward to objectType/srcObject in API order
     util_DebugReportMessage(inst, flags, objType, object, location, msgCode, pLayerPrefix, pMsg);
 
     loader_platform_thread_unlock_mutex(&loader_lock);
@@ -661,6 +668,7 @@ bool debug_utils_ReportFlagsToAnnotFlags(VkDebugReportFlagsEXT dr_flags, bool de
         return false;
     }
     *da_type = 0;
+    // NOLINTNEXTLINE(clang-analyzer-optin.core.EnumCastOutOfRange) - external Vulkan enum has no zero value we can add
     *da_severity = 0;
 
     if ((dr_flags & VK_DEBUG_REPORT_INFORMATION_BIT_EXT) != 0) {
@@ -690,6 +698,7 @@ bool debug_utils_ReportFlagsToAnnotFlags(VkDebugReportFlagsEXT dr_flags, bool de
     return true;
 }
 
+// NOLINTNEXTLINE(bugprone-easily-swappable-parameters) - mirrors VkDebugUtilsMessengerCallbackDataEXT severity/type fields
 bool debug_utils_AnnotFlagsToReportFlags(VkDebugUtilsMessageSeverityFlagBitsEXT da_severity,
                                          VkDebugUtilsMessageTypeFlagsEXT da_type, VkDebugReportFlagsEXT *dr_flags) {
     if (NULL == dr_flags) {
@@ -715,6 +724,7 @@ bool debug_utils_AnnotFlagsToReportFlags(VkDebugUtilsMessageSeverityFlagBitsEXT
     return true;
 }
 
+// NOLINTNEXTLINE(bugprone-easily-swappable-parameters) - mirrors the VkDebugReportCallbackEXT objectType/object parameters
 bool debug_utils_ReportObjectToAnnotObject(VkDebugReportObjectTypeEXT dr_object_type, uint64_t object_handle,
                                            VkDebugUtilsObjectNameInfoEXT *da_object_name_info) {
     if (NULL == da_object_name_info) {

loader/extension_manual.c

@@ -113,39 +113,38 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_GetPhysicalDeviceSurfaceCapabilities2E
     if (NULL != icd_term->dispatch.GetPhysicalDeviceSurfaceCapabilities2EXT) {
         // Pass the call to the driver
         return icd_term->dispatch.GetPhysicalDeviceSurfaceCapabilities2EXT(phys_dev_term->phys_dev, surface, pSurfaceCapabilities);
-    } else {
-        // Emulate the call
-        loader_log(icd_term->this_instance, VULKAN_LOADER_INFO_BIT, 0,
-                   "vkGetPhysicalDeviceSurfaceCapabilities2EXT: Emulating call in ICD \"%s\" using "
-                   "vkGetPhysicalDeviceSurfaceCapabilitiesKHR",
-                   icd_term->scanned_icd->lib_name);
-
-        VkSurfaceCapabilitiesKHR surface_caps;
-        res = icd_term->dispatch.GetPhysicalDeviceSurfaceCapabilitiesKHR(phys_dev_term->phys_dev, surface, &surface_caps);
-        if (res != VK_SUCCESS) {
-            // The driver did not populate surface_caps, so don't copy it out to the caller.
-            return res;
-        }
-        pSurfaceCapabilities->minImageCount = surface_caps.minImageCount;
-        pSurfaceCapabilities->maxImageCount = surface_caps.maxImageCount;
-        pSurfaceCapabilities->currentExtent = surface_caps.currentExtent;
-        pSurfaceCapabilities->minImageExtent = surface_caps.minImageExtent;
-        pSurfaceCapabilities->maxImageExtent = surface_caps.maxImageExtent;
-        pSurfaceCapabilities->maxImageArrayLayers = surface_caps.maxImageArrayLayers;
-        pSurfaceCapabilities->supportedTransforms = surface_caps.supportedTransforms;
-        pSurfaceCapabilities->currentTransform = surface_caps.currentTransform;
-        pSurfaceCapabilities->supportedCompositeAlpha = surface_caps.supportedCompositeAlpha;
-        pSurfaceCapabilities->supportedUsageFlags = surface_caps.supportedUsageFlags;
-        pSurfaceCapabilities->supportedSurfaceCounters = 0;
-
-        if (pSurfaceCapabilities->pNext != NULL) {
-            loader_log(icd_term->this_instance, VULKAN_LOADER_WARN_BIT, 0,
-                       "vkGetPhysicalDeviceSurfaceCapabilities2EXT: Emulation found unrecognized structure type in "
-                       "pSurfaceCapabilities->pNext - this struct will be ignored");
-        }
-
+    }
+    // Emulate the call
+    loader_log(icd_term->this_instance, VULKAN_LOADER_INFO_BIT, 0,
+               "vkGetPhysicalDeviceSurfaceCapabilities2EXT: Emulating call in ICD \"%s\" using "
+               "vkGetPhysicalDeviceSurfaceCapabilitiesKHR",
+               icd_term->scanned_icd->lib_name);
+
+    VkSurfaceCapabilitiesKHR surface_caps;
+    res = icd_term->dispatch.GetPhysicalDeviceSurfaceCapabilitiesKHR(phys_dev_term->phys_dev, surface, &surface_caps);
+    if (res != VK_SUCCESS) {
+        // The driver did not populate surface_caps, so don't copy it out to the caller.
         return res;
     }
+    pSurfaceCapabilities->minImageCount = surface_caps.minImageCount;
+    pSurfaceCapabilities->maxImageCount = surface_caps.maxImageCount;
+    pSurfaceCapabilities->currentExtent = surface_caps.currentExtent;
+    pSurfaceCapabilities->minImageExtent = surface_caps.minImageExtent;
+    pSurfaceCapabilities->maxImageExtent = surface_caps.maxImageExtent;
+    pSurfaceCapabilities->maxImageArrayLayers = surface_caps.maxImageArrayLayers;
+    pSurfaceCapabilities->supportedTransforms = surface_caps.supportedTransforms;
+    pSurfaceCapabilities->currentTransform = surface_caps.currentTransform;
+    pSurfaceCapabilities->supportedCompositeAlpha = surface_caps.supportedCompositeAlpha;
+    pSurfaceCapabilities->supportedUsageFlags = surface_caps.supportedUsageFlags;
+    pSurfaceCapabilities->supportedSurfaceCounters = 0;
+
+    if (pSurfaceCapabilities->pNext != NULL) {
+        loader_log(icd_term->this_instance, VULKAN_LOADER_WARN_BIT, 0,
+                   "vkGetPhysicalDeviceSurfaceCapabilities2EXT: Emulation found unrecognized structure type in "
+                   "pSurfaceCapabilities->pNext - this struct will be ignored");
+    }
+
+    return res;
 }
 
 // ---- VK_EXT_direct_mode_display extension trampoline/terminators
@@ -199,14 +198,13 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_AcquireXlibDisplayEXT(VkPhysicalDevice
     if (icd_term->dispatch.AcquireXlibDisplayEXT != NULL) {
         // Pass the call to the driver
         return icd_term->dispatch.AcquireXlibDisplayEXT(phys_dev_term->phys_dev, dpy, display);
-    } else {
-        // Emulate the call
-        loader_log(icd_term->this_instance, VULKAN_LOADER_INFO_BIT, 0,
-                   "vkAcquireXLibDisplayEXT: Emulating call in ICD \"%s\" by returning error", icd_term->scanned_icd->lib_name);
-
-        // Fail for the unsupported command
-        return VK_ERROR_INITIALIZATION_FAILED;
     }
+    // Emulate the call
+    loader_log(icd_term->this_instance, VULKAN_LOADER_INFO_BIT, 0,
+               "vkAcquireXLibDisplayEXT: Emulating call in ICD \"%s\" by returning error", icd_term->scanned_icd->lib_name);
+
+    // Fail for the unsupported command
+    return VK_ERROR_INITIALIZATION_FAILED;
 }
 
 VKAPI_ATTR VkResult VKAPI_CALL GetRandROutputDisplayEXT(VkPhysicalDevice physicalDevice, Display *dpy, RROutput rrOutput,
@@ -230,16 +228,15 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_GetRandROutputDisplayEXT(VkPhysicalDev
     if (icd_term->dispatch.GetRandROutputDisplayEXT != NULL) {
         // Pass the call to the driver
         return icd_term->dispatch.GetRandROutputDisplayEXT(phys_dev_term->phys_dev, dpy, rrOutput, pDisplay);
-    } else {
-        // Emulate the call
-        loader_log(icd_term->this_instance, VULKAN_LOADER_INFO_BIT, 0,
-                   "vkGetRandROutputDisplayEXT: Emulating call in ICD \"%s\" by returning null display",
-                   icd_term->scanned_icd->lib_name);
-
-        // Return a null handle to indicate this can't be done
-        *pDisplay = VK_NULL_HANDLE;
-        return VK_SUCCESS;
     }
+    // Emulate the call
+    loader_log(icd_term->this_instance, VULKAN_LOADER_INFO_BIT, 0,
+               "vkGetRandROutputDisplayEXT: Emulating call in ICD \"%s\" by returning null display",
+               icd_term->scanned_icd->lib_name);
+
+    // Return a null handle to indicate this can't be done
+    *pDisplay = VK_NULL_HANDLE;
+    return VK_SUCCESS;
 }
 
 #endif  // VK_USE_PLATFORM_XLIB_XRANDR_EXT