diff --git a/.clang-tidy b/.clang-tidy index 73488b0a9..b38aa3ecc 100644 --- a/.clang-tidy +++ b/.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/.*' diff --git a/loader/allocation.h b/loader/allocation.h index 8cfb4986a..1a5c54b82 100644 --- a/loader/allocation.h +++ b/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 diff --git a/loader/asm_offset.c b/loader/asm_offset.c index d514c21c0..4a4fe1998 100644 --- a/loader/asm_offset.c +++ b/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); } diff --git a/loader/debug_utils.c b/loader/debug_utils.c index d114071ec..2ebab6914 100644 --- a/loader/debug_utils.c +++ b/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) { diff --git a/loader/extension_manual.c b/loader/extension_manual.c index 76bbace22..e96c49450 100644 --- a/loader/extension_manual.c +++ b/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 diff --git a/loader/loader.c b/loader/loader.c index ca2949835..58f680466 100644 --- a/loader/loader.c +++ b/loader/loader.c @@ -127,6 +127,7 @@ loader_api_version loader_make_full_version(uint32_t version) { return out_version; } +// NOLINTNEXTLINE(bugprone-easily-swappable-parameters) - no safer ordering for these version component fields loader_api_version loader_combine_version(uint32_t major, uint32_t minor, uint32_t patch) { loader_api_version out_version; out_version.major = (uint16_t)major; @@ -147,23 +148,21 @@ bool loader_check_version_meets_required(loader_api_version required, loader_api const char *get_enabled_by_what_str(enum loader_layer_enabled_by_what enabled_by_what) { switch (enabled_by_what) { + case ENABLED_BY_WHAT_UNSET: default: assert(true && "Shouldn't reach this"); return "Unknown"; - case (ENABLED_BY_WHAT_UNSET): - assert(true && "Shouldn't reach this"); - return "Unknown"; - case (ENABLED_BY_WHAT_LOADER_SETTINGS_FILE): + case ENABLED_BY_WHAT_LOADER_SETTINGS_FILE: return "Loader Settings File (Vulkan Configurator)"; - case (ENABLED_BY_WHAT_IMPLICIT_LAYER): + case ENABLED_BY_WHAT_IMPLICIT_LAYER: return "Implicit Layer"; - case (ENABLED_BY_WHAT_VK_INSTANCE_LAYERS): + case ENABLED_BY_WHAT_VK_INSTANCE_LAYERS: return "Environment Variable VK_INSTANCE_LAYERS"; - case (ENABLED_BY_WHAT_VK_LOADER_LAYERS_ENABLE): + case ENABLED_BY_WHAT_VK_LOADER_LAYERS_ENABLE: return "Environment Variable VK_LOADER_LAYERS_ENABLE"; - case (ENABLED_BY_WHAT_IN_APPLICATION_API): + case ENABLED_BY_WHAT_IN_APPLICATION_API: return "By the Application"; - case (ENABLED_BY_WHAT_META_LAYER): + case ENABLED_BY_WHAT_META_LAYER: return "Meta Layer (Vulkan Configurator)"; } } @@ -277,7 +276,8 @@ void loader_free_layer_properties(const struct loader_instance *inst, struct loa VkResult loader_init_library_list(struct loader_layer_list *instance_layers, loader_platform_dl_handle **libs) { if (instance_layers->count > 0) { - *libs = loader_calloc(NULL, sizeof(loader_platform_dl_handle) * instance_layers->count, VK_SYSTEM_ALLOCATION_SCOPE_COMMAND); + *libs = (loader_platform_dl_handle *)loader_calloc(NULL, sizeof(loader_platform_dl_handle) * instance_layers->count, + VK_SYSTEM_ALLOCATION_SCOPE_COMMAND); if (*libs == NULL) { return VK_ERROR_OUT_OF_HOST_MEMORY; } @@ -307,7 +307,8 @@ VkResult loader_copy_to_new_str(const struct loader_instance *inst, const char * VkResult create_string_list(const struct loader_instance *inst, uint32_t allocated_count, struct loader_string_list *string_list) { assert(string_list); - string_list->list = loader_instance_heap_calloc(inst, sizeof(char *) * allocated_count, VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE); + string_list->list = + (char **)loader_instance_heap_calloc(inst, sizeof(char *) * allocated_count, VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE); if (NULL == string_list->list) { return VK_ERROR_OUT_OF_HOST_MEMORY; } @@ -320,19 +321,19 @@ VkResult increase_str_capacity_by_at_least_one(const struct loader_instance *ins assert(string_list); if (string_list->allocated_count == 0) { string_list->allocated_count = 32; - string_list->list = - loader_instance_heap_calloc(inst, sizeof(char *) * string_list->allocated_count, VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE); + string_list->list = (char **)loader_instance_heap_calloc(inst, sizeof(char *) * string_list->allocated_count, + VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE); if (NULL == string_list->list) { return VK_ERROR_OUT_OF_HOST_MEMORY; } } else if (string_list->count + 1 > string_list->allocated_count) { uint32_t new_allocated_count = string_list->allocated_count * 2; - void *new_ptr = loader_instance_heap_realloc(inst, string_list->list, sizeof(char *) * string_list->allocated_count, + void *new_ptr = loader_instance_heap_realloc(inst, (void *)string_list->list, sizeof(char *) * string_list->allocated_count, sizeof(char *) * new_allocated_count, VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE); if (NULL == new_ptr) { return VK_ERROR_OUT_OF_HOST_MEMORY; } - string_list->list = new_ptr; + string_list->list = (char **)new_ptr; string_list->allocated_count *= 2; } return VK_SUCCESS; @@ -367,7 +368,8 @@ VkResult prepend_str_to_string_list(const struct loader_instance *inst, struct l return res; } // Shift everything down one - void *ptr_to_list = memmove(string_list->list + 1, string_list->list, sizeof(char *) * string_list->count); + void *ptr_to_list = + memmove((void *)(string_list->list + 1), (const void *)string_list->list, sizeof(char *) * string_list->count); if (ptr_to_list) string_list->list[0] = str; // Write new string to start of list string_list->count++; return VK_SUCCESS; @@ -412,7 +414,7 @@ void free_string_list(const struct loader_instance *inst, struct loader_string_l loader_instance_heap_free(inst, string_list->list[i]); string_list->list[i] = NULL; } - loader_instance_heap_free(inst, string_list->list); + loader_instance_heap_free(inst, (void *)string_list->list); } memset(string_list, 0, sizeof(struct loader_string_list)); } @@ -693,7 +695,10 @@ VkResult fixup_library_binary_path(const struct loader_instance *inst, char **li // Given string of three part form "maj.min.pat" convert to a vulkan version number. // Also can understand four part form "variant.major.minor.patch" if provided. uint32_t loader_parse_version_string(char *vers_str) { - uint32_t variant = 0, major = 0, minor = 0, patch = 0; + uint32_t variant = 0; + uint32_t major = 0; + uint32_t minor = 0; + uint32_t patch = 0; char *vers_tok; char *context = NULL; if (!vers_str) { @@ -953,7 +958,8 @@ void loader_remove_layers_not_in_implicit_meta_layers(const struct loader_instan VkResult loader_add_instance_extensions(const struct loader_instance *inst, const PFN_vkEnumerateInstanceExtensionProperties fp_get_props, const char *lib_name, struct loader_extension_list *ext_list) { - uint32_t i, count = 0; + uint32_t i; + uint32_t count = 0; VkExtensionProperties *ext_props; VkResult res = VK_SUCCESS; @@ -1017,7 +1023,8 @@ VkResult loader_add_device_extensions(const struct loader_instance *inst, PFN_vkEnumerateDeviceExtensionProperties fpEnumerateDeviceExtensionProperties, VkPhysicalDevice physical_device, const char *lib_name, struct loader_extension_list *ext_list) { - uint32_t i = 0, count = 0; + uint32_t i = 0; + uint32_t count = 0; VkResult res = VK_SUCCESS; VkExtensionProperties *ext_props = NULL; @@ -1236,7 +1243,8 @@ VkResult loader_add_to_dev_ext_list(const struct loader_instance *inst, struct l // Create storage for pointers to loader_layer_properties bool loader_init_pointer_layer_list(const struct loader_instance *inst, struct loader_pointer_layer_list *list) { list->capacity = 32 * sizeof(void *); - list->list = loader_instance_heap_calloc(inst, list->capacity, VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE); + list->list = + (struct loader_layer_properties **)loader_instance_heap_calloc(inst, list->capacity, VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE); if (list->list == NULL) { return false; } @@ -1256,7 +1264,7 @@ bool loader_names_array_has_layer_property(const VkLayerProperties *vk_layer_pro } void loader_destroy_pointer_layer_list(const struct loader_instance *inst, struct loader_pointer_layer_list *layer_list) { - loader_instance_heap_free(inst, layer_list->list); + loader_instance_heap_free(inst, (void *)layer_list->list); memset(layer_list, 0, sizeof(struct loader_pointer_layer_list)); } @@ -1272,14 +1280,14 @@ VkResult loader_add_layer_properties_to_list(const struct loader_instance *inst, // Check for enough capacity if (((list->count + 1) * sizeof(struct loader_layer_properties)) >= list->capacity) { size_t new_capacity = list->capacity * 2; - void *new_ptr = - loader_instance_heap_realloc(inst, list->list, list->capacity, new_capacity, VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE); + void *new_ptr = loader_instance_heap_realloc(inst, (void *)list->list, list->capacity, new_capacity, + VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE); if (NULL == new_ptr) { loader_log(inst, VULKAN_LOADER_ERROR_BIT, 0, "loader_add_layer_properties_to_list: Realloc failed for when attempting to add new layer"); return VK_ERROR_OUT_OF_HOST_MEMORY; } - list->list = new_ptr; + list->list = (struct loader_layer_properties **)new_ptr; list->capacity = new_capacity; } list->list[list->count++] = props; @@ -1293,7 +1301,7 @@ bool loader_layer_is_available(const struct loader_instance *inst, const struct bool available = true; bool is_implicit = (0 == (prop->type_flags & VK_LAYER_TYPE_FLAG_EXPLICIT_LAYER)); bool disabled_by_type = - (is_implicit) ? (filters->disable_filter.disable_all_implicit) : (filters->disable_filter.disable_all_explicit); + is_implicit ? filters->disable_filter.disable_all_implicit : filters->disable_filter.disable_all_explicit; if ((filters->disable_filter.disable_all || disabled_by_type || check_name_matches_filter_environment_var(prop->info.layerName, &filters->disable_filter.additional_filters)) && !check_name_matches_filter_environment_var(prop->info.layerName, &filters->allow_filter)) { @@ -1440,6 +1448,7 @@ bool loader_implicit_layer_is_enabled(const struct loader_instance *inst, const // Check the individual implicit layer for the enable/disable environment variable settings. Only add it after // every check has passed indicating it should be used, including making sure a layer of the same name hasn't already been // added. +// NOLINTNEXTLINE(misc-no-recursion) - mutual recursion with loader_add_meta_layer is bounded by finite meta-layer nesting VkResult loader_add_implicit_layer(const struct loader_instance *inst, struct loader_layer_properties *prop, const struct loader_envvar_all_filters *filters, struct loader_pointer_layer_list *target_list, struct loader_pointer_layer_list *expanded_target_list, @@ -1465,6 +1474,7 @@ VkResult loader_add_implicit_layer(const struct loader_instance *inst, struct lo } // Add the component layers of a meta-layer to the active list of layers +// NOLINTNEXTLINE(misc-no-recursion) - mutual recursion with loader_add_implicit_layer is bounded by finite meta-layer nesting VkResult loader_add_meta_layer(const struct loader_instance *inst, const struct loader_envvar_all_filters *filters, struct loader_layer_properties *prop, struct loader_pointer_layer_list *target_list, struct loader_pointer_layer_list *expanded_target_list, const struct loader_layer_list *source_list, @@ -1729,7 +1739,8 @@ void loader_add_logical_device(struct loader_icd_term *icd_term, struct loader_d void loader_remove_logical_device(struct loader_icd_term *icd_term, struct loader_device *found_dev, const VkAllocationCallbacks *pAllocator) { - struct loader_device *dev, *prev_dev; + struct loader_device *dev; + struct loader_device *prev_dev; if (!icd_term || !found_dev) return; @@ -1762,7 +1773,7 @@ void loader_icd_close_objects(struct loader_instance *ptr_inst, struct loader_ic icd_term->surface_list.list[i] && NULL != icd_term->dispatch.DestroySurfaceKHR) { icd_term->dispatch.DestroySurfaceKHR(icd_term->instance, icd_term->surface_list.list[i], ignore_null_callback(&(ptr_inst->surfaces_list.list[i].allocation_callbacks))); - icd_term->surface_list.list[i] = (VkSurfaceKHR)(uintptr_t)NULL; + icd_term->surface_list.list[i] = VK_NULL_HANDLE; } } for (uint32_t i = 0; i < icd_term->debug_utils_messenger_list.capacity / sizeof(VkDebugUtilsMessengerEXT); i++) { @@ -1772,7 +1783,7 @@ void loader_icd_close_objects(struct loader_instance *ptr_inst, struct loader_ic icd_term->dispatch.DestroyDebugUtilsMessengerEXT( icd_term->instance, icd_term->debug_utils_messenger_list.list[i], ignore_null_callback(&(ptr_inst->debug_utils_messengers_list.list[i].allocation_callbacks))); - icd_term->debug_utils_messenger_list.list[i] = (VkDebugUtilsMessengerEXT)(uintptr_t)NULL; + icd_term->debug_utils_messenger_list.list[i] = VK_NULL_HANDLE; } } for (uint32_t i = 0; i < icd_term->debug_report_callback_list.capacity / sizeof(VkDebugReportCallbackEXT); i++) { @@ -1782,7 +1793,7 @@ void loader_icd_close_objects(struct loader_instance *ptr_inst, struct loader_ic icd_term->dispatch.DestroyDebugReportCallbackEXT( icd_term->instance, icd_term->debug_report_callback_list.list[i], ignore_null_callback(&(ptr_inst->debug_report_callbacks_list.list[i].allocation_callbacks))); - icd_term->debug_report_callback_list.list[i] = (VkDebugReportCallbackEXT)(uintptr_t)NULL; + icd_term->debug_report_callback_list.list[i] = VK_NULL_HANDLE; } } } @@ -2540,6 +2551,7 @@ void loader_get_fullpath(const char *file, const char *in_dirs, size_t out_size, // Verify that all component layers in a meta-layer are valid. // This function is potentially recursive so we pass in an array of "already checked" (length of the instance_layers->count) meta // layers, preventing a stack overflow verifying meta layers that are each other's component layers +// NOLINTNEXTLINE(misc-no-recursion) - self-recursion is bounded by already_checked_meta_layers sized to instance_layers->count bool verify_meta_layer_component_layers(const struct loader_instance *inst, size_t prop_index, struct loader_layer_list *instance_layers, bool *already_checked_meta_layers) { struct loader_layer_properties *prop = &instance_layers->list[prop_index]; @@ -3309,6 +3321,7 @@ VkResult loader_add_layer_properties(const struct loader_instance *inst, struct return result; } +// NOLINTNEXTLINE(bugprone-easily-swappable-parameters) - no safer ordering for these two path string parameters VkResult copy_data_file_info(const struct loader_instance *inst, const char *cur_path, const char *relative_path, size_t relative_path_size, struct loader_string_list *search_paths) { if (NULL != cur_path) { @@ -3394,6 +3407,7 @@ VkResult prepend_if_manifest_file(const struct loader_instance *inst, const char // Iterate all strings in search_paths and add any files found. If any path in the search path points to a specific JSON, attempt // to only open that one JSON. Otherwise, if the path is a folder, search the folder for JSON files. +// NOLINTNEXTLINE(bugprone-easily-swappable-parameters) - no safer ordering for these two string-list parameters VkResult add_data_files(const struct loader_instance *inst, struct loader_string_list *search_paths, struct loader_string_list *out_files) { VkResult vk_result = VK_SUCCESS; @@ -3441,7 +3455,8 @@ VkResult add_data_files(const struct loader_instance *inst, struct loader_string // Incomplete means this was not a valid data file. if (local_res == VK_INCOMPLETE) { continue; - } else if (local_res != VK_SUCCESS) { + } + if (local_res != VK_SUCCESS) { vk_result = local_res; break; } @@ -3452,9 +3467,11 @@ VkResult add_data_files(const struct loader_instance *inst, struct loader_string } while (1) { errno = 0; + // NOLINTNEXTLINE(concurrency-mt-unsafe) - readdir is safe here since each thread uses its own dir_stream struct dirent *dir_entry = readdir(dir_stream); #if !defined(WIN32) // Windows doesn't use readdir, don't check errors on functions which aren't called if (errno != 0) { + // NOLINTNEXTLINE(concurrency-mt-unsafe) - strerror's static buffer is used immediately and not retained loader_log(inst, VULKAN_LOADER_ERROR_BIT, 0, "readdir failed with %d: %s", errno, strerror(errno)); break; } @@ -3464,6 +3481,9 @@ VkResult add_data_files(const struct loader_instance *inst, struct loader_string } name = &(dir_entry->d_name[0]); + // 'name' is the bare filename, 'cur_file' is the directory being scanned; the substring "file" in + // cur_file just confuses the naming heuristic below. + // NOLINTNEXTLINE(readability-suspicious-call-argument) - arguments are correctly ordered, not swapped loader_get_fullpath(name, cur_file, sizeof(full_path), full_path); name = full_path; @@ -3473,7 +3493,8 @@ VkResult add_data_files(const struct loader_instance *inst, struct loader_string // Incomplete means this was not a valid data file. if (local_res == VK_INCOMPLETE) { continue; - } else if (local_res != VK_SUCCESS) { + } + if (local_res != VK_SUCCESS) { vk_result = local_res; break; } @@ -4649,7 +4670,7 @@ VKAPI_ATTR PFN_vkVoidFunction VKAPI_CALL loader_gpa_device_terminator(VkDevice d return icd_term->dispatch.GetDeviceProcAddr(device, pName); } -struct loader_instance *loader_get_instance(const VkInstance instance) { +struct loader_instance *loader_get_instance(VkInstance instance) { // look up the loader_instance in our list by comparing dispatch tables, as // there is no guarantee the instance is still a loader_instance* after any // layers which wrap the instance object. @@ -4657,22 +4678,23 @@ struct loader_instance *loader_get_instance(const VkInstance instance) { struct loader_instance *ptr_instance = (struct loader_instance *)instance; if (VK_NULL_HANDLE == instance || LOADER_MAGIC_NUMBER != ptr_instance->magic) { return NULL; - } else { - disp = loader_get_instance_layer_dispatch(instance); - loader_platform_thread_lock_mutex(&loader_lock); - for (struct loader_instance *inst = loader.instances; inst; inst = inst->next) { - if (&inst->disp->layer_inst_disp == disp) { - ptr_instance = inst; - break; - } + } + disp = loader_get_instance_layer_dispatch(instance); + loader_platform_thread_lock_mutex(&loader_lock); + for (struct loader_instance *inst = loader.instances; inst; inst = inst->next) { + if (&inst->disp->layer_inst_disp == disp) { + ptr_instance = inst; + break; } - loader_platform_thread_unlock_mutex(&loader_lock); } + loader_platform_thread_unlock_mutex(&loader_lock); return ptr_instance; } loader_platform_dl_handle loader_open_layer_file(const struct loader_instance *inst, struct loader_layer_properties *prop) { - if ((prop->lib_handle = loader_platform_open_library(prop->lib_name)) == NULL) { + loader_platform_dl_handle lib_handle = loader_platform_open_library(prop->lib_name); + prop->lib_handle = lib_handle; + if (lib_handle == NULL) { loader_handle_load_library_error(inst, prop->lib_name, &prop->lib_status); } else { prop->lib_status = LOADER_LAYER_LIB_SUCCESS_LOADED; @@ -5086,7 +5108,8 @@ VkResult loader_create_instance_chain(const VkInstanceCreateInfo *pCreateInfo, c } if (!functions_in_interface) { - if ((cur_gipa = layer_prop->functions.get_instance_proc_addr) == NULL) { + cur_gipa = layer_prop->functions.get_instance_proc_addr; + if (cur_gipa == NULL) { if (layer_prop->functions.str_gipa == NULL || strlen(layer_prop->functions.str_gipa) == 0) { cur_gipa = (PFN_vkGetInstanceProcAddr)loader_platform_get_proc_address(lib_handle, "vkGetInstanceProcAddr"); @@ -5309,12 +5332,12 @@ void loader_activate_instance_layer_extensions(struct loader_instance *inst, VkI } #if defined(__APPLE__) -VkResult loader_create_device_chain(const VkPhysicalDevice pd, const VkDeviceCreateInfo *pCreateInfo, +VkResult loader_create_device_chain(VkPhysicalDevice pd, const VkDeviceCreateInfo *pCreateInfo, const VkAllocationCallbacks *pAllocator, const struct loader_instance *inst, struct loader_device *dev, PFN_vkGetInstanceProcAddr callingLayer, PFN_vkGetDeviceProcAddr *layerNextGDPA) __attribute__((optnone)) { #else -VkResult loader_create_device_chain(const VkPhysicalDevice pd, const VkDeviceCreateInfo *pCreateInfo, +VkResult loader_create_device_chain(VkPhysicalDevice pd, const VkDeviceCreateInfo *pCreateInfo, const VkAllocationCallbacks *pAllocator, const struct loader_instance *inst, struct loader_device *dev, PFN_vkGetInstanceProcAddr callingLayer, PFN_vkGetDeviceProcAddr *layerNextGDPA) { @@ -5327,19 +5350,19 @@ VkResult loader_create_device_chain(const VkPhysicalDevice pd, const VkDeviceCre VkDeviceGroupDeviceCreateInfo *original_device_group_create_info_struct = NULL; VkResult res; - PFN_vkGetDeviceProcAddr fpGDPA = NULL, nextGDPA = loader_gpa_device_terminator; - PFN_vkGetInstanceProcAddr fpGIPA = NULL, nextGIPA = loader_gpa_instance_terminator; + PFN_vkGetDeviceProcAddr fpGDPA = NULL; + PFN_vkGetDeviceProcAddr nextGDPA = loader_gpa_device_terminator; + PFN_vkGetInstanceProcAddr fpGIPA = NULL; + PFN_vkGetInstanceProcAddr nextGIPA = loader_gpa_instance_terminator; memcpy(&loader_create_info, pCreateInfo, sizeof(VkDeviceCreateInfo)); if (loader_create_info.enabledLayerCount > 0 && loader_create_info.ppEnabledLayerNames != NULL) { bool invalid_device_layer_usage = false; - if (loader_create_info.enabledLayerCount != inst->enabled_layer_names.count && loader_create_info.enabledLayerCount > 0) { - invalid_device_layer_usage = true; - } else if (loader_create_info.enabledLayerCount > 0 && loader_create_info.ppEnabledLayerNames == NULL) { - invalid_device_layer_usage = true; - } else if (loader_create_info.enabledLayerCount == 0 && loader_create_info.ppEnabledLayerNames != NULL) { + if ((loader_create_info.enabledLayerCount != inst->enabled_layer_names.count && loader_create_info.enabledLayerCount > 0) || + (loader_create_info.enabledLayerCount > 0 && loader_create_info.ppEnabledLayerNames == NULL) || + (loader_create_info.enabledLayerCount == 0 && loader_create_info.ppEnabledLayerNames != NULL)) { invalid_device_layer_usage = true; } else if (inst->enabled_layer_names.list != NULL) { for (uint32_t i = 0; i < loader_create_info.enabledLayerCount; i++) { @@ -5442,7 +5465,8 @@ VkResult loader_create_device_chain(const VkPhysicalDevice pd, const VkDeviceCre // The Get*ProcAddr pointers will already be filled in if they were received from either the json file or the // version negotiation - if ((fpGIPA = layer_prop->functions.get_instance_proc_addr) == NULL) { + fpGIPA = layer_prop->functions.get_instance_proc_addr; + if (fpGIPA == NULL) { if (layer_prop->functions.str_gipa == NULL || strlen(layer_prop->functions.str_gipa) == 0) { fpGIPA = (PFN_vkGetInstanceProcAddr)loader_platform_get_proc_address(lib_handle, "vkGetInstanceProcAddr"); layer_prop->functions.get_instance_proc_addr = fpGIPA; @@ -5467,7 +5491,8 @@ VkResult loader_create_device_chain(const VkPhysicalDevice pd, const VkDeviceCre break; } - if ((fpGDPA = layer_prop->functions.get_device_proc_addr) == NULL) { + fpGDPA = layer_prop->functions.get_device_proc_addr; + if (fpGDPA == NULL) { if (layer_prop->functions.str_gdpa == NULL || strlen(layer_prop->functions.str_gdpa) == 0) { fpGDPA = (PFN_vkGetDeviceProcAddr)loader_platform_get_proc_address(lib_handle, "vkGetDeviceProcAddr"); layer_prop->functions.get_device_proc_addr = fpGDPA; @@ -6198,13 +6223,13 @@ VKAPI_ATTR void VKAPI_CALL terminator_DestroyInstance(VkInstance instance, const for (uint32_t i = 0; i < ptr_instance->phys_dev_count_term; i++) { loader_instance_heap_free(ptr_instance, ptr_instance->phys_devs_term[i]); } - loader_instance_heap_free(ptr_instance, ptr_instance->phys_devs_term); + loader_instance_heap_free(ptr_instance, (void *)ptr_instance->phys_devs_term); } if (NULL != ptr_instance->phys_dev_groups_term) { for (uint32_t i = 0; i < ptr_instance->phys_dev_group_count_term; i++) { loader_instance_heap_free(ptr_instance, ptr_instance->phys_dev_groups_term[i]); } - loader_instance_heap_free(ptr_instance, ptr_instance->phys_dev_groups_term); + loader_instance_heap_free(ptr_instance, (void *)ptr_instance->phys_dev_groups_term); } loader_free_dev_ext_table(ptr_instance); loader_free_phys_dev_ext_table(ptr_instance); @@ -6565,8 +6590,8 @@ VkResult setup_loader_tramp_phys_devs(struct loader_instance *inst, uint32_t phy // Something is different, so do the full path of checking every device and creating a new array to use. // This can happen if a device was added, or removed, or we hadn't previously queried all the data and we // have more to store. - new_phys_devs = loader_instance_heap_calloc(inst, sizeof(struct loader_physical_device_tramp *) * new_count, - VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE); + new_phys_devs = (struct loader_physical_device_tramp **)loader_instance_heap_calloc( + inst, sizeof(struct loader_physical_device_tramp *) * new_count, VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE); if (NULL == new_phys_devs) { loader_log(inst, VULKAN_LOADER_ERROR_BIT, 0, "setup_loader_tramp_phys_devs: Failed to allocate new physical device array of size %d", new_count); @@ -6647,7 +6672,7 @@ VkResult setup_loader_tramp_phys_devs(struct loader_instance *inst, uint32_t phy loader_instance_heap_free(inst, new_phys_devs[new_idx]); } } - loader_instance_heap_free(inst, new_phys_devs); + loader_instance_heap_free(inst, (void *)new_phys_devs); } else { if (new_count > inst->total_gpu_count) { inst->total_gpu_count = new_count; @@ -6669,7 +6694,7 @@ VkResult setup_loader_tramp_phys_devs(struct loader_instance *inst, uint32_t phy loader_instance_heap_free(inst, inst->phys_devs_tramp[i]); } } - loader_instance_heap_free(inst, inst->phys_devs_tramp); + loader_instance_heap_free(inst, (void *)inst->phys_devs_tramp); } inst->phys_devs_tramp = new_phys_devs; inst->phys_dev_count_tramp = found_count; @@ -6872,8 +6897,8 @@ VkResult setup_loader_term_phys_devs(struct loader_instance *inst) { // Create an allocation large enough to hold both the windows sorting enumeration and non-windows physical device // enumeration - new_phys_devs = loader_instance_heap_calloc(inst, sizeof(struct loader_physical_device_term *) * new_phys_devs_capacity, - VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE); + new_phys_devs = (struct loader_physical_device_term **)loader_instance_heap_calloc( + inst, sizeof(struct loader_physical_device_term *) * new_phys_devs_capacity, VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE); if (NULL == new_phys_devs) { loader_log(inst, VULKAN_LOADER_ERROR_BIT, 0, "setup_loader_term_phys_devs: Failed to allocate new physical device array of size %d", new_phys_devs_capacity); @@ -6973,7 +6998,7 @@ VkResult setup_loader_term_phys_devs(struct loader_instance *inst) { loader_instance_heap_free(inst, new_phys_devs[i]); } } - loader_instance_heap_free(inst, new_phys_devs); + loader_instance_heap_free(inst, (void *)new_phys_devs); } inst->total_gpu_count = 0; } else { @@ -6994,7 +7019,7 @@ VkResult setup_loader_term_phys_devs(struct loader_instance *inst) { loader_instance_heap_free(inst, inst->phys_devs_term[i]); } } - loader_instance_heap_free(inst, inst->phys_devs_term); + loader_instance_heap_free(inst, (void *)inst->phys_devs_term); } // Swap out old and new devices list @@ -7006,7 +7031,7 @@ VkResult setup_loader_term_phys_devs(struct loader_instance *inst) { if (windows_sorted_devices_array != NULL) { for (uint32_t i = 0; i < windows_sorted_devices_count; ++i) { if (windows_sorted_devices_array[i].device_count > 0 && windows_sorted_devices_array[i].physical_devices != NULL) { - loader_instance_heap_free(inst, windows_sorted_devices_array[i].physical_devices); + loader_instance_heap_free(inst, (void *)windows_sorted_devices_array[i].physical_devices); } } loader_instance_heap_free(inst, windows_sorted_devices_array); @@ -7422,7 +7447,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_EnumerateDeviceExtensionProperties(VkP } // user is querying driver extensions and has supplied their own storage - just fill it out - else if (pProperties) { + if (pProperties) { struct loader_icd_term *icd_term = phys_dev_term->this_icd_term; uint32_t written_count = *pPropertyCount; VkResult res = @@ -7512,7 +7537,8 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_EnumerateDeviceExtensionProperties(VkP TEST_FUNCTION_EXPORT VkStringErrorFlags vk_string_validate(const int max_length, const char *utf8) { VkStringErrorFlags result = VK_STRING_ERROR_NONE; int num_char_bytes = 0; - int i, j; + int i; + int j; if (utf8 == NULL) { return VK_STRING_ERROR_NULL_PTR; @@ -7521,10 +7547,12 @@ TEST_FUNCTION_EXPORT VkStringErrorFlags vk_string_validate(const int max_length, for (i = 0; i <= max_length; i++) { if (utf8[i] == 0) { break; - } else if (i == max_length) { + } + if (i == max_length) { result |= VK_STRING_ERROR_LENGTH; break; - } else if ((utf8[i] >= 0x20) && (utf8[i] < 0x7f)) { + } + if ((utf8[i] >= 0x20) && (utf8[i] < 0x7f)) { num_char_bytes = 0; } else if ((utf8[i] & UTF8_ONE_BYTE_MASK) == UTF8_ONE_BYTE_CODE) { num_char_bytes = 1; @@ -8054,7 +8082,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_EnumeratePhysicalDeviceGroups( "which the settings file device configurations exclude, so the group was not reported.", group); local_phys_dev_groups[group].group_props.physicalDeviceCount = 0; - memset(local_phys_dev_groups[group].group_props.physicalDevices, 0, + memset((void *)local_phys_dev_groups[group].group_props.physicalDevices, 0, sizeof(local_phys_dev_groups[group].group_props.physicalDevices)); } } @@ -8095,10 +8123,9 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_EnumeratePhysicalDeviceGroups( } if (!found_all_gpus) { continue; - } else { - new_phys_dev_groups[idx] = inst->phys_dev_groups_term[old_idx]; - break; } + new_phys_dev_groups[idx] = inst->phys_dev_groups_term[old_idx]; + break; } } // If this physical device group isn't in the old buffer, create it @@ -8149,7 +8176,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_EnumeratePhysicalDeviceGroups( loader_instance_heap_free(inst, new_phys_dev_groups[i]); } } - loader_instance_heap_free(inst, new_phys_dev_groups); + loader_instance_heap_free(inst, (void *)new_phys_dev_groups); } } else { if (NULL != inst->phys_dev_groups_term) { @@ -8169,7 +8196,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_EnumeratePhysicalDeviceGroups( loader_instance_heap_free(inst, inst->phys_dev_groups_term[i]); } } - loader_instance_heap_free(inst, inst->phys_dev_groups_term); + loader_instance_heap_free(inst, (void *)inst->phys_dev_groups_term); } // Swap in the new physical device group list @@ -8180,7 +8207,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_EnumeratePhysicalDeviceGroups( if (sorted_phys_dev_array != NULL) { for (uint32_t i = 0; i < sorted_count; ++i) { if (sorted_phys_dev_array[i].device_count > 0 && sorted_phys_dev_array[i].physical_devices != NULL) { - loader_instance_heap_free(inst, sorted_phys_dev_array[i].physical_devices); + loader_instance_heap_free(inst, (void *)sorted_phys_dev_array[i].physical_devices); } } loader_instance_heap_free(inst, sorted_phys_dev_array); @@ -8210,6 +8237,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_EnumeratePhysicalDeviceGroups( } VkResult get_device_driver_id(VkPhysicalDevice physicalDevice, VkDriverId *driverId) { + // NOLINTNEXTLINE(bugprone-invalid-enum-default-initialization) - VkDriverId is an external enum with no zero value VkPhysicalDeviceDriverProperties physical_device_driver_props = {0}; physical_device_driver_props.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_DRIVER_PROPERTIES; @@ -8233,6 +8261,7 @@ VkResult get_device_driver_id(VkPhysicalDevice physicalDevice, VkDriverId *drive } if (fpGetPhysicalDeviceProperties2 == NULL) { + // NOLINTNEXTLINE(clang-analyzer-optin.core.EnumCastOutOfRange) - VkDriverId is an external enum with no zero value *driverId = 0; return VK_ERROR_UNKNOWN; } diff --git a/loader/loader.h b/loader/loader.h index 9c8fe11f9..ceb8db8cc 100644 --- a/loader/loader.h +++ b/loader/loader.h @@ -169,7 +169,6 @@ VkResult loader_add_device_extensions(const struct loader_instance *inst, PFN_vkEnumerateDeviceExtensionProperties fpEnumerateDeviceExtensionProperties, VkPhysicalDevice physical_device, const char *lib_name, struct loader_extension_list *ext_list); -VkResult loader_init_generic_list(const struct loader_instance *inst, struct loader_generic_list *list_info, size_t element_size); void loader_destroy_generic_list(const struct loader_instance *inst, struct loader_generic_list *list); void loader_destroy_pointer_layer_list(const struct loader_instance *inst, struct loader_pointer_layer_list *layer_list); TEST_FUNCTION_EXPORT void loader_delete_layer_list_and_properties(const struct loader_instance *inst, @@ -190,7 +189,7 @@ VkResult loader_scan_for_implicit_layers(struct loader_instance *inst, struct lo VkResult loader_get_icd_loader_instance_extensions(const struct loader_instance *inst, struct loader_icd_tramp_list *icd_tramp_list, struct loader_extension_list *inst_exts); struct loader_icd_term *loader_get_icd_and_device(const void *device, struct loader_device **found_dev); -struct loader_instance *loader_get_instance(const VkInstance instance); +struct loader_instance *loader_get_instance(VkInstance instance); loader_platform_dl_handle loader_open_layer_file(const struct loader_instance *inst, struct loader_layer_properties *prop); struct loader_device *loader_create_logical_device(const struct loader_instance *inst, const VkAllocationCallbacks *pAllocator); void loader_add_logical_device(struct loader_icd_term *icd_term, struct loader_device *found_dev); @@ -216,7 +215,7 @@ VKAPI_ATTR VkResult VKAPI_CALL loader_layer_create_device(VkInstance instance, V VKAPI_ATTR void VKAPI_CALL loader_layer_destroy_device(VkDevice device, const VkAllocationCallbacks *pAllocator, PFN_vkDestroyDevice destroyFunction); -VkResult loader_create_device_chain(const VkPhysicalDevice pd, const VkDeviceCreateInfo *pCreateInfo, +VkResult loader_create_device_chain(VkPhysicalDevice pd, const VkDeviceCreateInfo *pCreateInfo, const VkAllocationCallbacks *pAllocator, const struct loader_instance *inst, struct loader_device *dev, PFN_vkGetInstanceProcAddr callingLayer, PFN_vkGetDeviceProcAddr *layerNextGDPA); @@ -233,7 +232,7 @@ void unload_drivers_without_physical_devices(struct loader_instance *inst); VkResult loader_apply_settings_device_configurations(struct loader_instance *inst, uint32_t *pPhysicalDeviceCount, VkPhysicalDevice *pPhysicalDevices); -TEST_FUNCTION_EXPORT VkStringErrorFlags vk_string_validate(const int max_length, const char *char_array); +TEST_FUNCTION_EXPORT VkStringErrorFlags vk_string_validate(const int max_length, const char *utf8); char *loader_get_next_path(char *path); VkResult add_if_manifest_file(const struct loader_instance *inst, const char *file_name, struct loader_string_list *out_files); VkResult prepend_if_manifest_file(const struct loader_instance *inst, const char *file_name, struct loader_string_list *out_files); diff --git a/loader/loader_common.h b/loader/loader_common.h index e125cc364..aa1458810 100644 --- a/loader/loader_common.h +++ b/loader/loader_common.h @@ -181,6 +181,7 @@ enum loader_layer_library_status { }; enum layer_type_flags { + VK_LAYER_TYPE_FLAG_NONE = 0x0, VK_LAYER_TYPE_FLAG_INSTANCE_LAYER = 0x1, // If not set, indicates Device layer VK_LAYER_TYPE_FLAG_EXPLICIT_LAYER = 0x2, // If not set, indicates Implicit layer VK_LAYER_TYPE_FLAG_META_LAYER = 0x4, // If not set, indicates standard layer @@ -326,6 +327,7 @@ struct loader_instance_dispatch_table { #define LOADER_MAGIC_NUMBER 0x10ADED010110ADEDUL // Per instance structure +// NOLINTNEXTLINE(clang-analyzer-optin.performance.Padding): members are intentionally grouped by usage, not by size struct loader_instance { struct loader_instance_dispatch_table *disp; // must be first entry in structure uint64_t magic; // Should be LOADER_MAGIC_NUMBER diff --git a/loader/loader_environment.c b/loader/loader_environment.c index 8ad700c4a..709747149 100644 --- a/loader/loader_environment.c +++ b/loader/loader_environment.c @@ -45,6 +45,7 @@ char *loader_getenv(const char *name, const struct loader_instance *inst) { // No allocation of memory necessary for Linux, but we should at least touch // the inst pointer to get rid of compiler warnings. (void)inst; + // NOLINTNEXTLINE(concurrency-mt-unsafe) - there is no portable reentrant getenv; the loader never calls setenv concurrently return getenv(name); } @@ -79,7 +80,7 @@ char *loader_secure_getenv(const char *name, const struct loader_instance *inst) #endif } -void loader_free_getenv(char *val, const struct loader_instance *inst) { +void loader_free_getenv(const char *val, const struct loader_instance *inst) { // No freeing of memory necessary for Linux, but we should at least touch // the val and inst pointers to get rid of compiler warnings. (void)val; @@ -160,7 +161,7 @@ char *loader_secure_getenv(const char *name, const struct loader_instance *inst) return loader_getenv(name, inst); } -void loader_free_getenv(char *val, const struct loader_instance *inst) { loader_instance_heap_free(inst, (void *)val); } +void loader_free_getenv(const char *val, const struct loader_instance *inst) { loader_instance_heap_free(inst, (void *)val); } #else @@ -173,7 +174,7 @@ char *loader_getenv(const char *name, const struct loader_instance *inst) { (void)name; return NULL; } -void loader_free_getenv(char *val, const struct loader_instance *inst) { +void loader_free_getenv(const char *val, const struct loader_instance *inst) { // stub func (void)val; (void)inst; @@ -206,9 +207,8 @@ void determine_filter_type(const char *filter_string, enum loader_filter_string_ *new_start = filter_string; *new_length = filter_length; return; - } else { - star_begin = true; } + star_begin = true; } if ('*' == filter_string[filter_length - 1]) { // Not really valid, but just catch this case so if someone accidentally types "**" it will also mean everything @@ -217,9 +217,8 @@ void determine_filter_type(const char *filter_string, enum loader_filter_string_ *new_start = filter_string; *new_length = filter_length; return; - } else { - star_end = true; } + star_end = true; } if (star_begin && star_end) { *filter_type = FILTER_STRING_SUBSTRING; @@ -387,6 +386,7 @@ VkResult parse_layer_environment_var_filters(const struct loader_instance *inst, // Case-insensitive compare of `count` bytes of a name against a filter value. Filter values are already lowercased when // they get parsed (see parse_generic_filter_environment_var), so we only need to fold the name side as we go. The caller // guarantees both sides have at least `count` valid bytes. +// NOLINTNEXTLINE(bugprone-easily-swappable-parameters) - distinct, well documented roles (raw name vs. pre-lowercased filter) static bool name_segment_matches_filter_value(const char *name_segment, const char *lowercase_filter_value, size_t count) { for (size_t iii = 0; iii < count; ++iii) { if ((char)tolower((unsigned char)name_segment[iii]) != lowercase_filter_value[iii]) { @@ -527,7 +527,7 @@ VkResult loader_add_environment_layers(struct loader_instance *inst, const char bool is_implicit = (0 == (source_prop->type_flags & VK_LAYER_TYPE_FLAG_EXPLICIT_LAYER)); bool disabled_by_type = - (is_implicit) ? (filters->disable_filter.disable_all_implicit) : (filters->disable_filter.disable_all_explicit); + is_implicit ? filters->disable_filter.disable_all_implicit : filters->disable_filter.disable_all_explicit; if (!force_enabled && (filters->disable_filter.disable_all || disabled_by_type || check_name_matches_filter_environment_var(source_prop->info.layerName, &filters->disable_filter.additional_filters)) && diff --git a/loader/loader_environment.h b/loader/loader_environment.h index ddbc29f10..319e85d8b 100644 --- a/loader/loader_environment.h +++ b/loader/loader_environment.h @@ -33,7 +33,7 @@ // Common functionality for accessing environemnt variables and whether the process is running in priviledged mode char *loader_getenv(const char *name, const struct loader_instance *inst); -void loader_free_getenv(char *val, const struct loader_instance *inst); +void loader_free_getenv(const char *val, const struct loader_instance *inst); #if defined(WIN32) || COMMON_UNIX_PLATFORMS diff --git a/loader/loader_linux.c b/loader/loader_linux.c index 1d79c2e60..dc5a3026c 100644 --- a/loader/loader_linux.c +++ b/loader/loader_linux.c @@ -61,7 +61,8 @@ int32_t device_type_compare(VkPhysicalDeviceType a, VkPhysicalDeviceType b) { uint32_t b_value = determine_priority_type_value(b); if (a_value > b_value) { return -1; - } else if (b_value > a_value) { + } + if (b_value > a_value) { return 1; } return 0; @@ -73,6 +74,7 @@ int32_t device_type_compare(VkPhysicalDeviceType a, VkPhysicalDeviceType b) { // 2) Sort by type // 3) Sort by PCI bus ID // 4) Ties broken by device_ID XOR vendor_ID comparison +// NOLINTNEXTLINE(bugprone-easily-swappable-parameters) - qsort comparator signature, order is fixed by the C standard library API int32_t compare_devices(const void *a, const void *b) { struct LinuxSortedDeviceInfo *left = (struct LinuxSortedDeviceInfo *)a; struct LinuxSortedDeviceInfo *right = (struct LinuxSortedDeviceInfo *)b; @@ -80,7 +82,8 @@ int32_t compare_devices(const void *a, const void *b) { // Default device always gets priority if (left->default_device) { return -1; - } else if (right->default_device) { + } + if (right->default_device) { return 1; } @@ -93,31 +96,37 @@ int32_t compare_devices(const void *a, const void *b) { // Sort by PCI info (prioritize devices that have info over those that don't) if (left->has_pci_bus_info && !right->has_pci_bus_info) { return -1; - } else if (!left->has_pci_bus_info && right->has_pci_bus_info) { + } + if (!left->has_pci_bus_info && right->has_pci_bus_info) { return 1; - } else if (left->has_pci_bus_info && right->has_pci_bus_info) { + } + if (left->has_pci_bus_info && right->has_pci_bus_info) { // Sort low to high PCI domain if (left->pci_domain < right->pci_domain) { return -1; - } else if (left->pci_domain > right->pci_domain) { + } + if (left->pci_domain > right->pci_domain) { return 1; } // Sort low to high PCI bus if (left->pci_bus < right->pci_bus) { return -1; - } else if (left->pci_bus > right->pci_bus) { + } + if (left->pci_bus > right->pci_bus) { return 1; } // Sort low to high PCI device if (left->pci_device < right->pci_device) { return -1; - } else if (left->pci_device > right->pci_device) { + } + if (left->pci_device > right->pci_device) { return 1; } // Sort low to high PCI function if (left->pci_function < right->pci_function) { return -1; - } else if (left->pci_function > right->pci_function) { + } + if (left->pci_function > right->pci_function) { return 1; } } @@ -127,7 +136,8 @@ int32_t compare_devices(const void *a, const void *b) { uint32_t right_xord_dev_vend = right->device_id ^ right->vendor_id; if (left_xord_dev_vend < right_xord_dev_vend) { return -1; - } else if (right_xord_dev_vend < left_xord_dev_vend) { + } + if (right_xord_dev_vend < left_xord_dev_vend) { return 1; } return 0; @@ -140,6 +150,7 @@ int32_t compare_devices(const void *a, const void *b) { // 2) Group with the best device type for device 0 wins // 3) Group with best PCI bus ID for device 0 wins // 4) Ties broken by group device 0 device_ID XOR vendor_ID comparison +// NOLINTNEXTLINE(bugprone-easily-swappable-parameters) - qsort comparator signature, order is fixed by the C standard library API int32_t compare_device_groups(const void *a, const void *b) { struct loader_physical_device_group_term *grp_a = (struct loader_physical_device_group_term *)a; struct loader_physical_device_group_term *grp_b = (struct loader_physical_device_group_term *)b; @@ -151,7 +162,8 @@ int32_t compare_device_groups(const void *a, const void *b) { // Default device always gets priority if (left->default_device) { return -1; - } else if (right->default_device) { + } + if (right->default_device) { return 1; } @@ -164,31 +176,37 @@ int32_t compare_device_groups(const void *a, const void *b) { // Sort by PCI info (prioritize devices that have info over those that don't) if (left->has_pci_bus_info && !right->has_pci_bus_info) { return -1; - } else if (!left->has_pci_bus_info && right->has_pci_bus_info) { + } + if (!left->has_pci_bus_info && right->has_pci_bus_info) { return 1; - } else if (left->has_pci_bus_info && right->has_pci_bus_info) { + } + if (left->has_pci_bus_info && right->has_pci_bus_info) { // Sort low to high PCI domain if (left->pci_domain < right->pci_domain) { return -1; - } else if (left->pci_domain > right->pci_domain) { + } + if (left->pci_domain > right->pci_domain) { return 1; } // Sort low to high PCI bus if (left->pci_bus < right->pci_bus) { return -1; - } else if (left->pci_bus > right->pci_bus) { + } + if (left->pci_bus > right->pci_bus) { return 1; } // Sort low to high PCI device if (left->pci_device < right->pci_device) { return -1; - } else if (left->pci_device > right->pci_device) { + } + if (left->pci_device > right->pci_device) { return 1; } // Sort low to high PCI function if (left->pci_function < right->pci_function) { return -1; - } else if (left->pci_function > right->pci_function) { + } + if (left->pci_function > right->pci_function) { return 1; } } @@ -198,7 +216,8 @@ int32_t compare_device_groups(const void *a, const void *b) { uint32_t right_xord_dev_vend = right->device_id ^ right->vendor_id; if (left_xord_dev_vend < right_xord_dev_vend) { return -1; - } else if (right_xord_dev_vend < left_xord_dev_vend) { + } + if (right_xord_dev_vend < left_xord_dev_vend) { return 1; } return 0; @@ -214,8 +233,9 @@ void linux_env_var_default_device(struct loader_instance *inst, uint32_t device_ // The environment variable exists, so grab the vendor ID and device ID of the // selected default device - unsigned vendor_id, device_id; - // NOLINTNEXTLINE(cert-err34-c) - return value is checked below via 'matched == 2' + unsigned vendor_id; + unsigned device_id; + // NOLINTNEXTLINE(cert-err34-c, bugprone-unchecked-string-to-number-conversion) - checked below via 'matched == 2' int32_t matched = sscanf(selection, "%x:%x", &vendor_id, &device_id); if (matched == 2) { for (int32_t i = 0; i < (int32_t)device_count; ++i) { diff --git a/loader/log.c b/loader/log.c index 3441f867f..cdfb14671 100644 --- a/loader/log.c +++ b/loader/log.c @@ -40,7 +40,8 @@ uint32_t g_loader_debug = 0; void loader_init_global_debug_level(void) { - char *env, *orig; + char *env; + char *orig; if (g_loader_debug > 0) return; @@ -88,6 +89,7 @@ void loader_init_global_debug_level(void) { void loader_set_global_debug_level(uint32_t new_loader_debug) { g_loader_debug = new_loader_debug; } +// NOLINTNEXTLINE(bugprone-easily-swappable-parameters) - msg_type/cmd_line_size are convertible types, always passed in this order void generate_debug_flag_str(VkFlags msg_type, size_t cmd_line_size, char *cmd_line_msg) { cmd_line_msg[0] = '\0'; @@ -135,6 +137,7 @@ void generate_debug_flag_str(VkFlags msg_type, size_t cmd_line_size, char *cmd_l } void DECORATE_PRINTF(4, 5) + // NOLINTNEXTLINE(bugprone-easily-swappable-parameters) - msg_type/msg_code (unused) are convertible types; public API order loader_log(const struct loader_instance *inst, VkFlags msg_type, int32_t msg_code, const char *format, ...) { (void)msg_code; char msg[512] = {0}; @@ -148,11 +151,13 @@ void DECORATE_PRINTF(4, 5) va_end(ap); if (inst) { + // NOLINTNEXTLINE(clang-analyzer-optin.core.EnumCastOutOfRange) - external Vulkan enum, has no zero value we can add VkDebugUtilsMessageSeverityFlagBitsEXT severity = 0; VkDebugUtilsMessageTypeFlagsEXT type = 0; VkDebugUtilsMessengerCallbackDataEXT callback_data = {0}; VkDebugUtilsObjectNameInfoEXT object_name = {0}; + // NOLINTNEXTLINE(bugprone-branch-clone) - duplicated so combined bit calls (e.g. WARN_BIT|LAYER_BIT) match first branch if ((msg_type & VULKAN_LOADER_INFO_BIT) != 0) { severity = VK_DEBUG_UTILS_MESSAGE_SEVERITY_INFO_BIT_EXT; } else if ((msg_type & VULKAN_LOADER_WARN_BIT) != 0) { diff --git a/loader/log.h b/loader/log.h index 6075e1a0a..767d5ab5c 100644 --- a/loader/log.h +++ b/loader/log.h @@ -36,6 +36,7 @@ struct loader_instance; enum vulkan_loader_debug_flags { + VULKAN_LOADER_NONE_BIT = 0x00, VULKAN_LOADER_INFO_BIT = 0x01, VULKAN_LOADER_WARN_BIT = 0x02, VULKAN_LOADER_PERF_BIT = 0x04, diff --git a/loader/settings.c b/loader/settings.c index 35077bf24..0bce5c851 100644 --- a/loader/settings.c +++ b/loader/settings.c @@ -92,13 +92,13 @@ loader_settings_layer_control parse_control_string(char* control_string) { const char* loader_settings_layer_control_to_string(loader_settings_layer_control control) { switch (control) { - case (LOADER_SETTINGS_LAYER_CONTROL_DEFAULT): + case LOADER_SETTINGS_LAYER_CONTROL_DEFAULT: return "auto"; - case (LOADER_SETTINGS_LAYER_CONTROL_ON): + case LOADER_SETTINGS_LAYER_CONTROL_ON: return "on"; - case (LOADER_SETTINGS_LAYER_CONTROL_OFF): + case LOADER_SETTINGS_LAYER_CONTROL_OFF: return "off"; - case (LOADER_SETTINGS_LAYER_UNORDERED_LAYER_LOCATION): + case LOADER_SETTINGS_LAYER_UNORDERED_LAYER_LOCATION: return "unordered_layer_location"; default: return "UNKNOWN_LAYER_CONTROl"; @@ -804,6 +804,7 @@ VkResult get_loader_settings(const struct loader_instance* inst, loader_settings res = VK_ERROR_OUT_OF_HOST_MEMORY; goto out; } + // NOLINTNEXTLINE(clang-analyzer-optin.core.EnumCastOutOfRange) - 0xFF is a valid bitwise-OR of all flags (the "all" filter) loader_settings->debug_level = parse_log_filters_from_strings(&stderr_log); free_string_list(inst, &stderr_log); } diff --git a/loader/terminator.c b/loader/terminator.c index 466f9216f..9524bb6e9 100644 --- a/loader/terminator.c +++ b/loader/terminator.c @@ -49,20 +49,21 @@ VKAPI_ATTR void VKAPI_CALL terminator_GetPhysicalDeviceProperties(VkPhysicalDevi VKAPI_ATTR void VKAPI_CALL terminator_GetPhysicalDeviceQueueFamilyProperties(VkPhysicalDevice physicalDevice, uint32_t *pQueueFamilyPropertyCount, - VkQueueFamilyProperties *pProperties) { + VkQueueFamilyProperties *pQueueFamilyProperties) { struct loader_physical_device_term *phys_dev_term = (struct loader_physical_device_term *)physicalDevice; struct loader_icd_term *icd_term = phys_dev_term->this_icd_term; if (NULL != icd_term->dispatch.GetPhysicalDeviceQueueFamilyProperties) { - icd_term->dispatch.GetPhysicalDeviceQueueFamilyProperties(phys_dev_term->phys_dev, pQueueFamilyPropertyCount, pProperties); + icd_term->dispatch.GetPhysicalDeviceQueueFamilyProperties(phys_dev_term->phys_dev, pQueueFamilyPropertyCount, + pQueueFamilyProperties); } } VKAPI_ATTR void VKAPI_CALL terminator_GetPhysicalDeviceMemoryProperties(VkPhysicalDevice physicalDevice, - VkPhysicalDeviceMemoryProperties *pProperties) { + VkPhysicalDeviceMemoryProperties *pMemoryProperties) { struct loader_physical_device_term *phys_dev_term = (struct loader_physical_device_term *)physicalDevice; struct loader_icd_term *icd_term = phys_dev_term->this_icd_term; if (NULL != icd_term->dispatch.GetPhysicalDeviceMemoryProperties) { - icd_term->dispatch.GetPhysicalDeviceMemoryProperties(phys_dev_term->phys_dev, pProperties); + icd_term->dispatch.GetPhysicalDeviceMemoryProperties(phys_dev_term->phys_dev, pMemoryProperties); } } @@ -76,11 +77,11 @@ VKAPI_ATTR void VKAPI_CALL terminator_GetPhysicalDeviceFeatures(VkPhysicalDevice } VKAPI_ATTR void VKAPI_CALL terminator_GetPhysicalDeviceFormatProperties(VkPhysicalDevice physicalDevice, VkFormat format, - VkFormatProperties *pFormatInfo) { + VkFormatProperties *pFormatProperties) { struct loader_physical_device_term *phys_dev_term = (struct loader_physical_device_term *)physicalDevice; struct loader_icd_term *icd_term = phys_dev_term->this_icd_term; if (NULL != icd_term->dispatch.GetPhysicalDeviceFormatProperties) { - icd_term->dispatch.GetPhysicalDeviceFormatProperties(phys_dev_term->phys_dev, format, pFormatInfo); + icd_term->dispatch.GetPhysicalDeviceFormatProperties(phys_dev_term->phys_dev, format, pFormatProperties); } } @@ -103,16 +104,17 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_GetPhysicalDeviceImageFormatProperties VKAPI_ATTR void VKAPI_CALL terminator_GetPhysicalDeviceSparseImageFormatProperties(VkPhysicalDevice physicalDevice, VkFormat format, VkImageType type, VkSampleCountFlagBits samples, VkImageUsageFlags usage, VkImageTiling tiling, - uint32_t *pNumProperties, + uint32_t *pPropertyCount, VkSparseImageFormatProperties *pProperties) { struct loader_physical_device_term *phys_dev_term = (struct loader_physical_device_term *)physicalDevice; struct loader_icd_term *icd_term = phys_dev_term->this_icd_term; if (NULL != icd_term->dispatch.GetPhysicalDeviceSparseImageFormatProperties) { icd_term->dispatch.GetPhysicalDeviceSparseImageFormatProperties(phys_dev_term->phys_dev, format, type, samples, usage, - tiling, pNumProperties, pProperties); + tiling, pPropertyCount, pProperties); } } +// NOLINTNEXTLINE(readability-non-const-parameter) - matches the mandated Vulkan API signature of vkEnumerateDeviceLayerProperties VKAPI_ATTR VkResult VKAPI_CALL terminator_EnumerateDeviceLayerProperties(VkPhysicalDevice physicalDevice, uint32_t *pPropertyCount, VkLayerProperties *pProperties) { (void)pPropertyCount; @@ -308,23 +310,22 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_GetPhysicalDeviceImageFormatProperties if (fpGetPhysicalDeviceImageFormatProperties2 != NULL) { // Pass the call to the driver return fpGetPhysicalDeviceImageFormatProperties2(phys_dev_term->phys_dev, pImageFormatInfo, pImageFormatProperties); - } else { - // Emulate the call - loader_log(icd_term->this_instance, VULKAN_LOADER_INFO_BIT, 0, - "vkGetPhysicalDeviceImageFormatProperties2: Emulating call in ICD \"%s\" using " - "vkGetPhysicalDeviceImageFormatProperties", - icd_term->scanned_icd->lib_name); - - // If there is more info in either pNext, then this is unsupported - if (pImageFormatInfo->pNext != NULL || pImageFormatProperties->pNext != NULL) { - return VK_ERROR_FORMAT_NOT_SUPPORTED; - } + } + // Emulate the call + loader_log(icd_term->this_instance, VULKAN_LOADER_INFO_BIT, 0, + "vkGetPhysicalDeviceImageFormatProperties2: Emulating call in ICD \"%s\" using " + "vkGetPhysicalDeviceImageFormatProperties", + icd_term->scanned_icd->lib_name); - // Write to the VkImageFormatProperties2KHR struct - return icd_term->dispatch.GetPhysicalDeviceImageFormatProperties( - phys_dev_term->phys_dev, pImageFormatInfo->format, pImageFormatInfo->type, pImageFormatInfo->tiling, - pImageFormatInfo->usage, pImageFormatInfo->flags, &pImageFormatProperties->imageFormatProperties); + // If there is more info in either pNext, then this is unsupported + if (pImageFormatInfo->pNext != NULL || pImageFormatProperties->pNext != NULL) { + return VK_ERROR_FORMAT_NOT_SUPPORTED; } + + // Write to the VkImageFormatProperties2KHR struct + return icd_term->dispatch.GetPhysicalDeviceImageFormatProperties( + phys_dev_term->phys_dev, pImageFormatInfo->format, pImageFormatInfo->type, pImageFormatInfo->tiling, + pImageFormatInfo->usage, pImageFormatInfo->flags, &pImageFormatProperties->imageFormatProperties); } VKAPI_ATTR void VKAPI_CALL terminator_GetPhysicalDeviceQueueFamilyProperties2(VkPhysicalDevice physicalDevice, diff --git a/loader/trampoline.c b/loader/trampoline.c index ccaeba7d1..dba0f28b2 100644 --- a/loader/trampoline.c +++ b/loader/trampoline.c @@ -75,41 +75,38 @@ LOADER_EXPORT VKAPI_ATTR PFN_vkVoidFunction VKAPI_CALL vkGetInstanceProcAddr(VkI // Always can get a global entrypoint from vkGetInstanceProcAddr with a NULL instance handle if (instance == VK_NULL_HANDLE) { return addr; - } else { - // New behavior only returns a global entrypoint if the instance handle is NULL. - // Old behavior is to return a global entrypoint regardless of the value of the instance handle. - // Use new behavior if: The instance is valid and the minor version of the instance is greater than 1.2, which - // was when the new behavior was added. (eg, it is enforced in the next minor version of vulkan, which will be 1.3) - - // First check if instance is valid - loader_get_instance() returns NULL if it isn't. - struct loader_instance *ptr_instance = loader_get_instance(instance); - if (ptr_instance != NULL && - loader_check_version_meets_required(loader_combine_version(1, 3, 0), ptr_instance->app_api_version)) { - // New behavior - return NULL; - } else { - // Old behavior - return addr; - } - } - } else { - // All other functions require a valid instance handle to get - if (instance == VK_NULL_HANDLE) { - return NULL; } + // New behavior only returns a global entrypoint if the instance handle is NULL. + // Old behavior is to return a global entrypoint regardless of the value of the instance handle. + // Use new behavior if: The instance is valid and the minor version of the instance is greater than 1.2, which + // was when the new behavior was added. (eg, it is enforced in the next minor version of vulkan, which will be 1.3) + + // First check if instance is valid - loader_get_instance() returns NULL if it isn't. 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_ERROR_BIT | VULKAN_LOADER_VALIDATION_BIT, 0, - "vkGetInstanceProcAddr: Invalid instance [VUID-vkGetInstanceProcAddr-instance-parameter]"); - abort(); /* Intentionally fail so user can correct issue. */ + if (ptr_instance != NULL && + loader_check_version_meets_required(loader_combine_version(1, 3, 0), ptr_instance->app_api_version)) { + // New behavior + return NULL; } - // Return trampoline code for non-global entrypoints including any extensions. - // Device extensions are returned if a layer or ICD supports the extension. - // Instance extensions are returned if the extension is enabled and the - // loader or someone else supports the extension - return trampoline_get_proc_addr(ptr_instance, pName); + // Old behavior + return addr; } + // All other functions require a valid instance handle to get + if (instance == VK_NULL_HANDLE) { + return NULL; + } + 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_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. + // Device extensions are returned if a layer or ICD supports the extension. + // Instance extensions are returned if the extension is enabled and the + // loader or someone else supports the extension + return trampoline_get_proc_addr(ptr_instance, pName); } // Get a device level or global level entry point address. @@ -452,7 +449,7 @@ void loader_add_instance_only_debug_funcs(struct loader_instance *ptr_instance) break; } // Last item - else if (cur_node->pNext == NULL) { + if (cur_node->pNext == NULL) { cur_node->pNext = ptr_instance->instance_only_dbg_function_head; } cur_node = cur_node->pNext; @@ -857,7 +854,7 @@ LOADER_EXPORT VKAPI_ATTR void VKAPI_CALL vkDestroyInstance(VkInstance instance, for (uint32_t i = 0; i < ptr_instance->phys_dev_count_tramp; i++) { loader_instance_heap_free(ptr_instance, ptr_instance->phys_devs_tramp[i]); } - loader_instance_heap_free(ptr_instance, ptr_instance->phys_devs_tramp); + loader_instance_heap_free(ptr_instance, (void *)ptr_instance->phys_devs_tramp); } // Destroy the debug callbacks created during instance creation @@ -2774,9 +2771,8 @@ vkGetPhysicalDeviceImageFormatProperties2(VkPhysicalDevice physicalDevice, const if (inst != NULL && inst->enabled_extensions.khr_get_physical_device_properties2) { return disp->GetPhysicalDeviceImageFormatProperties2KHR(unwrapped_phys_dev, pImageFormatInfo, pImageFormatProperties); - } else { - return disp->GetPhysicalDeviceImageFormatProperties2(unwrapped_phys_dev, pImageFormatInfo, pImageFormatProperties); } + return disp->GetPhysicalDeviceImageFormatProperties2(unwrapped_phys_dev, pImageFormatInfo, pImageFormatProperties); } LOADER_EXPORT VKAPI_ATTR void VKAPI_CALL vkGetPhysicalDeviceQueueFamilyProperties2( diff --git a/loader/unknown_function_handling.c b/loader/unknown_function_handling.c index 2ff4be1d6..23e75fac8 100644 --- a/loader/unknown_function_handling.c +++ b/loader/unknown_function_handling.c @@ -143,7 +143,7 @@ bool loader_check_layer_list_for_dev_ext_address(struct loader_instance *inst, c if (inst->expanded_activated_layer_list.count > 0) { const struct loader_layer_functions *const functions = &(inst->expanded_activated_layer_list.list[0]->functions); if (NULL != functions->get_instance_proc_addr) { - return NULL != functions->get_instance_proc_addr((VkInstance)inst->instance, funcName); + return NULL != functions->get_instance_proc_addr(inst->instance, funcName); } } @@ -154,7 +154,7 @@ void loader_free_dev_ext_table(struct loader_instance *inst) { for (uint32_t i = 0; i < inst->dev_ext_disp_function_count; i++) { loader_instance_heap_free(inst, inst->dev_ext_disp_functions[i]); } - memset(inst->dev_ext_disp_functions, 0, sizeof(inst->dev_ext_disp_functions)); + memset((void *)inst->dev_ext_disp_functions, 0, sizeof(inst->dev_ext_disp_functions)); } /* @@ -244,7 +244,7 @@ bool loader_check_layer_list_for_phys_dev_ext_address(struct loader_instance *in if (layer_prop_list->interface_version > 1) { const struct loader_layer_functions *const functions = &(layer_prop_list->functions); if (NULL != functions->get_physical_device_proc_addr) { - return NULL != functions->get_physical_device_proc_addr((VkInstance)inst->instance, funcName); + return NULL != functions->get_physical_device_proc_addr(inst->instance, funcName); } } } @@ -255,7 +255,7 @@ void loader_free_phys_dev_ext_table(struct loader_instance *inst) { for (uint32_t i = 0; i < MAX_NUM_UNKNOWN_EXTS; i++) { loader_instance_heap_free(inst, inst->phys_dev_ext_disp_functions[i]); } - memset(inst->phys_dev_ext_disp_functions, 0, sizeof(inst->phys_dev_ext_disp_functions)); + memset((void *)inst->phys_dev_ext_disp_functions, 0, sizeof(inst->phys_dev_ext_disp_functions)); } // This function returns a generic trampoline or terminator function @@ -364,9 +364,8 @@ void *loader_phys_dev_ext_gpa_impl(struct loader_instance *inst, const char *fun if (is_tramp) { return loader_get_phys_dev_ext_tramp(new_function_index); - } else { - return loader_get_phys_dev_ext_termin(new_function_index); } + return loader_get_phys_dev_ext_termin(new_function_index); } // Main interface functions, makes it clear whether it is getting a terminator or trampoline void *loader_phys_dev_ext_gpa_tramp(struct loader_instance *inst, const char *funcName) { diff --git a/loader/vk_loader_platform.h b/loader/vk_loader_platform.h index 9549017fd..d642961c5 100644 --- a/loader/vk_loader_platform.h +++ b/loader/vk_loader_platform.h @@ -287,21 +287,18 @@ static inline void loader_platform_thread_win32_once_fn(INIT_ONCE *ctl, PINIT_ON // File IO static inline bool loader_platform_file_exists(const char *path) { - if (access(path, F_OK)) - return false; - else - return true; + if (access(path, F_OK)) return false; + return true; } // Returns true if the given string appears to be a relative or absolute // path, as opposed to a bare filename. static inline bool loader_platform_is_path_absolute(const char *path) { - if (path[0] == '/') - return true; - else - return false; + if (path[0] == '/') return true; + return false; } +// NOLINTNEXTLINE(concurrency-mt-unsafe) - no portable reentrant dirname; the returned buffer is copied by callers immediately static inline char *loader_platform_dirname(char *path) { return dirname(path); } // loader_platform_executable_path finds application path + name. @@ -427,6 +424,7 @@ static inline const char *loader_platform_open_library_error(const char *libPath #if defined(__Fuchsia__) return dlerror_fuchsia(); #else + // NOLINTNEXTLINE(concurrency-mt-unsafe) - dlerror has no reentrant variant; the message is used immediately, not retained return dlerror(); #endif } @@ -444,6 +442,7 @@ static inline void *loader_platform_get_proc_address(loader_platform_dl_handle l } static inline const char *loader_platform_get_proc_address_error(const char *name) { (void)name; + // NOLINTNEXTLINE(concurrency-mt-unsafe) - dlerror has no reentrant variant; the message is used immediately, not retained return dlerror(); } diff --git a/loader/wsi.c b/loader/wsi.c index cb3415712..cbd866e3d 100644 --- a/loader/wsi.c +++ b/loader/wsi.c @@ -44,6 +44,12 @@ struct loader_struct_type_info { size_t size; }; +// VK_DEFINE_NON_DISPATCHABLE_HANDLE is a real pointer on 64-bit builds and a uint64_t on 32-bit builds; +// round-tripping through uintptr_t is required to convert a loader-internal pointer into the handle type. +static inline VkSurfaceKHR wrap_surface_handle(void *icd_surface) { + return (VkSurfaceKHR)(uintptr_t)icd_surface; // NOLINT(performance-no-int-to-ptr) +} + // This function unwraps the application-provided surface handle into the ICD-specific surface handle // corresponding to the specified physical device. If the ICD-specific surface handle does not exist // yet, then this function also creates of the ICD-specific surface object. This enables lazy creation @@ -52,6 +58,7 @@ struct loader_struct_type_info { // where the VkDisplayModeKHR handles the surfaces are created from are physical-device-specific // non-dispatchable handles and therefore they should not be passed down to a foreign ICD). VkResult wsi_unwrap_icd_surface(struct loader_icd_term *icd_term, VkSurfaceKHR *surface) { + // NOLINTNEXTLINE(performance-no-int-to-ptr) - VkSurfaceKHR handle decode requires round-tripping through uintptr_t VkIcdSurface *icd_surface = (VkIcdSurface *)(uintptr_t)(*surface); #if defined(VK_USE_PLATFORM_ANDROID_KHR) @@ -309,13 +316,15 @@ VKAPI_ATTR void VKAPI_CALL terminator_DestroySurfaceKHR(VkInstance instance, VkS const VkAllocationCallbacks *pAllocator) { struct loader_instance *loader_inst = loader_get_instance(instance); - VkIcdSurface *icd_surface = (VkIcdSurface *)(uintptr_t)(surface); + // NOLINTNEXTLINE(performance-no-int-to-ptr) - VkSurfaceKHR handle decode requires round-tripping through uintptr_t + VkIcdSurface *icd_surface = (VkIcdSurface *)(uintptr_t)surface; if (NULL != icd_surface) { #if defined(VK_USE_PLATFORM_ANDROID_KHR) if (icd_surface->base.platform == VK_ICD_WSI_PLATFORM_ANDROID) { // Android surfaces are not loader-created VkIcdSurface objects: they are a smaller VkIcdSurfaceAndroid with no // surface_index or create_info, so reading those fields would run past the allocation. Just free the handle, // matching the early-out in wsi_unwrap_icd_surface. + // NOLINTNEXTLINE(performance-no-int-to-ptr) - decoding the loader-internal pointer out of the handle loader_instance_heap_free(loader_inst, (void *)(uintptr_t)surface); return; } @@ -323,6 +332,7 @@ VKAPI_ATTR void VKAPI_CALL terminator_DestroySurfaceKHR(VkInstance instance, VkS #if defined(VK_USE_PLATFORM_MACOS_MVK) if (icd_surface->base.platform == VK_ICD_WSI_PLATFORM_IOS) { // Same as Android: an iOS surface is a smaller VkIcdSurfaceIOS without a surface_index or create_info. + // NOLINTNEXTLINE(performance-no-int-to-ptr) - decoding the loader-internal pointer out of the handle loader_instance_heap_free(loader_inst, (void *)(uintptr_t)surface); return; } @@ -333,12 +343,14 @@ VKAPI_ATTR void VKAPI_CALL terminator_DestroySurfaceKHR(VkInstance instance, VkS NULL != icd_term->dispatch.DestroySurfaceKHR && icd_term->surface_list.list[icd_surface->surface_index]) { icd_term->dispatch.DestroySurfaceKHR(icd_term->instance, icd_term->surface_list.list[icd_surface->surface_index], pAllocator); + // NOLINTNEXTLINE(performance-no-int-to-ptr) - encoding the NULL non-dispatchable handle, not a real pointer icd_term->surface_list.list[icd_surface->surface_index] = (VkSurfaceKHR)(uintptr_t)NULL; } else { // The real_icd_surface for any ICD not supporting the // proper interface version should be NULL. If not, then // we have a problem. + // NOLINTNEXTLINE(performance-no-int-to-ptr) - encoding the NULL non-dispatchable handle, not a real pointer assert(!(icd_term->enabled_instance_extensions.khr_surface && icd_term->scanned_icd->interface_version >= ICD_VER_SUPPORTS_ICD_SURFACE_KHR) || (VkSurfaceKHR)(uintptr_t)NULL == icd_term->surface_list.list[icd_surface->surface_index]); @@ -348,6 +360,7 @@ VKAPI_ATTR void VKAPI_CALL terminator_DestroySurfaceKHR(VkInstance instance, VkS loader_instance_heap_free(loader_inst, icd_surface->create_info); } loader_release_object_from_list(&loader_inst->surfaces_list, icd_surface->surface_index); + // NOLINTNEXTLINE(performance-no-int-to-ptr) - decoding the loader-internal pointer out of the handle loader_instance_heap_free(loader_inst, (void *)(uintptr_t)surface); } } @@ -677,6 +690,7 @@ LOADER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkQueuePresentKHR(VkQueue queue, co return disp->QueuePresentKHR(queue, pPresentInfo); } +// NOLINTNEXTLINE(bugprone-easily-swappable-parameters) - base_size/platform_size are plain sizes with no safer distinguishing type VkResult allocate_icd_surface_struct(struct loader_instance *instance, size_t base_size, size_t platform_size, const VkAllocationCallbacks *pAllocator, VkIcdSurface **out_icd_surface) { uint32_t next_index = 0; @@ -757,7 +771,7 @@ VkResult copy_surface_create_info(struct loader_instance *loader_inst, VkIcdSurf return VK_ERROR_OUT_OF_HOST_MEMORY; } - uint8_t *dst = (uint8_t *)icd_surface->create_info; + uint8_t *dst = icd_surface->create_info; VkBaseInStructure *prev_struct = NULL; pnext = create_info; while (NULL != pnext) { @@ -865,7 +879,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateWin32SurfaceKHR(VkInstance insta goto out; } - *pSurface = (VkSurfaceKHR)(uintptr_t)icd_surface; + *pSurface = wrap_surface_handle(icd_surface); out: cleanup_surface_creation(loader_inst, result, icd_surface, pAllocator); @@ -967,7 +981,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateWaylandSurfaceKHR(VkInstance ins goto out; } - *pSurface = (VkSurfaceKHR)(uintptr_t)icd_surface; + *pSurface = wrap_surface_handle(icd_surface); out: cleanup_surface_creation(loader_inst, result, icd_surface, pAllocator); @@ -1073,7 +1087,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateXcbSurfaceKHR(VkInstance instanc goto out; } - *pSurface = (VkSurfaceKHR)(uintptr_t)icd_surface; + *pSurface = wrap_surface_handle(icd_surface); out: cleanup_surface_creation(loader_inst, result, icd_surface, pAllocator); @@ -1182,7 +1196,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateXlibSurfaceKHR(VkInstance instan goto out; } - *pSurface = (VkSurfaceKHR)(uintptr_t)icd_surface; + *pSurface = wrap_surface_handle(icd_surface); out: cleanup_surface_creation(loader_inst, result, icd_surface, pAllocator); @@ -1290,7 +1304,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateDirectFBSurfaceEXT(VkInstance in goto out; } - *pSurface = (VkSurfaceKHR)(uintptr_t)icd_surface; + *pSurface = wrap_surface_handle(icd_surface); out: cleanup_surface_creation(loader_inst, result, icd_surface, pAllocator); @@ -1384,7 +1398,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateAndroidSurfaceKHR(VkInstance ins icd_surface->base.platform = VK_ICD_WSI_PLATFORM_ANDROID; icd_surface->window = pCreateInfo->window; - *pSurface = (VkSurfaceKHR)(uintptr_t)icd_surface; + *pSurface = wrap_surface_handle(icd_surface); return VK_SUCCESS; } @@ -1441,7 +1455,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateHeadlessSurfaceEXT(VkInstance in goto out; } - *pSurface = (VkSurfaceKHR)(uintptr_t)icd_surface; + *pSurface = wrap_surface_handle(icd_surface); out: cleanup_surface_creation(loader_inst, result, icd_surface, pAllocator); @@ -1530,7 +1544,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateMacOSSurfaceMVK(VkInstance insta goto out; } - *pSurface = (VkSurfaceKHR)(uintptr_t)icd_surface; + *pSurface = wrap_surface_handle(icd_surface); out: cleanup_surface_creation(loader_inst, result, icd_surface, pAllocator); @@ -1582,7 +1596,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateIOSSurfaceMVK(VkInstance instanc icd_surface->base.platform = VK_ICD_WSI_PLATFORM_IOS; icd_surface->pView = pCreateInfo->pView; - *pSurface = (VkSurfaceKHR)(uintptr_t)icd_surface; + *pSurface = wrap_surface_handle(icd_surface); return VK_SUCCESS; } @@ -1644,7 +1658,7 @@ terminator_CreateStreamDescriptorSurfaceGGP(VkInstance instance, const VkStreamD goto out; } - *pSurface = (VkSurfaceKHR)(uintptr_t)icd_surface; + *pSurface = wrap_surface_handle(icd_surface); out: cleanup_surface_creation(loader_inst, result, icd_surface, pAllocator); @@ -1704,7 +1718,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateMetalSurfaceEXT(VkInstance insta goto out; } - *pSurface = (VkSurfaceKHR)(uintptr_t)icd_surface; + *pSurface = wrap_surface_handle(icd_surface); out: cleanup_surface_creation(loader_inst, result, icd_surface, pAllocator); @@ -1768,7 +1782,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateScreenSurfaceQNX(VkInstance inst goto out; } - *pSurface = (VkSurfaceKHR)(uintptr_t)icd_surface; + *pSurface = wrap_surface_handle(icd_surface); out: cleanup_surface_creation(loader_inst, result, icd_surface, pAllocator); @@ -1871,7 +1885,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateViSurfaceNN(VkInstance instance, goto out; } - *pSurface = (VkSurfaceKHR)(uintptr_t)icd_surface; + *pSurface = wrap_surface_handle(icd_surface); out: cleanup_surface_creation(loader_inst, result, icd_surface, pAllocator); @@ -2188,7 +2202,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateDisplayPlaneSurfaceKHR(VkInstanc goto out; } - *pSurface = (VkSurfaceKHR)(uintptr_t)icd_surface; + *pSurface = wrap_surface_handle(icd_surface); out: cleanup_surface_creation(loader_inst, result, icd_surface, pAllocator); @@ -2620,7 +2634,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateImagePipeSurfaceFUCHSIA(VkInstan goto out; } - *pSurface = (VkSurfaceKHR)(uintptr_t)icd_surface; + *pSurface = wrap_surface_handle(icd_surface); out: cleanup_surface_creation(loader_inst, result, icd_surface, pAllocator); @@ -2745,31 +2759,30 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_GetPhysicalDeviceSurfaceCapabilities2K } return res; - } else { - // Emulate the call - loader_log(icd_term->this_instance, VULKAN_LOADER_INFO_BIT, 0, - "vkGetPhysicalDeviceSurfaceCapabilities2KHR: Emulating call in ICD \"%s\" using " - "vkGetPhysicalDeviceSurfaceCapabilitiesKHR", - icd_term->scanned_icd->lib_name); - - // Write to the VkSurfaceCapabilities2KHR struct - - // If the icd doesn't support VK_KHR_surface, then there are no capabilities - if (NULL == icd_term->dispatch.GetPhysicalDeviceSurfaceCapabilitiesKHR) { - if (pSurfaceCapabilities) { - memset(&pSurfaceCapabilities->surfaceCapabilities, 0, sizeof(VkSurfaceCapabilitiesKHR)); - } - return VK_SUCCESS; - } - VkResult res = icd_term->dispatch.GetPhysicalDeviceSurfaceCapabilitiesKHR(phys_dev_term->phys_dev, surface, - &pSurfaceCapabilities->surfaceCapabilities); + } + // Emulate the call + loader_log(icd_term->this_instance, VULKAN_LOADER_INFO_BIT, 0, + "vkGetPhysicalDeviceSurfaceCapabilities2KHR: Emulating call in ICD \"%s\" using " + "vkGetPhysicalDeviceSurfaceCapabilitiesKHR", + icd_term->scanned_icd->lib_name); - if (!icd_term->enabled_instance_extensions.khr_surface_maintenance1 && - !icd_term->enabled_instance_extensions.ext_surface_maintenance1) { - emulate_VK_KHR_surface_maintenance1(pSurfaceInfo, pSurfaceCapabilities); + // Write to the VkSurfaceCapabilities2KHR struct + + // If the icd doesn't support VK_KHR_surface, then there are no capabilities + if (NULL == icd_term->dispatch.GetPhysicalDeviceSurfaceCapabilitiesKHR) { + if (pSurfaceCapabilities) { + memset(&pSurfaceCapabilities->surfaceCapabilities, 0, sizeof(VkSurfaceCapabilitiesKHR)); } - return res; + return VK_SUCCESS; } + VkResult res = icd_term->dispatch.GetPhysicalDeviceSurfaceCapabilitiesKHR(phys_dev_term->phys_dev, surface, + &pSurfaceCapabilities->surfaceCapabilities); + + if (!icd_term->enabled_instance_extensions.khr_surface_maintenance1 && + !icd_term->enabled_instance_extensions.ext_surface_maintenance1) { + emulate_VK_KHR_surface_maintenance1(pSurfaceInfo, pSurfaceCapabilities); + } + return res; } LOADER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL @@ -2816,53 +2829,50 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_GetPhysicalDeviceSurfaceFormats2KHR(Vk return icd_term->dispatch.GetPhysicalDeviceSurfaceFormats2KHR(phys_dev_term->phys_dev, &info_copy, pSurfaceFormatCount, pSurfaceFormats); - } else { - // Emulate the call - loader_log(icd_term->this_instance, VULKAN_LOADER_INFO_BIT, 0, - "vkGetPhysicalDeviceSurfaceFormats2KHR: Emulating call in ICD \"%s\" using vkGetPhysicalDeviceSurfaceFormatsKHR", - icd_term->scanned_icd->lib_name); + } + // Emulate the call + loader_log(icd_term->this_instance, VULKAN_LOADER_INFO_BIT, 0, + "vkGetPhysicalDeviceSurfaceFormats2KHR: Emulating call in ICD \"%s\" using vkGetPhysicalDeviceSurfaceFormatsKHR", + icd_term->scanned_icd->lib_name); - if (pSurfaceInfo->pNext != NULL) { - loader_log(icd_term->this_instance, VULKAN_LOADER_WARN_BIT, 0, - "vkGetPhysicalDeviceSurfaceFormats2KHR: Emulation found unrecognized structure type in pSurfaceInfo->pNext " - "- this struct will be ignored"); - } + if (pSurfaceInfo->pNext != NULL) { + loader_log(icd_term->this_instance, VULKAN_LOADER_WARN_BIT, 0, + "vkGetPhysicalDeviceSurfaceFormats2KHR: Emulation found unrecognized structure type in pSurfaceInfo->pNext " + "- this struct will be ignored"); + } - // If the icd doesn't support VK_KHR_surface, then there are no formats - if (NULL == icd_term->dispatch.GetPhysicalDeviceSurfaceFormatsKHR) { - if (pSurfaceFormatCount) { - *pSurfaceFormatCount = 0; - } - return VK_SUCCESS; + // If the icd doesn't support VK_KHR_surface, then there are no formats + if (NULL == icd_term->dispatch.GetPhysicalDeviceSurfaceFormatsKHR) { + if (pSurfaceFormatCount) { + *pSurfaceFormatCount = 0; } + return VK_SUCCESS; + } - if (*pSurfaceFormatCount == 0 || pSurfaceFormats == NULL) { - // Write to pSurfaceFormatCount - return icd_term->dispatch.GetPhysicalDeviceSurfaceFormatsKHR(phys_dev_term->phys_dev, surface, pSurfaceFormatCount, - NULL); - } else { - // Allocate a temporary array for the output of the old function - uint32_t allocated_count = *pSurfaceFormatCount; - VkSurfaceFormatKHR *formats = loader_stack_alloc(allocated_count * sizeof(VkSurfaceFormatKHR)); - if (formats == NULL) { - return VK_ERROR_OUT_OF_HOST_MEMORY; - } + if (*pSurfaceFormatCount == 0 || pSurfaceFormats == NULL) { + // Write to pSurfaceFormatCount + return icd_term->dispatch.GetPhysicalDeviceSurfaceFormatsKHR(phys_dev_term->phys_dev, surface, pSurfaceFormatCount, NULL); + } + // Allocate a temporary array for the output of the old function + uint32_t allocated_count = *pSurfaceFormatCount; + VkSurfaceFormatKHR *formats = loader_stack_alloc(allocated_count * sizeof(VkSurfaceFormatKHR)); + if (formats == NULL) { + return VK_ERROR_OUT_OF_HOST_MEMORY; + } - VkResult res = icd_term->dispatch.GetPhysicalDeviceSurfaceFormatsKHR(phys_dev_term->phys_dev, surface, - pSurfaceFormatCount, formats); - // The driver reports the written count back in pSurfaceFormatCount; never copy past the array we sized. - for (uint32_t i = 0; i < *pSurfaceFormatCount && i < allocated_count; ++i) { - pSurfaceFormats[i].surfaceFormat = formats[i]; - if (pSurfaceFormats[i].pNext != NULL) { - loader_log(icd_term->this_instance, VULKAN_LOADER_WARN_BIT, 0, - "vkGetPhysicalDeviceSurfaceFormats2KHR: Emulation found unrecognized structure type in " - "pSurfaceFormats[%d].pNext - this struct will be ignored", - i); - } - } - return res; + VkResult res = + icd_term->dispatch.GetPhysicalDeviceSurfaceFormatsKHR(phys_dev_term->phys_dev, surface, pSurfaceFormatCount, formats); + // The driver reports the written count back in pSurfaceFormatCount; never copy past the array we sized. + for (uint32_t i = 0; i < *pSurfaceFormatCount && i < allocated_count; ++i) { + pSurfaceFormats[i].surfaceFormat = formats[i]; + if (pSurfaceFormats[i].pNext != NULL) { + loader_log(icd_term->this_instance, VULKAN_LOADER_WARN_BIT, 0, + "vkGetPhysicalDeviceSurfaceFormats2KHR: Emulation found unrecognized structure type in " + "pSurfaceFormats[%d].pNext - this struct will be ignored", + i); } } + return res; } bool wsi_swapchain_instance_gpa(struct loader_instance *loader_inst, const char *name, void **addr) { diff --git a/tests/loader_envvar_tests.cpp b/tests/loader_envvar_tests.cpp index 6f9885d1a..355c52f95 100644 --- a/tests/loader_envvar_tests.cpp +++ b/tests/loader_envvar_tests.cpp @@ -196,6 +196,32 @@ TEST(EnvVarICDOverrideSetup, TestOnlyDriverEnvVarInFolderWithElevatedPrivileges) EXPECT_TRUE(env.debug_log.find("vkCreateInstance: Found no drivers!")); } +// Make sure that a loader_log() call combining a severity bit with VULKAN_LOADER_LAYER_BIT (e.g. the +// "forced enabled due to env var" message emitted by VK_LOADER_LAYERS_ENABLE) maps to that severity +// (WARNING) in the debug utils messenger callback, rather than being downgraded to the LAYER_BIT-only +// fallback severity (INFO). Filtering the logger to WARNING only means the message will only show up +// if the loader picked the correct (higher priority) branch. +TEST(EnvVarICDOverrideSetup, LayersEnableLogsAtWarningSeverity) { + FrameworkEnvironment env{}; + env.add_icd(TEST_ICD_PATH_VERSION_2_EXPORT_ICD_GPDPA).add_physical_device("physical_device_0"); + + const char* explicit_layer_name = "VK_LAYER_LUNARG_test_layer"; + env.add_explicit_layer(ManifestOptions{}.set_json_name("test_layer.json"), + ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{} + .set_name(explicit_layer_name) + .set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2) + .set_api_version(VK_MAKE_API_VERSION(0, 1, 0, 0)))); + + EnvVarWrapper layers_enable_env_var{"VK_LOADER_LAYERS_ENABLE", explicit_layer_name}; + + DebugUtilsLogger log{VK_DEBUG_UTILS_MESSAGE_SEVERITY_WARNING_BIT_EXT}; + InstWrapper inst{env.vulkan_functions}; + FillDebugUtilsCreateDetails(inst.create_info, log); + inst.CheckCreate(); + + EXPECT_TRUE(log.find_prefix_then_postfix(explicit_layer_name, "forced enabled due to env var")); +} + #if defined(__linux__) || defined(__FreeBSD__) || defined(__OpenBSD__) || defined(__GNU__) || defined(__QNX__) // Make sure the loader reports the correct message based on if LOADER_USE_UNSAFE_FILE_SEARCH is set or not TEST(EnvVarICDOverrideSetup, NonSecureEnvVarLookup) { diff --git a/tests/loader_layer_tests.cpp b/tests/loader_layer_tests.cpp index b1031499d..c1cdafef3 100644 --- a/tests/loader_layer_tests.cpp +++ b/tests/loader_layer_tests.cpp @@ -1398,6 +1398,71 @@ TEST(MetaLayers, MetaLayerWhichAddsMetaLayer) { " which also contains meta-layer " + meta_layer_name)); } +// A meta-layer cycle discovered through normal manifest scanning (VK_LAYER_PATH / standard layer discovery, +// not a settings.json file) exercises verify_meta_layer_component_layers's already_checked_meta_layers guard, +// as opposed to CyclicMetaLayerComponentDoesNotRecurse (loader_settings_tests.cpp) which exercises +// loader_add_meta_layer's is_being_expanded guard used only by the settings-file path. +TEST(MetaLayers, CyclicMetaLayerComponentsFoundThroughManifestDiscovery) { + FrameworkEnvironment env; + env.add_icd(TEST_ICD_PATH_VERSION_2_EXPORT_ICD_GPDPA); + + const char* meta_a = "VK_LAYER_meta_A"; + const char* meta_b = "VK_LAYER_meta_B"; + env.add_explicit_layer({}, ManifestLayer{}.set_file_format_version({1, 2, 0}).add_layer( + ManifestLayer::LayerDescription{}.set_name(meta_a).add_component_layer(meta_b))); + env.add_explicit_layer({}, ManifestLayer{}.set_file_format_version({1, 2, 0}).add_layer( + ManifestLayer::LayerDescription{}.set_name(meta_b).add_component_layer(meta_a))); + + InstWrapper inst{env.vulkan_functions}; + FillDebugUtilsCreateDetails(inst.create_info, env.debug_log); + // Before the already_checked_meta_layers guard, this recursion would stack overflow instead of returning. + inst.CheckCreate(); + ASSERT_TRUE(env.debug_log.find("Recursive dependency between Meta-layer")); + + // Both cyclic meta-layers are considered invalid and are dropped from the instance layer list entirely. + env.GetLayerProperties(0); +} + +// Stress test for a long, strictly acyclic chain of meta-layers: meta_0 -> meta_1 -> ... -> meta_{N-1} -> a +// real (non-meta) terminal layer. Neither loader_add_meta_layer nor verify_meta_layer_component_layers has a +// maximum-depth guard - only a cycle guard - so both recurse to a depth proportional to the chain length. This +// test does not assert a depth limit (there isn't one); it exists to make that unbounded-recursion risk area +// measurable, and to catch a regression if a future change makes the existing cycle guards ineffective (which +// would turn this bounded, acyclic chain into a crash/hang instead of a successful, if deep, CheckCreate()). +// Chain depths in the hundreds were observed to crash (stack overflow) on some CI platforms with smaller +// default thread stacks than Linux/x86_64, so a conservative depth is used here - enough to exercise several +// tens of levels of mutual recursion without depending on a platform-specific stack size. +TEST(MetaLayers, DeepAcyclicMetaLayerChainDoesNotCrash) { + FrameworkEnvironment env; + env.add_icd(TEST_ICD_PATH_VERSION_2_EXPORT_ICD_GPDPA); + + const int chain_depth = 50; + const char* real_layer_name = "VK_LAYER_TestLayer"; + env.add_explicit_layer( + {}, ManifestLayer{}.add_layer( + ManifestLayer::LayerDescription{}.set_name(real_layer_name).set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2))); + + std::vector meta_layer_names; + meta_layer_names.reserve(chain_depth); + for (int i = 0; i < chain_depth; i++) { + meta_layer_names.push_back(std::string("VK_LAYER_meta_chain_") + std::to_string(i)); + } + for (int i = 0; i < chain_depth; i++) { + const std::string& next_layer = (i + 1 < chain_depth) ? meta_layer_names[i + 1] : std::string(real_layer_name); + env.add_explicit_layer( + {}, ManifestLayer{}.set_file_format_version({1, 2, 0}).add_layer( + ManifestLayer::LayerDescription{}.set_name(meta_layer_names[i].c_str()).add_component_layer(next_layer))); + } + + InstWrapper inst{env.vulkan_functions}; + inst.create_info.add_layer(meta_layer_names[0].c_str()); + FillDebugUtilsCreateDetails(inst.create_info, env.debug_log); + inst.CheckCreate(); + + // The whole chain should have been found valid and expanded down to the real terminal layer. + env.GetLayerProperties(static_cast(chain_depth) + 1); +} + TEST(MetaLayers, InstanceExtensionInComponentLayer) { FrameworkEnvironment env; env.add_icd(TEST_ICD_PATH_VERSION_2_EXPORT_ICD_GPDPA).add_physical_device({}); @@ -3879,6 +3944,48 @@ TEST(TestLayers, EnvironLayerEnableExplicitLayer) { ASSERT_FALSE(env.debug_log.find_prefix_then_postfix(explicit_layer_name_3, "disabled because name matches filter of env var")); } +// Verify that a layer enabled through VK_INSTANCE_LAYERS reports the correct "Enabled By" reason +TEST(TestLayers, EnabledByReportsVkInstanceLayers) { + FrameworkEnvironment env; + env.add_icd(TEST_ICD_PATH_VERSION_2_EXPORT_ICD_GPDPA, {}, ManifestICD{}.set_api_version(VK_API_VERSION_1_2)) + .add_physical_device(PhysicalDevice{}); + + const char* explicit_layer_name = "VK_LAYER_LUNARG_wrap_objects"; + env.add_explicit_layer({}, ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{} + .set_name(explicit_layer_name) + .set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2) + .set_api_version(VK_MAKE_API_VERSION(0, 1, 0, 0)))); + + EnvVarWrapper instance_layers_env_var{"VK_INSTANCE_LAYERS", explicit_layer_name}; + + InstWrapper inst{env.vulkan_functions}; + FillDebugUtilsCreateDetails(inst.create_info, env.debug_log); + inst.CheckCreate(); + + ASSERT_TRUE(env.debug_log.find("Enabled By: Environment Variable VK_INSTANCE_LAYERS")); +} + +// Verify that a layer enabled through VK_LOADER_LAYERS_ENABLE reports the correct "Enabled By" reason +TEST(TestLayers, EnabledByReportsVkLoaderLayersEnable) { + FrameworkEnvironment env; + env.add_icd(TEST_ICD_PATH_VERSION_2_EXPORT_ICD_GPDPA, {}, ManifestICD{}.set_api_version(VK_API_VERSION_1_2)) + .add_physical_device(PhysicalDevice{}); + + const char* explicit_layer_name = "VK_LAYER_LUNARG_wrap_objects"; + env.add_explicit_layer({}, ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{} + .set_name(explicit_layer_name) + .set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2) + .set_api_version(VK_MAKE_API_VERSION(0, 1, 0, 0)))); + + EnvVarWrapper layers_enable_env_var{"VK_LOADER_LAYERS_ENABLE", explicit_layer_name}; + + InstWrapper inst{env.vulkan_functions}; + FillDebugUtilsCreateDetails(inst.create_info, env.debug_log); + inst.CheckCreate(); + + ASSERT_TRUE(env.debug_log.find("Enabled By: Environment Variable VK_LOADER_LAYERS_ENABLE")); +} + // Verify that VK_LOADER_LAYERS_DISABLE work. To test this, make sure that an explicit layer does not affect an instance until // it is set with VK_LOADER_LAYERS_DISABLE TEST(TestLayers, EnvironLayerDisableExplicitLayer) { diff --git a/tests/loader_settings_tests.cpp b/tests/loader_settings_tests.cpp index 85ea6a398..1f2b15567 100644 --- a/tests/loader_settings_tests.cpp +++ b/tests/loader_settings_tests.cpp @@ -116,6 +116,25 @@ TEST(SettingsFile, FileExist) { } } +// Make sure a layer enabled through the settings file reports the correct "Enabled By" reason +TEST(SettingsFile, EnabledByReportsSettingsFile) { + FrameworkEnvironment env{}; + env.add_icd(TEST_ICD_PATH_VERSION_2).add_physical_device({}); + env.loader_settings.add_app_specific_setting(AppSpecificSettings{}.add_stderr_log_filter("all")); + const char* regular_layer_name = add_layer_and_settings(env, "VK_LAYER_TestLayer_0", LayerType::exp, "on"); + env.update_loader_settings(env.loader_settings); + { + auto layer_props = env.GetLayerProperties(1); + EXPECT_TRUE(string_eq(layer_props.at(0).layerName, regular_layer_name)); + + InstWrapper inst{env.vulkan_functions}; + FillDebugUtilsCreateDetails(inst.create_info, env.debug_log); + inst.CheckCreate(); + + ASSERT_TRUE(env.debug_log.find("Enabled By: Loader Settings File (Vulkan Configurator)")); + } +} + // Make sure that if the settings file is in a user local path, that it isn't used when running with elevated privileges TEST(SettingsFile, SettingsInUnsecuredLocation) { FrameworkEnvironment env{};