From d3d79d9b1f6ae424b7b76188f3ce645870932d6a Mon Sep 17 00:00:00 2001 From: "s.kolesnik" <7fosGbc9S> Date: Wed, 26 May 2021 16:04:57 +0500 Subject: [PATCH 1/9] added clean build option in cmake to disallow warnings --- CMakeLists.txt | 14 ++++++++++++++ test/loader.cpp | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d6fdc75..acfefc2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -6,6 +6,20 @@ project(dynalo) #set(CMAKE_CXX_STANDARD_REQUIRED ON ) #set(CMAKE_CXX_EXTENSIONS OFF) +option(CLEAN_BUILD "Clean build with no warnings allowed" OFF) +message(STATUS "Clean build, warnings are not allowed: " ${CLEAN_BUILD}) + +if (${CLEAN_BUILD}) + if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") + add_compile_options(/W4 /WX) + elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + add_compile_options(-Wall -Wextra -pedantic -Werror) + else() + # TODO: implement flags for other compilers + message(STATUS "CLEAN_BUILD is not supported for ${CMAKE_CXX_COMPILER_ID}") + endif() +endif() + add_library(${PROJECT_NAME} INTERFACE) target_include_directories(${PROJECT_NAME} INTERFACE "$" diff --git a/test/loader.cpp b/test/loader.cpp index 6b490aa..12f8af4 100644 --- a/test/loader.cpp +++ b/test/loader.cpp @@ -4,7 +4,7 @@ #include // usage: loader "path/to/shared/lib/dir" -int main(int argc, char* argv[]) +int main(int /*argc*/, char* argv[]) { dynalo::library lib(std::string(argv[1]) + "/" + dynalo::to_native_name("shared")); From 4e6833a0a967a2909a59577b39f6c57d11b85c09 Mon Sep 17 00:00:00 2001 From: "s.kolesnik" <7fosGbc9S> Date: Wed, 26 May 2021 16:43:00 +0500 Subject: [PATCH 2/9] moved enable_testing to the root directory as required by the cmake documentation. Otherwise ctest might not be included and no targets for test generated --- CMakeLists.txt | 1 + test/CMakeLists.txt | 7 +++---- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index acfefc2..9946cb2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -72,4 +72,5 @@ install( DESTINATION "${CONFIG_PACKAGE_INSTALL_LOCATION}" ) +enable_testing() add_subdirectory(test) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index be4aeaa..e69c7be 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -1,6 +1,6 @@ -cmake_minimum_required(VERSION 3.0.0) +#cmake_minimum_required(VERSION 3.0.0) -project(dynalo-test) +# project(dynalo-test) set(CMAKE_CXX_STANDARD 11 ) set(CMAKE_CXX_STANDARD_REQUIRED ON ) @@ -13,7 +13,6 @@ add_library(shared SHARED shared.cpp) add_dependencies(loader shared) -enable_testing() -add_test(NAME all_tests COMMAND +add_test(NAME loader COMMAND "$" "$" "shared" ) From 6c59fa08936b99ca9490b227787f575bec9be862 Mon Sep 17 00:00:00 2001 From: "s.kolesnik" <7fosGbc9S> Date: Wed, 26 May 2021 16:49:18 +0500 Subject: [PATCH 3/9] added try-catch block for loader. Added check for input. --- test/loader.cpp | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/test/loader.cpp b/test/loader.cpp index 12f8af4..e2a53e7 100644 --- a/test/loader.cpp +++ b/test/loader.cpp @@ -1,17 +1,33 @@ #include #include +#include #include // usage: loader "path/to/shared/lib/dir" -int main(int /*argc*/, char* argv[]) +int main(int argc, char* argv[]) { - dynalo::library lib(std::string(argv[1]) + "/" + dynalo::to_native_name("shared")); + if (argc < 2) + { + std::cerr << "No path for a dynamic library was provided.\nUsage: ./loader path_to_dyn_lib" << std::endl; + } + try + { + dynalo::library lib(std::string(argv[1]) + "/" + dynalo::to_native_name("shared")); - auto add_integers = lib.get_function("add_integers"); - auto print_message = lib.get_function("print_message"); + auto add_integers = lib.get_function("add_integers"); + auto print_message = lib.get_function("print_message"); - std::ostringstream oss; - oss << "it works: " << add_integers(1, 2); - print_message(oss.str().c_str()); + std::ostringstream oss; + oss << "it works: " << add_integers(1, 2); + print_message(oss.str().c_str()); + + return 0; + } + catch (const std::exception& error) + { + std::cerr << "Test failed: " << error.what() << std::endl; + } + + return -1; } From 3feaa7ed91e5987bb2a7b0825fb12bd7b2c7e3b7 Mon Sep 17 00:00:00 2001 From: "s.kolesnik" <7fosGbc9S> Date: Wed, 26 May 2021 16:55:54 +0500 Subject: [PATCH 4/9] fixed test: using absolute path from cmake target property --- test/CMakeLists.txt | 2 +- test/loader.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index e69c7be..10251d1 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -14,5 +14,5 @@ add_library(shared SHARED shared.cpp) add_dependencies(loader shared) add_test(NAME loader COMMAND - "$" "$" "shared" + "$" "$" ) diff --git a/test/loader.cpp b/test/loader.cpp index e2a53e7..dae97a6 100644 --- a/test/loader.cpp +++ b/test/loader.cpp @@ -13,7 +13,7 @@ int main(int argc, char* argv[]) } try { - dynalo::library lib(std::string(argv[1]) + "/" + dynalo::to_native_name("shared")); + dynalo::library lib(argv[1]); auto add_integers = lib.get_function("add_integers"); auto print_message = lib.get_function("print_message"); From f616ca0ed3f8f9a7660d1e4387ee21bb9e28a390 Mon Sep 17 00:00:00 2001 From: "s.kolesnik" <7fosGbc9S> Date: Wed, 26 May 2021 17:02:53 +0500 Subject: [PATCH 5/9] fixed cmake to remove warnings --- CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9946cb2..e1614ff 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -10,9 +10,9 @@ option(CLEAN_BUILD "Clean build with no warnings allowed" OFF) message(STATUS "Clean build, warnings are not allowed: " ${CLEAN_BUILD}) if (${CLEAN_BUILD}) - if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") + if (CMAKE_CXX_COMPILER_ID MATCHES "MSVC") add_compile_options(/W4 /WX) - elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + elseif(CMAKE_CXX_COMPILER_ID MATCHES "GNU") add_compile_options(-Wall -Wextra -pedantic -Werror) else() # TODO: implement flags for other compilers From 4c1c0fdba32b33581b819a718bcea0ee4f869016 Mon Sep 17 00:00:00 2001 From: "s.kolesnik" <7fosGbc9S> Date: Wed, 26 May 2021 17:06:27 +0500 Subject: [PATCH 6/9] worked around function cast warning. --- include/dynalo/detail/linux/dynalo.hpp | 4 +++- include/dynalo/detail/windows/dynalo.hpp | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/include/dynalo/detail/linux/dynalo.hpp b/include/dynalo/detail/linux/dynalo.hpp index 77c1c76..441ae1d 100644 --- a/include/dynalo/detail/linux/dynalo.hpp +++ b/include/dynalo/detail/linux/dynalo.hpp @@ -64,7 +64,9 @@ FunctionSignature* get_function(native::handle lib_handle, const std::string& fu throw std::runtime_error(std::string("Failed to get [func_name:") + func_name + "]: " + last_error()); } - return reinterpret_cast(func_ptr); +// TODO: disable optimization for these lines since it is UB + void **intermediate_ptr = reinterpret_cast(&func_ptr); + return reinterpret_cast(*intermediate_ptr); } }} diff --git a/include/dynalo/detail/windows/dynalo.hpp b/include/dynalo/detail/windows/dynalo.hpp index c4eea6d..1d4aa0e 100644 --- a/include/dynalo/detail/windows/dynalo.hpp +++ b/include/dynalo/detail/windows/dynalo.hpp @@ -92,7 +92,9 @@ FunctionSignature* get_function(native::handle lib_handle, const std::string& fu throw std::runtime_error(std::string("Failed to get [func_name:") + func_name + "]: " + last_error()); } - return reinterpret_cast(func_ptr); +// TODO: disable optimization for these lines since it is UB + void **intermediate_ptr = reinterpret_cast(&func_ptr); + return reinterpret_cast(*intermediate_ptr); } }} From 61c0cbea74560b0adf0a89d1e075b06de757c9fb Mon Sep 17 00:00:00 2001 From: "s.kolesnik" <7fosGbc9S> Date: Wed, 26 May 2021 17:11:06 +0500 Subject: [PATCH 7/9] added conditional option to build tests. Tests must not be included when project added via add_subdirectory. if EXCLUDE_FROM_ALL is specified as an option in the parent cmake list, its tests will fail --- CMakeLists.txt | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e1614ff..63ffcd1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -9,6 +9,12 @@ project(dynalo) option(CLEAN_BUILD "Clean build with no warnings allowed" OFF) message(STATUS "Clean build, warnings are not allowed: " ${CLEAN_BUILD}) +include(CMakeDependentOption) + +# do not build tests if included as a subdirectory +cmake_dependent_option(BUILD_TESTS "Build Tests" ON + "NOT hasParent" OFF) + if (${CLEAN_BUILD}) if (CMAKE_CXX_COMPILER_ID MATCHES "MSVC") add_compile_options(/W4 /WX) @@ -72,5 +78,7 @@ install( DESTINATION "${CONFIG_PACKAGE_INSTALL_LOCATION}" ) -enable_testing() -add_subdirectory(test) +if (BUILD_TESTS) + enable_testing() + add_subdirectory(test) +endif() From 30a56d86d8c4006b97ea2c87fe572f8b021164f8 Mon Sep 17 00:00:00 2001 From: "s.kolesnik" <7fosGbc9S> Date: Wed, 26 May 2021 17:32:28 +0500 Subject: [PATCH 8/9] modified conditional enable_testing() --- CMakeLists.txt | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 63ffcd1..699eca2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -8,12 +8,7 @@ project(dynalo) option(CLEAN_BUILD "Clean build with no warnings allowed" OFF) message(STATUS "Clean build, warnings are not allowed: " ${CLEAN_BUILD}) - -include(CMakeDependentOption) - -# do not build tests if included as a subdirectory -cmake_dependent_option(BUILD_TESTS "Build Tests" ON - "NOT hasParent" OFF) +option(BUILD_TESTS "Build tests for ${PROJECT_NAME}: " ON) if (${CLEAN_BUILD}) if (CMAKE_CXX_COMPILER_ID MATCHES "MSVC") @@ -78,7 +73,10 @@ install( DESTINATION "${CONFIG_PACKAGE_INSTALL_LOCATION}" ) -if (BUILD_TESTS) +get_directory_property(IS_EXCLUDED ${CMAKE_CURRENT_SOURCE_DIR} EXCLUDE_FROM_ALL) + +if (NOT IS_EXCLUDED AND BUILD_TESTS) + message(STATUS "Building tests for ${PROJECT_NAME}") enable_testing() add_subdirectory(test) endif() From e9206ba283a5e06afded46b79f6c93e7cad5ba20 Mon Sep 17 00:00:00 2001 From: "s.kolesnik" <7fosGbc9S> Date: Wed, 26 May 2021 17:43:09 +0500 Subject: [PATCH 9/9] modified conditional enable_testing() --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 699eca2..59382f0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -73,7 +73,7 @@ install( DESTINATION "${CONFIG_PACKAGE_INSTALL_LOCATION}" ) -get_directory_property(IS_EXCLUDED ${CMAKE_CURRENT_SOURCE_DIR} EXCLUDE_FROM_ALL) +get_directory_property(IS_EXCLUDED EXCLUDE_FROM_ALL) if (NOT IS_EXCLUDED AND BUILD_TESTS) message(STATUS "Building tests for ${PROJECT_NAME}")