From 1f853dd592b7a42512bcfb08acb32eede406ff29 Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Thu, 1 Aug 2024 08:15:22 -0400 Subject: [PATCH] feedbck: better CopyableWlArray + only running death tests if not asan --- src/include/server/mir/scene/text_input_hub.h | 24 ++--- tests/unit-tests/CMakeLists.txt | 13 ++- tests/unit-tests/test_udev_wrapper.cpp | 66 -------------- .../test_udev_wrapper_asan_skip.cpp | 88 +++++++++++++++++++ 4 files changed, 108 insertions(+), 83 deletions(-) create mode 100644 tests/unit-tests/test_udev_wrapper_asan_skip.cpp diff --git a/src/include/server/mir/scene/text_input_hub.h b/src/include/server/mir/scene/text_input_hub.h index 42e58ba48de..f6a970b898b 100644 --- a/src/include/server/mir/scene/text_input_hub.h +++ b/src/include/server/mir/scene/text_input_hub.h @@ -102,13 +102,14 @@ struct TextInputState class CopyableWlArray { public: - explicit CopyableWlArray(wl_array* in_data) - : data_{copy(in_data)} + explicit CopyableWlArray(wl_array const* in_data) { + wl_array_init(&data_); + wl_array_copy(&data_, const_cast(in_data)); } - + CopyableWlArray(CopyableWlArray const& other) - : CopyableWlArray{other.data_} + : CopyableWlArray{&other.data_} { } @@ -120,24 +121,15 @@ class CopyableWlArray ~CopyableWlArray() { - wl_array_release(data_); + wl_array_release(&data_); } [[nodiscard]] wl_array* data() const { - return data_; + return const_cast(&data_); } - private: - static wl_array* copy(wl_array* in_data) - { - auto data = new wl_array(); - wl_array_init(data); - wl_array_copy(data, in_data); - return data; - } - - wl_array* data_; + wl_array data_; }; /// See text-input-unstable-v3.xml for details diff --git a/tests/unit-tests/CMakeLists.txt b/tests/unit-tests/CMakeLists.txt index fd641868536..f297e1fb503 100644 --- a/tests/unit-tests/CMakeLists.txt +++ b/tests/unit-tests/CMakeLists.txt @@ -42,7 +42,11 @@ set_target_properties( # Umockdev uses glib, which uses the deprecated "register" allocation specifier set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Dregister=") -set(UMOCK_UNIT_TEST_SOURCES test_udev_wrapper.cpp) +if(CMAKE_BUILD_TYPE STREQUAL "AddressSanitizer") + set(UMOCK_UNIT_TEST_SOURCES test_udev_wrapper.cpp) +else() + set(UMOCK_UNIT_TEST_SOURCES test_udev_wrapper.cpp test_udev_wrapper_asan_skip.cpp) +endif() set( UNIT_TEST_SOURCES @@ -133,8 +137,15 @@ set_property( SOURCE input/test_logind_console_services.cpp input/test_input_platform_probing.cpp SOURCE input/evdev/test_evdev_input_platform.cpp SOURCE graphics/test_platform_prober.cpp + SOURCE test_udev_wrapper_asan_skip.cpp PROPERTY COMPILE_OPTIONS -Wno-variadic-macros) +if(NOT CMAKE_BUILD_TYPE STREQUAL "AddressSanitizer") + set_property( + SOURCE test_udev_wrapper_asan_skip.cpp + PROPERTY COMPILE_OPTIONS -Wno-variadic-macros) +endif() + if ("test_touchspot_controller.cpp:array-bounds" IN_LIST MIR_COMPILER_QUIRKS) set_property( SOURCE input/test_touchspot_controller.cpp diff --git a/tests/unit-tests/test_udev_wrapper.cpp b/tests/unit-tests/test_udev_wrapper.cpp index 26c83e86853..4c1e8b5645d 100644 --- a/tests/unit-tests/test_udev_wrapper.cpp +++ b/tests/unit-tests/test_udev_wrapper.cpp @@ -26,33 +26,15 @@ #include #include -#if defined(__has_feature) -# if __has_feature(address_sanitizer) // for clang -# define __SANITIZE_ADDRESS__ // GCC already sets this -# endif -#endif - namespace mtf=mir_test_framework; namespace { -#if !defined(__SANITIZE_ADDRESS__) -bool KilledByInvalidMemoryAccess(int exit_status) -{ - return testing::KilledBySignal(SIGSEGV)(exit_status) || - testing::KilledBySignal(SIGBUS)(exit_status) || - testing::KilledBySignal(SIGABRT)(exit_status) || - // It seems that valgrind kills us with SIGKILL - testing::KilledBySignal(SIGKILL)(exit_status); -} -#endif - class UdevWrapperTest : public ::testing::Test { public: mtf::UdevEnvironment udev_environment; }; - } TEST_F(UdevWrapperTest, IteratesOverCorrectNumberOfDevices) @@ -321,31 +303,6 @@ TEST_F(UdevWrapperTest, EnumeratorAddMatchSysnameIncludesCorrectDevices) } } -typedef UdevWrapperTest UdevWrapperDeathTest; - -#if !defined(__SANITIZE_ADDRESS__) -TEST_F(UdevWrapperDeathTest, DereferencingEndReturnsInvalidObject) -{ - ::testing::FLAGS_gtest_death_test_style = "threadsafe"; - udev_environment.add_device("drm", "control64D", NULL, {}, {}); - udev_environment.add_device("drm", "card1", NULL, {}, {}); - - mir::udev::Enumerator devices(std::make_shared()); - - devices.scan_devices(); - - MIR_EXPECT_EXIT((*devices.end()).subsystem(), KilledByInvalidMemoryAccess, ""); - - auto iter = devices.begin(); - - while(iter != devices.end()) - { - iter++; - } - MIR_EXPECT_EXIT((*iter).subsystem(), KilledByInvalidMemoryAccess, ""); -} -#endif - TEST_F(UdevWrapperTest, MemberDereferenceWorks) { udev_environment.add_device("drm", "control64D", NULL, {}, {}); @@ -360,29 +317,6 @@ TEST_F(UdevWrapperTest, MemberDereferenceWorks) EXPECT_STREQ("drm", iter->subsystem()); } -#if !defined(__SANITIZE_ADDRESS__) -TEST_F(UdevWrapperDeathTest, MemberDereferenceOfEndDies) -{ - ::testing::FLAGS_gtest_death_test_style = "threadsafe"; - udev_environment.add_device("drm", "control64D", NULL, {}, {}); - udev_environment.add_device("drm", "card1", NULL, {}, {}); - - mir::udev::Enumerator devices(std::make_shared()); - - devices.scan_devices(); - - MIR_EXPECT_EXIT(devices.end()->subsystem(), KilledByInvalidMemoryAccess, ""); - - auto iter = devices.begin(); - - while(iter != devices.end()) - { - iter++; - } - MIR_EXPECT_EXIT(iter->subsystem(), KilledByInvalidMemoryAccess, ""); -} -#endif - TEST_F(UdevWrapperTest, UdevMonitorDoesNotTriggerBeforeEnabling) { mir::udev::Monitor monitor{mir::udev::Context()}; diff --git a/tests/unit-tests/test_udev_wrapper_asan_skip.cpp b/tests/unit-tests/test_udev_wrapper_asan_skip.cpp new file mode 100644 index 00000000000..3a1c505bc3c --- /dev/null +++ b/tests/unit-tests/test_udev_wrapper_asan_skip.cpp @@ -0,0 +1,88 @@ +/* + * Copyright © Canonical Ltd. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 or 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "mir_test_framework/udev_environment.h" +#include "mir/udev/wrapper.h" +#include "mir/test/death.h" + +#include +#include +#include +#include + +namespace mtf=mir_test_framework; + +namespace +{ +bool KilledByInvalidMemoryAccess(int exit_status) +{ + return testing::KilledBySignal(SIGSEGV)(exit_status) || + testing::KilledBySignal(SIGBUS)(exit_status) || + testing::KilledBySignal(SIGABRT)(exit_status) || + // It seems that valgrind kills us with SIGKILL + testing::KilledBySignal(SIGKILL)(exit_status); +} + +/// Tests in this suite are skipped during the address sanitizer build, +/// as they purposefully do things that the address sanitizer would not like. +class UdevWrapperDeathTest : public ::testing::Test +{ +public: + mtf::UdevEnvironment udev_environment; +}; +} + +TEST_F(UdevWrapperDeathTest, DereferencingEndReturnsInvalidObject) +{ + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + udev_environment.add_device("drm", "control64D", NULL, {}, {}); + udev_environment.add_device("drm", "card1", NULL, {}, {}); + + mir::udev::Enumerator devices(std::make_shared()); + + devices.scan_devices(); + + MIR_EXPECT_EXIT((*devices.end()).subsystem(), KilledByInvalidMemoryAccess, ""); + + auto iter = devices.begin(); + + while(iter != devices.end()) + { + iter++; + } + MIR_EXPECT_EXIT((*iter).subsystem(), KilledByInvalidMemoryAccess, ""); +} + +TEST_F(UdevWrapperDeathTest, MemberDereferenceOfEndDies) +{ + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + udev_environment.add_device("drm", "control64D", NULL, {}, {}); + udev_environment.add_device("drm", "card1", NULL, {}, {}); + + mir::udev::Enumerator devices(std::make_shared()); + + devices.scan_devices(); + + MIR_EXPECT_EXIT(devices.end()->subsystem(), KilledByInvalidMemoryAccess, ""); + + auto iter = devices.begin(); + + while(iter != devices.end()) + { + iter++; + } + MIR_EXPECT_EXIT(iter->subsystem(), KilledByInvalidMemoryAccess, ""); +} \ No newline at end of file