Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow tests to directly call loader internal functions & Always memset newly reallocated memory #1629

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions loader/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,10 @@ if (UNKNOWN_FUNCTIONS_SUPPORTED)
add_dependencies(vulkan loader_asm_gen_files)
endif()

if (BUILD_TESTS)
target_compile_definitions(vulkan PRIVATE SHOULD_EXPORT_TEST_FUNCTIONS)
endif()

if (APPLE_STATIC_LOADER)
# TLDR: This feature only exists at the request of Google for Chromium. No other project should use this!
message(NOTICE "Apple STATIC lib: it will be built but not installed, and vulkan.pc and VulkanLoaderConfig.cmake won't be generated!")
Expand Down
4 changes: 4 additions & 0 deletions loader/allocation.c
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ void *loader_realloc(const VkAllocationCallbacks *pAllocator, void *pMemory, siz
#endif
} else {
pNewMem = realloc(pMemory, size);
// Clear out the newly allocated memory
if (size > orig_size) {
memset((uint8_t *)pNewMem + orig_size, 0, size - orig_size);
}
}
return pNewMem;
}
Expand Down
6 changes: 4 additions & 2 deletions loader/cJSON.c
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ static cJSON *cJSON_New_Item(const VkAllocationCallbacks *pAllocator) {
}

