Add CRT Warnings & fix them

Fixes the many uses of strncpy and strncat to use versions of the functions
which the CRT approves of. This means adding the length of the destination
string as a parameter.

The function loader_platform_combine_path was removed and the single use of
it was replaced with a call to snprinf, which achieves the same goal.

There are a few other functions that the CRT warns about, like fopen and
the wide char version of strcpy. These were also replaced with the 'safe'
functions.

loader/CMakeLists.txt

@@ -30,7 +30,6 @@ if(WIN32)
         set(CMAKE_C_STANDARD_LIBRARIES ${CMAKE_CXX_STANDARD_LIBRARIES})
     endif()
 
-    target_compile_options(loader_specific_options INTERFACE -D_CRT_SECURE_NO_WARNINGS)
     # ~~~
     # Build dev_ext_trampoline.c and unknown_ext_chain.c with /O2 to allow tail-call optimization.
     # Setup two CMake targets (loader-norm and loader-opt) for the different compilation flags.

loader/asm_offset.c

@@ -114,7 +114,7 @@ int main(int argc, char **argv) {
         // clang-format on
     };
 
-    FILE *file = fopen("gen_defines.asm", "w");
+    FILE *file = loader_fopen("gen_defines.asm", "w");
     fprintf(file, "\n");
     if (!strcmp(assembler, "MASM")) {
         for (size_t i = 0; i < sizeof(values) / sizeof(values[0]); ++i) {

loader/cJSON.c

@@ -41,11 +41,11 @@
 #include "log.h"
 
 void *cJSON_malloc(const VkAllocationCallbacks *pAllocator, size_t size) {
-    return loader_alloc(pAllocator, size, VK_SYSTEM_ALLOCATION_SCOPE_COMMAND);
+    return loader_calloc(pAllocator, size, VK_SYSTEM_ALLOCATION_SCOPE_COMMAND);
 }
 
 void *cJSON_malloc_instance_scope(const VkAllocationCallbacks *pAllocator, size_t size) {
-    return loader_alloc(pAllocator, size, VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE);
+    return loader_calloc(pAllocator, size, VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE);
 }
 
 void cJSON_Free(const VkAllocationCallbacks *pAllocator, void *pMemory) { loader_free(pAllocator, pMemory); }
@@ -182,7 +182,7 @@ char *print_number(cJSON *item, printbuffer *p) {
             str = ensure(item->pAllocator, p, str_buf_size);
         else
             str = (char *)cJSON_malloc(item->pAllocator, str_buf_size);
-        if (str) strncpy(str, "0", str_buf_size);
+        if (str) loader_strncpy(str, str_buf_size, "0", 2);
     } else if (fabs(((double)item->valueint) - d) <= DBL_EPSILON && d <= INT_MAX && d >= INT_MIN) {
         str_buf_size = 21; /* 2^64+1 can be represented in 21 chars. */
         if (p)
@@ -361,7 +361,7 @@ char *print_string_ptr(const VkAllocationCallbacks *pAllocator, const char *str,
         if (!out) return 0;
         ptr2 = out;
         // *ptr2++ = '\"'; // Modified to not put quotes around the string
-        strncpy(ptr2, str, out_buf_size);
+        loader_strncpy(ptr2, out_buf_size, str, out_buf_size);
         // ptr2[len] = '\"'; // Modified to not put quotes around the string
         ptr2[len] = 0;  // ptr2[len + 1] = 0; // Modified to not put quotes around the string
         return out;
@@ -374,7 +374,7 @@ char *print_string_ptr(const VkAllocationCallbacks *pAllocator, const char *str,
         else
             out = (char *)cJSON_malloc_instance_scope(pAllocator, out_buf_size);
         if (!out) return 0;
-        strncpy(out, "\"\"", out_buf_size);
+        loader_strncpy(out, out_buf_size, "\"\"", 3);
         return out;
     }
     ptr = str;
@@ -538,17 +538,17 @@ char *print_value(cJSON *item, int depth, int fmt, printbuffer *p) {
         switch ((item->type) & 255) {
             case cJSON_NULL: {
                 out = ensure(item->pAllocator, p, 5);
-                if (out) strncpy(out, "null", 5);
+                if (out) loader_strncpy(out, 5, "null", 5);
                 break;
             }
             case cJSON_False: {
                 out = ensure(item->pAllocator, p, 6);
-                if (out) strncpy(out, "false", 6);
+                if (out) loader_strncpy(out, 6, "false", 6);
                 break;
             }
             case cJSON_True: {
                 out = ensure(item->pAllocator, p, 5);
-                if (out) strncpy(out, "true", 5);
+                if (out) loader_strncpy(out, 5, "true", 5);
                 break;
             }
             case cJSON_Number:
@@ -642,7 +642,7 @@ char *print_array(cJSON *item, int depth, int fmt, printbuffer *p) {
             out = ensure(item->pAllocator, p, 3);
         else
             out = (char *)cJSON_malloc(item->pAllocator, 3);
-        if (out) strncpy(out, "[]", 3);
+        if (out) loader_strncpy(out, 3, "[]", 3);
         return out;
     }
 
@@ -903,8 +903,9 @@ char *print_object(cJSON *item, int depth, int fmt, printbuffer *p) {
             ptr += tmplen;
             *ptr++ = ':';
             if (fmt) *ptr++ = '\t';
-            strcpy(ptr, entries[j]);
-            ptr += strlen(entries[j]);
+            size_t entries_size = strlen(entries[j]);
+            loader_strncpy(ptr, len - (ptr - out), entries[j], entries_size);
+            ptr += entries_size;
             if (j != numentries - 1) *ptr++ = ',';
             if (fmt) *ptr++ = '\n';
             *ptr = 0;
@@ -1242,7 +1243,10 @@ VkResult loader_get_json(const struct loader_instance *inst, const char *filenam
     if (filename_utf16_size > 0) {
         wchar_t *filename_utf16 = (wchar_t *)loader_stack_alloc(filename_utf16_size * sizeof(wchar_t));
         if (MultiByteToWideChar(CP_UTF8, 0, filename, -1, filename_utf16, filename_utf16_size) == filename_utf16_size) {
-            file = _wfopen(filename_utf16, L"rb");
+            errno_t wfopen_error = _wfopen_s(&file, filename_utf16, L"rb");
+            if (0 != wfopen_error) {
+                loader_log(inst, VULKAN_LOADER_ERROR_BIT, 0, "loader_get_json: Failed to open JSON file %s", filename);
+            }
         }
     }
 #elif COMMON_UNIX_PLATFORMS
@@ -1317,7 +1321,7 @@ VkResult loader_parse_json_string_to_existing_str(const struct loader_instance *
         return VK_ERROR_OUT_OF_HOST_MEMORY;
     }
     if (NULL != out_string) {
-        strncpy(out_string, str, out_str_len);
+        loader_strncpy(out_string, out_str_len, str, out_str_len);
         if (out_str_len > 0) {
             out_string[out_str_len - 1] = '\0';
         }

loader/dirent_on_windows.c

@@ -36,11 +36,12 @@ DIR *opendir(const VkAllocationCallbacks *pAllocator, const char *name) {
         size_t base_length = strlen(name);
         const char *all = /* search pattern must end with suitable wildcard */
             strchr("/\\", name[base_length - 1]) ? "*" : "/*";
+        size_t full_length = base_length + strlen(all) + 1;
 
         if ((dir = (DIR *)loader_alloc(pAllocator, sizeof *dir, VK_SYSTEM_ALLOCATION_SCOPE_COMMAND)) != 0 &&
-            (dir->name = (char *)loader_alloc(pAllocator, base_length + strlen(all) + 1, VK_SYSTEM_ALLOCATION_SCOPE_COMMAND)) !=
-                0) {
-            strcat(strcpy(dir->name, name), all);
+            (dir->name = (char *)loader_calloc(pAllocator, full_length, VK_SYSTEM_ALLOCATION_SCOPE_COMMAND)) != 0) {
+            loader_strncpy(dir->name, full_length, name, base_length);
+            loader_strncat(dir->name, full_length, all, strlen(all));
 
             if ((dir->handle = (handle_type)_findfirst(dir->name, &dir->info)) != -1) {
                 dir->result.d_name = 0;

loader/loader.c

@@ -257,11 +257,11 @@ VkResult loader_init_library_list(struct loader_layer_list *instance_layers, loa
 
 VkResult loader_copy_to_new_str(const struct loader_instance *inst, const char *source_str, char **dest_str) {
     assert(source_str && dest_str);
-    size_t str_len = strlen(source_str);
-    *dest_str = loader_instance_heap_calloc(inst, str_len + 1, VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE);
+    size_t str_len = strlen(source_str) + 1;
+    *dest_str = loader_instance_heap_calloc(inst, str_len, VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE);
     if (NULL == *dest_str) return VK_ERROR_OUT_OF_HOST_MEMORY;
-    strncpy(*dest_str, source_str, str_len);
-    (*dest_str)[str_len] = 0;
+    loader_strncpy(*dest_str, str_len, source_str, str_len);
+    (*dest_str)[str_len - 1] = 0;
     return VK_SUCCESS;
 }
 
@@ -307,7 +307,7 @@ VkResult copy_str_to_string_list(const struct loader_instance *inst, struct load
     if (NULL == new_str) {
         return VK_ERROR_OUT_OF_HOST_MEMORY;
     }
-    strncpy(new_str, str, str_len);
+    loader_strncpy(new_str, sizeof(char *) * str_len + 1, str, str_len);
     new_str[str_len] = '\0';
     VkResult res = append_str_to_string_list(inst, string_list, new_str);
     if (res != VK_SUCCESS) {
@@ -331,51 +331,6 @@ void free_string_list(const struct loader_instance *inst, struct loader_string_l
     string_list->allocated_count = 0;
 }
 
-// Combine path elements, separating each element with the platform-specific
-// directory separator, and save the combined string to a destination buffer,
-// not exceeding the given length. Path elements are given as variable args,
-// with a NULL element terminating the list.
-//
-// \returns the total length of the combined string, not including an ASCII
-// NUL termination character. This length may exceed the available storage:
-// in this case, the written string will be truncated to avoid a buffer
-// overrun, and the return value will greater than or equal to the storage
-// size. A NULL argument may be provided as the destination buffer in order
-// to determine the required string length without actually writing a string.
-size_t loader_platform_combine_path(char *dest, size_t len, ...) {
-    size_t required_len = 0;
-    va_list ap;
-    const char *component;
-
-    va_start(ap, len);
-    component = va_arg(ap, const char *);
-    while (component) {
-        if (required_len > 0) {
-            // This path element is not the first non-empty element; prepend
-            // a directory separator if space allows
-            if (dest && required_len + 1 < len) {
-                (void)snprintf(dest + required_len, len - required_len, "%c", DIRECTORY_SYMBOL);
-            }
-            required_len++;
-        }
-
-        if (dest && required_len < len) {
-            strncpy(dest + required_len, component, len - required_len);
-        }
-        required_len += strlen(component);
-        component = va_arg(ap, const char *);
-    }
-
-    va_end(ap);
-
-    // strncpy(3) won't add a NUL terminating byte in the event of truncation.
-    if (dest && required_len >= len) {
-        dest[len - 1] = '\0';
-    }
-
-    return required_len;
-}
-
 // 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) {
@@ -2028,10 +1983,11 @@ VkResult combine_manifest_directory_and_library_path(const struct loader_instanc
     }
     // Add manifest_file_path up to the last directory symbol
     if (found_directory_symbol) {
-        strncpy(*out_fullpath, manifest_file_path, last_directory_symbol);
+        loader_strncpy(*out_fullpath, new_str_len, manifest_file_path, last_directory_symbol);
         cur_loc_in_out_fullpath += last_directory_symbol;
     }
-    strncpy(&(*out_fullpath)[cur_loc_in_out_fullpath], library_path, library_path_len);
+    loader_strncpy(&(*out_fullpath)[cur_loc_in_out_fullpath], new_str_len - cur_loc_in_out_fullpath, library_path,
+                   library_path_len);
     cur_loc_in_out_fullpath += library_path_len + 1;
     (*out_fullpath)[cur_loc_in_out_fullpath] = '\0';
 
@@ -2041,22 +1997,25 @@ VkResult combine_manifest_directory_and_library_path(const struct loader_instanc
     return res;
 }
 
-// Given a filename (file)  and a list of paths (dir), try to find an existing
+// Given a filename (file)  and a list of paths (in_dirs), try to find an existing
 // file in the paths.  If filename already is a path then no searching in the given paths.
 //
 // @return - A string in out_fullpath of either the full path or file.
 void loader_get_fullpath(const char *file, const char *in_dirs, size_t out_size, char *out_fullpath) {
     if (!loader_platform_is_path(file) && *in_dirs) {
         size_t dirs_copy_len = strlen(in_dirs) + 1;
         char *dirs_copy = loader_stack_alloc(dirs_copy_len);
-        strncpy(dirs_copy, in_dirs, dirs_copy_len);
+        loader_strncpy(dirs_copy, dirs_copy_len, in_dirs, dirs_copy_len);
 
         // find if file exists after prepending paths in given list
         // for (dir = dirs_copy; *dir && (next_dir = loader_get_next_path(dir)); dir = next_dir) {
         char *dir = dirs_copy;
         char *next_dir = loader_get_next_path(dir);
         while (*dir && next_dir) {
-            loader_platform_combine_path(out_fullpath, out_size, dir, file, NULL);
+            int path_concat_ret = snprintf(out_fullpath, out_size, "%s%c%s", dir, DIRECTORY_SYMBOL, file);
+            if (path_concat_ret < 0) {
+                continue;
+            }
             if (loader_platform_file_exists(out_fullpath)) {
                 return;
             }
@@ -3185,8 +3144,9 @@ VkResult read_data_files_in_search_paths(const struct loader_instance *inst, enu
 
     // Add the remaining paths to the list
     if (NULL != override_path) {
-        strncpy(cur_path_ptr, override_path, search_path_size);
-        cur_path_ptr += strlen(override_path);
+        size_t override_path_len = strlen(override_path);
+        loader_strncpy(cur_path_ptr, search_path_size, override_path, override_path_len);
+        cur_path_ptr += override_path_len;
     } else {
         // Add any additional search paths defined in the additive environment variable
         if (NULL != additional_env) {
@@ -3286,7 +3246,7 @@ VkResult read_data_files_in_search_paths(const struct loader_instance *inst, enu
     if (search_path_size > 0) {
         char *tmp_search_path = loader_instance_heap_alloc(inst, search_path_size + 1, VK_SYSTEM_ALLOCATION_SCOPE_COMMAND);
         if (NULL != tmp_search_path) {
-            strncpy(tmp_search_path, search_path, search_path_size);
+            loader_strncpy(tmp_search_path, search_path_size + 1, search_path, search_path_size);
             tmp_search_path[search_path_size] = '\0';
             if (manifest_type == LOADER_DATA_FILE_MANIFEST_DRIVER) {
                 log_flags = VULKAN_LOADER_DRIVER_BIT;

loader/loader.h

@@ -110,6 +110,7 @@ VkResult append_str_to_string_list(const struct loader_instance *inst, struct lo
 // Resize if there isn't enough space, then copy the string str to a new string the end of the loader_string_list
 // This function does not take ownership of the string, it merely copies it.
 // This function appends a null terminator to the string automatically
+// The str_len parameter does not include the null terminator
 VkResult copy_str_to_string_list(const struct loader_instance *inst, struct loader_string_list *string_list, const char *str,
                                  size_t str_len);
 

loader/loader_environment.c

@@ -275,9 +275,11 @@ VkResult parse_generic_filter_environment_var(const struct loader_instance *inst
         size_t actual_len;
         determine_filter_type(token, &cur_filter_type, &actual_start, &actual_len);
         if (actual_len > VK_MAX_EXTENSION_NAME_SIZE) {
-            strncpy(filter_struct->filters[filter_struct->count].value, actual_start, VK_MAX_EXTENSION_NAME_SIZE);
+            loader_strncpy(filter_struct->filters[filter_struct->count].value, VK_MAX_EXTENSION_NAME_SIZE, actual_start,
+                           VK_MAX_EXTENSION_NAME_SIZE);
         } else {
-            strncpy(filter_struct->filters[filter_struct->count].value, actual_start, actual_len);
+            loader_strncpy(filter_struct->filters[filter_struct->count].value, VK_MAX_EXTENSION_NAME_SIZE, actual_start,
+                           actual_len);
         }
         filter_struct->filters[filter_struct->count].length = actual_len;
         filter_struct->filters[filter_struct->count++].type = cur_filter_type;
@@ -344,9 +346,11 @@ VkResult parse_layers_disable_filter_environment_var(const struct loader_instanc
             }
         } else {
             if (actual_len > VK_MAX_EXTENSION_NAME_SIZE) {
-                strncpy(disable_struct->additional_filters.filters[cur_count].value, actual_start, VK_MAX_EXTENSION_NAME_SIZE);
+                loader_strncpy(disable_struct->additional_filters.filters[cur_count].value, VK_MAX_EXTENSION_NAME_SIZE,
+                               actual_start, VK_MAX_EXTENSION_NAME_SIZE);
             } else {
-                strncpy(disable_struct->additional_filters.filters[cur_count].value, actual_start, actual_len);
+                loader_strncpy(disable_struct->additional_filters.filters[cur_count].value, VK_MAX_EXTENSION_NAME_SIZE,
+                               actual_start, actual_len);
             }
             disable_struct->additional_filters.filters[cur_count].length = actual_len;
             disable_struct->additional_filters.filters[cur_count].type = cur_filter_type;
@@ -440,7 +444,7 @@ VkResult loader_add_environment_layers(struct loader_instance *inst, const enum
         size_t layer_env_len = strlen(layer_env) + 1;
         char *name = loader_stack_alloc(layer_env_len);
         if (name != NULL) {
-            strncpy(name, layer_env, layer_env_len);
+            loader_strncpy(name, layer_env_len, layer_env, layer_env_len);
 
             loader_log(inst, VULKAN_LOADER_WARN_BIT | VULKAN_LOADER_LAYER_BIT, 0, "env var \'%s\' defined and adding layers \"%s\"",
                        ENABLED_LAYERS_ENV, name);

loader/loader_windows.c

@@ -622,7 +622,8 @@ VkResult windows_read_manifest_from_d3d_adapters(const struct loader_instance *i
             .value_type = REG_MULTI_SZ,
             .physical_adapter_index = 0,
         };
-        wcsncpy(filename_info.value_name, value_name, sizeof(filename_info.value_name) / sizeof(WCHAR));
+        size_t value_name_size = wcslen(value_name);
+        wcsncpy_s(filename_info.value_name, MAX_PATH, value_name, value_name_size);
         LoaderQueryAdapterInfo query_info;
         query_info.handle = adapters.adapters[i].handle;
         query_info.type = LOADER_QUERY_TYPE_REGISTRY;
@@ -1105,11 +1106,12 @@ VkResult get_settings_path_if_exists_in_registry_key(const struct loader_instanc
         }
 
         if (strcmp(VK_LOADER_SETTINGS_FILENAME, &(name[start_of_path_filename])) == 0) {
-            *out_path = loader_instance_heap_alloc(inst, name_size, VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE);
+            *out_path = loader_instance_heap_calloc(inst, name_size + 1, VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE);
             if (*out_path == NULL) {
                 return VK_ERROR_OUT_OF_HOST_MEMORY;
             }
-            strcpy(*out_path, name);
+            loader_strncpy(*out_path, name_size + 1, name, name_size);
+            (*out_path)[name_size] = '\0';
             result = VK_SUCCESS;
             break;
         }

loader/log.c

@@ -167,16 +167,16 @@ void loader_log(const struct loader_instance *inst, VkFlags msg_type, int32_t ms
     // Also use the same header for all output
     char cmd_line_msg[64];
     size_t cmd_line_size = sizeof(cmd_line_msg);
-    size_t num_used = 1;
+    size_t num_used = 0;
 
     cmd_line_msg[0] = '\0';
 
 // Helper macro which strncat's the given string literal, then updates num_used & cmd_line_end
 // Assumes that we haven't used the entire buffer - must manually check this when adding new filter types
 // We concat at the end of cmd_line_msg, so that strncat isn't a victim of Schlemiel the Painter
 // We write to the end - 1 of cmd_line_msg, as the end is actually a null terminator
-#define STRNCAT_TO_BUFFER(string_literal_to_cat)                                             \
-    strncat(cmd_line_msg + (num_used - 1), string_literal_to_cat, cmd_line_size - num_used); \
+#define STRNCAT_TO_BUFFER(string_literal_to_cat)                                                                             \
+    loader_strncat(cmd_line_msg + num_used, cmd_line_size - num_used, string_literal_to_cat, sizeof(string_literal_to_cat)); \
     num_used += sizeof(string_literal_to_cat) - 1;  // subtract one to remove the null terminator in the string literal
 
     if ((msg_type & VULKAN_LOADER_ERROR_BIT) != 0) {
@@ -216,8 +216,8 @@ void loader_log(const struct loader_instance *inst, VkFlags msg_type, int32_t ms
     if (num_used < 19) {
         const char *space_buffer = "                   ";
         // Only write (19 - num_used) spaces
-        strncat(cmd_line_msg + (num_used - 1), space_buffer, 19 - num_used);
-        num_used += sizeof(space_buffer) - (num_used - 1);
+        loader_strncat(cmd_line_msg + num_used, cmd_line_size - num_used, space_buffer, 19 - num_used);
+        num_used += sizeof(space_buffer) - 1 - num_used;
     }
     // Assert that we didn't write more than what is available in cmd_line_msg
     assert(cmd_line_size > num_used);