Skip to content

Commit

Permalink
Fix problems detected by CodeQL static analysis
Browse files Browse the repository at this point in the history
3 types of issues were found:
* Incorrect format specifiers
* Not providing required format specifies
* Using alloca in a loop

There are multiple instances of alloca in loops, but to fix them would
require significant refactoring. This commit includes 1 move of alloca inside
a for loop to the outside, but this is because the logic was redoing work
in a for loop that could have been done once at the start of the function.
  • Loading branch information
charles-lunarg committed Sep 28, 2023
1 parent af32f69 commit ebf6962
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 25 deletions.
2 changes: 1 addition & 1 deletion loader/cJSON.c
Original file line number Diff line number Diff line change
Expand Up @@ -1271,7 +1271,7 @@ VkResult loader_get_json(const struct loader_instance *inst, const char *filenam
json_buf = (char *)loader_instance_heap_calloc(inst, len + 1, VK_SYSTEM_ALLOCATION_SCOPE_COMMAND);
if (json_buf == NULL) {
loader_log(inst, VULKAN_LOADER_ERROR_BIT, 0,
"loader_get_json: Failed to allocate space for JSON file %s buffer of length %d", filename, len);
"loader_get_json: Failed to allocate space for JSON file %s buffer of length %lu", filename, len);
res = VK_ERROR_OUT_OF_HOST_MEMORY;
goto out;
}
Expand Down
5 changes: 3 additions & 2 deletions loader/loader.c
Original file line number Diff line number Diff line change
Expand Up @@ -1495,7 +1495,8 @@ VkResult loader_add_direct_driver(const struct loader_instance *inst, uint32_t i
void *new_ptr = loader_instance_heap_realloc(inst, icd_tramp_list->scanned_list, icd_tramp_list->capacity,
icd_tramp_list->capacity * 2, VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE);
if (NULL == new_ptr) {
loader_log(inst, VULKAN_LOADER_ERROR_BIT, 0, "loader_add_direct_driver: Realloc failed on icd library list for ICD %s");
loader_log(inst, VULKAN_LOADER_ERROR_BIT, 0,
"loader_add_direct_driver: Realloc failed on icd library list for ICD index %u", index);
return VK_ERROR_OUT_OF_HOST_MEMORY;
}
icd_tramp_list->scanned_list = new_ptr;
Expand Down Expand Up @@ -5210,7 +5211,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateInstance(const VkInstanceCreateI
"#LLP_LAYER_21)");
} else if (LOADER_MAGIC_NUMBER != ptr_instance->magic) {
loader_log(ptr_instance, VULKAN_LOADER_WARN_BIT, 0,
"terminator_CreateInstance: Instance pointer (%p) has invalid MAGIC value 0x%08x. Instance value possibly "
"terminator_CreateInstance: Instance pointer (%p) has invalid MAGIC value 0x%08lx. Instance value possibly "
"corrupted by active layer (Policy #LLP_LAYER_21). ",
ptr_instance, ptr_instance->magic);
}
Expand Down
52 changes: 31 additions & 21 deletions loader/settings.c
Original file line number Diff line number Diff line change
Expand Up @@ -711,15 +711,35 @@ VkResult combine_settings_layers_with_regular_layers(const struct loader_instanc
}