/* Delete a cJSON structure. */
CJSON_PUBLIC(void) loader_cJSON_Delete(cJSON *item) {
TEST_FUNCTION_EXPORT CJSON_PUBLIC(void) loader_cJSON_Delete(cJSON *item) {
cJSON *next = NULL;
while (item != NULL) {
next = item->next;
Expand Down Expand Up @@ -949,7 +949,9 @@ static unsigned char *print(const cJSON *const item, cJSON_bool format, bool *ou
}

/* Render a cJSON item/entity/structure to text. */
CJSON_PUBLIC(char *) loader_cJSON_Print(const cJSON *item, bool *out_of_memory) { return (char *)print(item, true, out_of_memory); }
TEST_FUNCTION_EXPORT CJSON_PUBLIC(char *) loader_cJSON_Print(const cJSON *item, bool *out_of_memory) {
return (char *)print(item, true, out_of_memory);
}

CJSON_PUBLIC(char *) loader_cJSON_PrintUnformatted(const cJSON *item, bool *out_of_memory) {
return (char *)print(item, false, out_of_memory);
Expand Down
6 changes: 4 additions & 2 deletions loader/cJSON.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ then using the CJSON_API_VISIBILITY flag to "export" the same symbols the way CJ
#include <stddef.h>
#include <stdbool.h>

#include "vk_loader_platform.h"

/* cJSON Types: */
#define cJSON_Invalid (0)
#define cJSON_False (1 << 0)
Expand Down Expand Up @@ -163,7 +165,7 @@ loader_cJSON_ParseWithLengthOpts(const VkAllocationCallbacks *pAllocator, const
const char **return_parse_end, cJSON_bool require_null_terminated, bool *out_of_memory);

/* Render a cJSON entity to text for transfer/storage. */
CJSON_PUBLIC(char *) loader_cJSON_Print(const cJSON *item, bool *out_of_memory);
TEST_FUNCTION_EXPORT CJSON_PUBLIC(char *) loader_cJSON_Print(const cJSON *item, bool *out_of_memory);
/* Render a cJSON entity to text for transfer/storage without any formatting. */
CJSON_PUBLIC(char *) loader_cJSON_PrintUnformatted(const cJSON *item, bool *out_of_memory);
/* Render a cJSON entity to text using a buffered strategy. prebuffer is a guess at the final size. guessing well reduces
Expand All @@ -177,7 +179,7 @@ loader_cJSON_PrintBuffered(const cJSON *item, int prebuffer, cJSON_bool fmt, boo
CJSON_PUBLIC(cJSON_bool)
loader_cJSON_PrintPreallocated(cJSON *item, char *buffer, const int length, const cJSON_bool format);
/* Delete a cJSON entity and all subentities. */
CJSON_PUBLIC(void) loader_cJSON_Delete(cJSON *item);
TEST_FUNCTION_EXPORT CJSON_PUBLIC(void) loader_cJSON_Delete(cJSON *item);

/* Returns the number of items in an array (or object). */
CJSON_PUBLIC(int) loader_cJSON_GetArraySize(const cJSON *array);
Expand Down
6 changes: 2 additions & 4 deletions loader/loader.c
Original file line number Diff line number Diff line change
Expand Up @@ -323,8 +323,6 @@ VkResult append_str_to_string_list(const struct loader_instance *inst, struct lo
loader_instance_heap_free(inst, str); // Must clean up in case of failure
return VK_ERROR_OUT_OF_HOST_MEMORY;
}
// Null out the new space
memset(string_list->list + string_list->allocated_count, 0, string_list->allocated_count);
string_list->allocated_count *= 2;
}
string_list->list[string_list->count++] = str;
Expand Down Expand Up @@ -439,7 +437,6 @@ VkResult loader_append_layer_property(const struct loader_instance *inst, struct
goto out;
}
layer_list->list = new_ptr;
memset((uint8_t *)layer_list->list + layer_list->capacity, 0, layer_list->capacity);
layer_list->capacity *= 2;
}
memcpy(&layer_list->list[layer_list->count], layer_property, sizeof(struct loader_layer_properties));
Expand Down Expand Up @@ -508,7 +505,8 @@ bool loader_find_layer_name_in_blacklist(const char *layer_name, struct loader_l
}

// Remove all layer properties entries from the list
void loader_delete_layer_list_and_properties(const struct loader_instance *inst, struct loader_layer_list *layer_list) {
TEST_FUNCTION_EXPORT void loader_delete_layer_list_and_properties(const struct loader_instance *inst,
struct loader_layer_list *layer_list) {
uint32_t i;
if (!layer_list) return;

Expand Down
3 changes: 2 additions & 1 deletion loader/loader.h
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,8 @@ VkResult loader_add_device_extensions(const struct loader_instance *inst,
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);
void loader_delete_layer_list_and_properties(const struct loader_instance *inst, struct loader_layer_list *layer_list);
TEST_FUNCTION_EXPORT void loader_delete_layer_list_and_properties(const struct loader_instance *inst,
struct loader_layer_list *layer_list);
void loader_remove_layer_in_list(const struct loader_instance *inst, struct loader_layer_list *layer_list,
uint32_t layer_to_remove);
VkResult loader_init_scanned_icd_list(const struct loader_instance *inst, struct loader_icd_tramp_list *icd_tramp_list);
Expand Down
2 changes: 1 addition & 1 deletion loader/loader_json.c
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ VkResult loader_read_entire_file(const struct loader_instance *inst, const char
}
#endif

VkResult loader_get_json(const struct loader_instance *inst, const char *filename, cJSON **json) {
TEST_FUNCTION_EXPORT VkResult loader_get_json(const struct loader_instance *inst, const char *filename, cJSON **json) {
char *json_buf = NULL;
VkResult res = VK_SUCCESS;

Expand Down
2 changes: 1 addition & 1 deletion loader/loader_json.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ struct loader_string_list;
//
// @return - A pointer to a cJSON object representing the JSON parse tree.
// This returned buffer should be freed by caller.
VkResult loader_get_json(const struct loader_instance *inst, const char *filename, cJSON **json);
TEST_FUNCTION_EXPORT VkResult loader_get_json(const struct loader_instance *inst, const char *filename, cJSON **json);

// Given a cJSON object, find the string associated with the key and puts an pre-allocated string into out_string.
// Length is given by out_str_len, and this function truncates the string with a null terminator if it the provided space isn't
Expand Down
6 changes: 3 additions & 3 deletions loader/settings.c
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ VkResult get_loader_settings(const struct loader_instance* inst, loader_settings
return res;
}

VkResult update_global_loader_settings(void) {
TEST_FUNCTION_EXPORT VkResult update_global_loader_settings(void) {
loader_settings settings = {0};
VkResult res = get_loader_settings(NULL, &settings);
loader_platform_thread_lock_mutex(&global_loader_settings_lock);
Expand Down Expand Up @@ -538,8 +538,8 @@ void release_current_settings_lock(const struct loader_instance* inst) {
}
}

VkResult get_settings_layers(const struct loader_instance* inst, struct loader_layer_list* settings_layers,
bool* should_search_for_other_layers) {
TEST_FUNCTION_EXPORT VkResult get_settings_layers(const struct loader_instance* inst, struct loader_layer_list* settings_layers,
bool* should_search_for_other_layers) {
VkResult res = VK_SUCCESS;
*should_search_for_other_layers = true; // default to true

Expand Down
7 changes: 4 additions & 3 deletions loader/settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "vulkan/vulkan_core.h"

#include "log.h"
#include "vk_loader_platform.h"

struct loader_instance;
struct loader_layer_list;
Expand Down Expand Up @@ -83,7 +84,7 @@ void free_loader_settings(const struct loader_instance* inst, loader_settings* l
void log_settings(const struct loader_instance* inst, loader_settings* settings);

// Every global function needs to call this at startup to insure that
VkResult update_global_loader_settings(void);
TEST_FUNCTION_EXPORT VkResult update_global_loader_settings(void);

// Needs to be called during startup -
void init_global_loader_settings(void);
Expand All @@ -94,8 +95,8 @@ bool should_skip_logging_global_messages(VkFlags msg_type);

// Query the current settings (either global or per-instance) and return the list of layers contained within.
// should_search_for_other_layers tells the caller if the settings file should be used exclusively for layer searching or not
VkResult get_settings_layers(const struct loader_instance* inst, struct loader_layer_list* settings_layers,
bool* should_search_for_other_layers);
TEST_FUNCTION_EXPORT VkResult get_settings_layers(const struct loader_instance* inst, struct loader_layer_list* settings_layers,
bool* should_search_for_other_layers);

// Take the provided list of settings_layers and add in the layers from regular search paths
// Only adds layers that aren't already present in the settings_layers and in the location of the
Expand Down
11 changes: 11 additions & 0 deletions loader/vk_loader_platform.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,17 @@
#define LOADER_EXPORT
#endif

// For testing purposes, we want to expose some functions not normally callable on the library
#if defined(SHOULD_EXPORT_TEST_FUNCTIONS)
#if defined(_WIN32)
#define TEST_FUNCTION_EXPORT __declspec(dllexport)
#else
#define TEST_FUNCTION_EXPORT LOADER_EXPORT
#endif
#else
#define TEST_FUNCTION_EXPORT
#endif

#define MAX_STRING_SIZE 1024

// This is defined in vk_layer.h, but if there's problems we need to create the define
Expand Down
28 changes: 14 additions & 14 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ if (IS_DIRECTORY "${GOOGLETEST_INSTALL_DIR}/googletest")
set(BUILD_GTEST ON)
set(BUILD_GMOCK OFF)
set(gtest_force_shared_crt ON)
set(BUILD_SHARED_LIBS ON)
set(INSTALL_GTEST OFF)
add_subdirectory("${GOOGLETEST_INSTALL_DIR}" ${CMAKE_CURRENT_BINARY_DIR}/gtest)
else()
Expand Down Expand Up @@ -84,7 +83,6 @@ add_executable(
loader_envvar_tests.cpp
loader_get_proc_addr_tests.cpp
loader_debug_ext_tests.cpp
loader_fuzz_tests.cpp
loader_handle_validation_tests.cpp
loader_layer_tests.cpp
loader_regression_tests.cpp
Expand All @@ -96,6 +94,14 @@ add_executable(
target_link_libraries(test_regression PUBLIC testing_dependencies)
target_compile_definitions(test_regression PUBLIC VK_NO_PROTOTYPES)

add_executable(
test_fuzzing
loader_testing_main.cpp
loader_fuzz_tests.cpp
)
target_link_libraries(test_fuzzing PUBLIC testing_dependencies vulkan)
target_include_directories(test_fuzzing PUBLIC ${CMAKE_SOURCE_DIR}/loader ${CMAKE_SOURCE_DIR}/loader/generated)

# Threading tests live in separate executabe just for threading tests as it'll need support
# in the test harness to enable in CI, as thread sanitizer doesn't work with address sanitizer enabled.
add_executable(
Expand All @@ -111,19 +117,10 @@ if (ENABLE_LIVE_VERIFICATION_TESTS)
endif()

if(WIN32)
# Copy loader and googletest (gtest) libs to test dir so the test executable can find them.
add_custom_command(TARGET test_regression POST_BUILD
COMMAND ${CMAKE_COMMAND} -E copy $<TARGET_FILE:gtest> $<TARGET_FILE_DIR:test_regression>)
# Copy the loader shared lib (if built) to the test application directory so the test app finds it.
# Copy vulkan-1.dll to the test_fuzzing build directory so that the test_fuzzing exe can find it.
if(TARGET vulkan)
add_custom_command(TARGET test_regression POST_BUILD
COMMAND ${CMAKE_COMMAND} -E copy $<TARGET_FILE:vulkan> $<TARGET_FILE_DIR:test_regression>)
endif()

# Copy the gtest shared lib (if built) to the live verification tests directory so the tests finds it.
if(ENABLE_LIVE_VERIFICATION_TESTS)
add_custom_command(TARGET test_regression POST_BUILD
COMMAND ${CMAKE_COMMAND} -E copy $<TARGET_FILE:gtest> $<TARGET_FILE_DIR:dynamic_rendering_get_proc_addr>)
add_custom_command(TARGET test_fuzzing POST_BUILD
COMMAND ${CMAKE_COMMAND} -E copy $<TARGET_FILE:vulkan> $<TARGET_FILE_DIR:test_fuzzing>)
endif()
endif()

Expand All @@ -138,8 +135,10 @@ endif()
# must happen after the dll's get copied over
if(NOT CMAKE_CROSSCOMPILING)
gtest_discover_tests(test_regression PROPERTIES DISCOVERY_TIMEOUT 100)
gtest_discover_tests(test_fuzzing PROPERTIES DISCOVERY_TIMEOUT 100)
else()
gtest_add_tests(TARGET test_regression)
gtest_add_tests(TARGET test_fuzzing)
endif()

# When APPLE_STATIC_LOADER is ON installation is disabled
Expand Down Expand Up @@ -173,4 +172,5 @@ set_tests_properties(integration.find_package PROPERTIES DEPENDS integration.ins

if (CODE_COVERAGE)
target_code_coverage(test_regression AUTO ALL )
target_code_coverage(test_fuzzing AUTO ALL )
endif()
1 change: 0 additions & 1 deletion tests/framework/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ add_library(testing_dependencies STATIC test_environment.cpp test_environment.h)
target_link_libraries(testing_dependencies
PUBLIC gtest Vulkan::Headers testing_framework_util shim-library)
target_include_directories(testing_dependencies PUBLIC ${CMAKE_CURRENT_BINARY_DIR})
target_compile_definitions(testing_dependencies PUBLIC "GTEST_LINKED_AS_SHARED_LIBRARY=1")
if (APPLE_STATIC_LOADER)
target_compile_definitions(testing_dependencies PUBLIC "APPLE_STATIC_LOADER=1")
target_link_libraries(testing_dependencies PUBLIC vulkan)
Expand Down
Binary file not shown.
Binary file not shown.
42 changes: 20 additions & 22 deletions tests/framework/test_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -242,53 +242,51 @@ class FolderManager {
inline void copy_string_to_char_array(std::string const& src, char* dst, size_t size_dst) { dst[src.copy(dst, size_dst - 1)] = 0; }

#if defined(WIN32)
typedef HMODULE loader_platform_dl_handle;
inline loader_platform_dl_handle loader_platform_open_library(const wchar_t* lib_path) {
typedef HMODULE test_platform_dl_handle;
inline test_platform_dl_handle test_platform_open_library(const wchar_t* lib_path) {
// Try loading the library the original way first.
loader_platform_dl_handle lib_handle = LoadLibraryW(lib_path);
test_platform_dl_handle lib_handle = LoadLibraryW(lib_path);
if (lib_handle == nullptr && GetLastError() == ERROR_MOD_NOT_FOUND) {
// If that failed, then try loading it with broader search folders.
lib_handle = LoadLibraryExW(lib_path, nullptr, LOAD_LIBRARY_SEARCH_DEFAULT_DIRS | LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR);
}
return lib_handle;
}
inline void loader_platform_open_library_print_error(std::filesystem::path const& libPath) {
inline void test_platform_open_library_print_error(std::filesystem::path const& libPath) {
std::wcerr << L"Unable to open library: " << libPath << L" due to: " << std::to_wstring(GetLastError()) << L"\n";
}
inline void loader_platform_close_library(loader_platform_dl_handle library) { FreeLibrary(library); }
inline void* loader_platform_get_proc_address(loader_platform_dl_handle library, const char* name) {
inline void test_platform_close_library(test_platform_dl_handle library) { FreeLibrary(library); }
inline void* test_platform_get_proc_address(test_platform_dl_handle library, const char* name) {
assert(library);
assert(name);
return reinterpret_cast<void*>(GetProcAddress(library, name));
}
inline char* loader_platform_get_proc_address_error(const char* name) {
inline char* test_platform_get_proc_address_error(const char* name) {
static char errorMsg[120];
snprintf(errorMsg, 119, "Failed to find function \"%s\" in dynamic library", name);
return errorMsg;
}

#elif COMMON_UNIX_PLATFORMS

typedef void* loader_platform_dl_handle;
inline loader_platform_dl_handle loader_platform_open_library(const char* libPath) {
return dlopen(libPath, RTLD_LAZY | RTLD_LOCAL);
}
inline void loader_platform_open_library_print_error(std::filesystem::path const& libPath) {
typedef void* test_platform_dl_handle;
inline test_platform_dl_handle test_platform_open_library(const char* libPath) { return dlopen(libPath, RTLD_LAZY | RTLD_LOCAL); }
inline void test_platform_open_library_print_error(std::filesystem::path const& libPath) {
std::wcerr << "Unable to open library: " << libPath << " due to: " << dlerror() << "\n";
}
inline void loader_platform_close_library(loader_platform_dl_handle library) {
inline void test_platform_close_library(test_platform_dl_handle library) {
char* loader_disable_dynamic_library_unloading_env_var = getenv("VK_LOADER_DISABLE_DYNAMIC_LIBRARY_UNLOADING");
if (NULL == loader_disable_dynamic_library_unloading_env_var ||
0 != strncmp(loader_disable_dynamic_library_unloading_env_var, "1", 2)) {
dlclose(library);
}
}
inline void* loader_platform_get_proc_address(loader_platform_dl_handle library, const char* name) {
inline void* test_platform_get_proc_address(test_platform_dl_handle library, const char* name) {
assert(library);
assert(name);
return dlsym(library, name);
}
inline const char* loader_platform_get_proc_address_error([[maybe_unused]] const char* name) { return dlerror(); }
inline const char* test_platform_get_proc_address_error([[maybe_unused]] const char* name) { return dlerror(); }
#endif

class FromVoidStarFunc {
Expand All @@ -308,15 +306,15 @@ class FromVoidStarFunc {
struct LibraryWrapper {
explicit LibraryWrapper() noexcept {}
explicit LibraryWrapper(std::filesystem::path const& lib_path) noexcept : lib_path(lib_path) {
lib_handle = loader_platform_open_library(lib_path.native().c_str());
lib_handle = test_platform_open_library(lib_path.native().c_str());
if (lib_handle == nullptr) {
loader_platform_open_library_print_error(lib_path);
test_platform_open_library_print_error(lib_path);
assert(lib_handle != nullptr && "Must be able to open library");
}
}
~LibraryWrapper() noexcept {
if (lib_handle != nullptr) {
loader_platform_close_library(lib_handle);
test_platform_close_library(lib_handle);
lib_handle = nullptr;
}
}
Expand All @@ -328,7 +326,7 @@ struct LibraryWrapper {
LibraryWrapper& operator=(LibraryWrapper&& wrapper) noexcept {
if (this != &wrapper) {
if (lib_handle != nullptr) {
loader_platform_close_library(lib_handle);
test_platform_close_library(lib_handle);
}
lib_handle = wrapper.lib_handle;
lib_path = wrapper.lib_path;
Expand All @@ -338,17 +336,17 @@ struct LibraryWrapper {
}
FromVoidStarFunc get_symbol(const char* symbol_name) const {
assert(lib_handle != nullptr && "Cannot get symbol with null library handle");
void* symbol = loader_platform_get_proc_address(lib_handle, symbol_name);
void* symbol = test_platform_get_proc_address(lib_handle, symbol_name);
if (symbol == nullptr) {
fprintf(stderr, "Unable to open symbol %s: %s\n", symbol_name, loader_platform_get_proc_address_error(symbol_name));
fprintf(stderr, "Unable to open symbol %s: %s\n", symbol_name, test_platform_get_proc_address_error(symbol_name));
assert(symbol != nullptr && "Must be able to get symbol");
}
return FromVoidStarFunc(symbol);
}

explicit operator bool() const noexcept { return lib_handle != nullptr; }

loader_platform_dl_handle lib_handle = nullptr;
test_platform_dl_handle lib_handle = nullptr;
std::filesystem::path lib_path;
};

Expand Down
Loading
Loading