VkResult enable_correct_layers_from_settings(const struct loader_instance* inst, const struct loader_envvar_all_filters* filters,
uint32_t name_count, const char* const* names,
uint32_t app_enabled_name_count, const char* const* app_enabled_names,
const struct loader_layer_list* instance_layers,
struct loader_pointer_layer_list* target_layer_list,
struct loader_pointer_layer_list* activated_layer_list) {
VkResult res = VK_SUCCESS;
char* vk_instance_layers_env = loader_getenv(ENABLED_LAYERS_ENV, inst);
struct loader_string_list vk_instance_layers_env_list = {0};
if (vk_instance_layers_env != NULL) {
loader_log(inst, VULKAN_LOADER_WARN_BIT | VULKAN_LOADER_LAYER_BIT, 0, "env var \'%s\' defined and adding layers \"%s\"",
ENABLED_LAYERS_ENV, names);
size_t vk_instance_layers_env_len = strlen(vk_instance_layers_env) + 1;
char* layers_env_copy = loader_stack_alloc(vk_instance_layers_env_len);
if (layers_env_copy) {
loader_strncpy(layers_env_copy, vk_instance_layers_env_len, vk_instance_layers_env, vk_instance_layers_env_len);

while (layers_env_copy && *layers_env_copy) {
char* next = loader_get_next_path(layers_env_copy);
if (strlen(layers_env_copy)) {
append_str_to_string_list(inst, &vk_instance_layers_env_list, layers_env_copy);
}
layers_env_copy = next;
}
}
loader_log(inst, VULKAN_LOADER_WARN_BIT | VULKAN_LOADER_LAYER_BIT, 0,
"env var \'%s\' defined and adding layers:", ENABLED_LAYERS_ENV);
for (uint32_t i = 0; i < vk_instance_layers_env_list.count; i++) {
if (vk_instance_layers_env_list.list && vk_instance_layers_env_list.list[i]) {
loader_log(inst, VULKAN_LOADER_WARN_BIT | VULKAN_LOADER_LAYER_BIT, 0, " \"%s\"",
vk_instance_layers_env_list.list[i]);
}
}
}
for (uint32_t i = 0; i < instance_layers->count; i++) {
bool enable_layer = false;
Expand All @@ -745,30 +765,20 @@ VkResult enable_correct_layers_from_settings(const struct loader_instance* inst,
enable_layer = true;
}

if (!enable_layer && vk_instance_layers_env) {
size_t vk_instance_layers_env_len = strlen(vk_instance_layers_env) + 1;
char* name = loader_stack_alloc(vk_instance_layers_env_len);
if (name != NULL) {
loader_strncpy(name, vk_instance_layers_env_len, vk_instance_layers_env, vk_instance_layers_env_len);
// First look for the old-fashion layers forced on with VK_INSTANCE_LAYERS
while (name && *name) {
char* next = loader_get_next_path(name);

if (strlen(name) > 0) {
if (0 == strcmp(name, props->info.layerName)) {
enable_layer = true;
break;
}
name = next;
}
if (!enable_layer && vk_instance_layers_env_list.count > 0) {
// First look for the old-fashion layers forced on with VK_INSTANCE_LAYERS
for (uint32_t j = 0; j < vk_instance_layers_env_list.count; j++) {
if (0 == strcmp(vk_instance_layers_env_list.list[j], props->info.layerName)) {
enable_layer = true;
break;
}
}
}

// Check if it should be enabled by the application
if (!enable_layer) {
for (uint32_t j = 0; j < name_count; j++) {
if (strcmp(props->info.layerName, names[j]) == 0) {
for (uint32_t j = 0; j < app_enabled_name_count; j++) {
if (strcmp(props->info.layerName, app_enabled_names[j]) == 0) {
enable_layer = true;
break;
}
Expand Down
2 changes: 1 addition & 1 deletion loader/settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ VkResult combine_settings_layers_with_regular_layers(const struct loader_instanc
// Fill out activated_layer_list with the layers that should be activated, based on environment variables, VkInstanceCreateInfo, and
// the settings
VkResult enable_correct_layers_from_settings(const struct loader_instance* inst, const struct loader_envvar_all_filters* filters,
uint32_t name_count, const char* const* names,
uint32_t app_enabled_name_count, const char* const* app_enabled_names,
const struct loader_layer_list* instance_layers,
struct loader_pointer_layer_list* target_layer_list,
struct loader_pointer_layer_list* activated_layer_list);

0 comments on commit ebf6962

Please sign in to comment.