From bbdd1a47af730b6e546b50499181cc7e8db38a60 Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Wed, 17 Jul 2024 15:16:32 -0400 Subject: [PATCH] asan: changing how stub input platform is allocated to avoid statics leaking into libraries that include it --- .../mir_test_framework/stub_input_platform.h | 25 +++-- .../stub_input_platform_accessor.h | 39 +++++++ tests/mir_test_framework/CMakeLists.txt | 17 ++- .../fake_input_device_impl.cpp | 10 +- tests/mir_test_framework/stub_input.cpp | 4 +- .../stub_input_platform.cpp | 66 +++-------- .../stub_input_platform_accessor.cpp | 103 ++++++++++++++++++ tests/unit-tests/CMakeLists.txt | 1 - 8 files changed, 196 insertions(+), 69 deletions(-) create mode 100644 tests/include/mir_test_framework/stub_input_platform_accessor.h create mode 100644 tests/mir_test_framework/stub_input_platform_accessor.cpp diff --git a/tests/include/mir_test_framework/stub_input_platform.h b/tests/include/mir_test_framework/stub_input_platform.h index 82af68c6e0e..7abf07c91e8 100644 --- a/tests/include/mir_test_framework/stub_input_platform.h +++ b/tests/include/mir_test_framework/stub_input_platform.h @@ -36,11 +36,22 @@ class InputDevice; } namespace mir_test_framework { + +class DeviceStore +{ +public: + virtual ~DeviceStore() = default; + virtual void foreach_device(std::function const&)> const&) = 0; + virtual void clear() = 0; +}; + class FakeInputDevice; class StubInputPlatform : public mir::input::Platform { public: - explicit StubInputPlatform(std::shared_ptr const& input_device_registry); + explicit StubInputPlatform( + std::shared_ptr const& input_device_registry, + std::shared_ptr const& device_store); ~StubInputPlatform(); std::shared_ptr dispatchable() override; @@ -49,18 +60,16 @@ class StubInputPlatform : public mir::input::Platform void pause_for_config() override; void continue_after_config() override; - static void add(std::shared_ptr const& dev); - static void remove(std::shared_ptr const& dev); - static void register_dispatchable(std::shared_ptr const& queue); - static void unregister_dispatchable(std::shared_ptr const& queue); + void add(std::shared_ptr const& dev); + void remove(std::shared_ptr const& dev); + void register_dispatchable(std::shared_ptr const& queue); + void unregister_dispatchable(std::shared_ptr const& queue); private: std::shared_ptr const platform_dispatchable; std::shared_ptr const platform_queue; std::shared_ptr const registry; - static std::atomic stub_input_platform; - static std::vector> device_store; - static std::mutex device_store_guard; + std::shared_ptr const device_store; }; } diff --git a/tests/include/mir_test_framework/stub_input_platform_accessor.h b/tests/include/mir_test_framework/stub_input_platform_accessor.h new file mode 100644 index 00000000000..062b8e90031 --- /dev/null +++ b/tests/include/mir_test_framework/stub_input_platform_accessor.h @@ -0,0 +1,39 @@ +/* + * 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 . + */ + +#ifndef MIR_TEST_FRAMEWORK_STUB_INPUT_PLATFORM_ACCESSOR_H +#define MIR_TEST_FRAMEWORK_STUB_INPUT_PLATFORM_ACCESSOR_H + +#include "stub_input_platform.h" + +namespace mir_test_framework +{ +class StubInputPlatformAccessor +{ +public: + static mir::UniqueModulePtr get(std::shared_ptr const& input_device_registry); + static void add(std::shared_ptr const& dev); + static void remove(std::shared_ptr const& dev); + static void register_dispatchable(std::shared_ptr const& queue); + static void unregister_dispatchable(std::shared_ptr const& queue); + +private: + static std::atomic stub_input_platform; + +}; +} + +#endif //MIR_TEST_FRAMEWORK_STUB_INPUT_PLATFORM_ACCESSOR_H diff --git a/tests/mir_test_framework/CMakeLists.txt b/tests/mir_test_framework/CMakeLists.txt index 64c58f6bd12..155619f2bfb 100644 --- a/tests/mir_test_framework/CMakeLists.txt +++ b/tests/mir_test_framework/CMakeLists.txt @@ -21,6 +21,17 @@ add_compile_definitions( string (REPLACE " -flto " " " CMAKE_C_FLAGS ${CMAKE_C_FLAGS}) string (REPLACE " -flto " " " CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS}) +add_library(mir-stub-input-platform OBJECT + stub_input_platform.cpp +) + +target_link_libraries(mir-stub-input-platform + PUBLIC + mirplatform + mircommon + mircore +) + add_library(mir-public-test-framework OBJECT async_server_runner.cpp command_line_server_configuration.cpp @@ -93,6 +104,7 @@ target_link_libraries(mir-umock-test-framework add_library(mir-test-framework-static STATIC $ + $ $ $ ) @@ -115,8 +127,8 @@ add_library( mir-test-input-framework OBJECT stub_input.cpp + stub_input_platform_accessor.cpp fake_input_device_impl.cpp - stub_input_platform.cpp ) target_link_libraries(mir-test-input-framework @@ -129,7 +141,8 @@ target_link_libraries(mir-test-input-framework add_library( mirplatforminputstub MODULE $ - $) + $ + $) target_link_libraries(mirplatforminputstub mircommon) set_target_properties( diff --git a/tests/mir_test_framework/fake_input_device_impl.cpp b/tests/mir_test_framework/fake_input_device_impl.cpp index 3c087c81615..e9ae1b9cb60 100644 --- a/tests/mir_test_framework/fake_input_device_impl.cpp +++ b/tests/mir_test_framework/fake_input_device_impl.cpp @@ -15,7 +15,7 @@ */ #include "fake_input_device_impl.h" -#include "mir_test_framework/stub_input_platform.h" +#include "mir_test_framework/stub_input_platform_accessor.h" #include "mir/input/input_device.h" #include "mir/input/input_device_info.h" @@ -45,12 +45,12 @@ namespace synthesis = mir::input::synthesis; mtf::FakeInputDeviceImpl::FakeInputDeviceImpl(mi::InputDeviceInfo const& info) : queue{std::make_shared()}, device{std::make_shared(info, queue)} { - mtf::StubInputPlatform::add(device); + mtf::StubInputPlatformAccessor::add(device); } void mtf::FakeInputDeviceImpl::emit_device_removal() { - mtf::StubInputPlatform::remove(device); + mtf::StubInputPlatformAccessor::remove(device); } void mtf::FakeInputDeviceImpl::emit_runtime_error() @@ -328,14 +328,14 @@ void mtf::FakeInputDeviceImpl::InputDevice::start(mi::InputSink* destination, mi { sink = destination; builder = event_builder; - mtf::StubInputPlatform::register_dispatchable(queue); + mtf::StubInputPlatformAccessor::register_dispatchable(queue); } void mtf::FakeInputDeviceImpl::InputDevice::stop() { sink = nullptr; builder = nullptr; - mtf::StubInputPlatform::unregister_dispatchable(queue); + mtf::StubInputPlatformAccessor::unregister_dispatchable(queue); } mi::OutputInfo mtf::FakeInputDeviceImpl::InputDevice::get_output_info() const diff --git a/tests/mir_test_framework/stub_input.cpp b/tests/mir_test_framework/stub_input.cpp index a2ee65a80ee..2b6ae54ff2e 100644 --- a/tests/mir_test_framework/stub_input.cpp +++ b/tests/mir_test_framework/stub_input.cpp @@ -14,7 +14,7 @@ * along with this program. If not, see . */ -#include "mir_test_framework/stub_input_platform.h" +#include "mir_test_framework/stub_input_platform_accessor.h" #include "fake_input_device_impl.h" #include "mir/module_properties.h" #include "mir/assert_module_entry_point.h" @@ -33,7 +33,7 @@ mir::UniqueModulePtr create_input_platform( std::shared_ptr const& /*report*/) { mir::assert_entry_point_signature(&create_input_platform); - return mir::make_module_ptr(input_device_registry); + return mtf::StubInputPlatformAccessor::get(input_device_registry); } void add_input_platform_options( diff --git a/tests/mir_test_framework/stub_input_platform.cpp b/tests/mir_test_framework/stub_input_platform.cpp index eb6245c7889..868fbe1448f 100644 --- a/tests/mir_test_framework/stub_input_platform.cpp +++ b/tests/mir_test_framework/stub_input_platform.cpp @@ -27,31 +27,29 @@ namespace mtf = mir_test_framework; namespace mi = mir::input; mtf::StubInputPlatform::StubInputPlatform( - std::shared_ptr const& input_device_registry) + std::shared_ptr const& input_device_registry, + std::shared_ptr const& device_store) : platform_dispatchable{std::make_shared()}, platform_queue{std::make_shared()}, - registry(input_device_registry) + registry(input_device_registry), + device_store(device_store) { - stub_input_platform = this; platform_dispatchable->add_watch(platform_queue); } mtf::StubInputPlatform::~StubInputPlatform() { - std::lock_guard lk{device_store_guard}; - device_store.clear(); - stub_input_platform = nullptr; + device_store->clear(); } void mtf::StubInputPlatform::start() { - std::lock_guard lk{device_store_guard}; - for (auto const& dev : device_store) + device_store->foreach_device([&](auto const& dev) { auto device = dev.lock(); if (device) registry->add_device(device); - } + }); } std::shared_ptr mtf::StubInputPlatform::dispatchable() @@ -61,13 +59,12 @@ std::shared_ptr mtf::StubInputPlatform::dispatchabl void mtf::StubInputPlatform::stop() { - std::lock_guard lk{device_store_guard}; - for (auto const& dev : device_store) + device_store->foreach_device([&](auto const& dev) { auto device = dev.lock(); if (device) registry->remove_device(device); - } + }); } void mtf::StubInputPlatform::pause_for_config() @@ -80,16 +77,8 @@ void mtf::StubInputPlatform::continue_after_config() void mtf::StubInputPlatform::add(std::shared_ptr const& dev) { - auto input_platform = stub_input_platform.load(); - if (!input_platform) - { - std::lock_guard lk{device_store_guard}; - device_store.push_back(dev); - return; - } - - input_platform->platform_queue->enqueue( - [registry=input_platform->registry,dev] + platform_queue->enqueue( + [registry=registry,dev] { registry->add_device(dev); }); @@ -97,21 +86,8 @@ void mtf::StubInputPlatform::add(std::shared_ptr const& void mtf::StubInputPlatform::remove(std::shared_ptr const& dev) { - auto input_platform = stub_input_platform.load(); - if (!input_platform) - BOOST_THROW_EXCEPTION(std::runtime_error("No stub input platform available")); - - std::lock_guard lk{device_store_guard}; - device_store.erase( - std::remove_if(begin(device_store), - end(device_store), - [dev](auto weak_dev) - { - return (weak_dev.lock() == dev); - }), - end(device_store)); - input_platform->platform_queue->enqueue( - [ registry = input_platform->registry, dev ] + platform_queue->enqueue( + [ registry = registry, dev ] { registry->remove_device(dev); }); @@ -119,22 +95,10 @@ void mtf::StubInputPlatform::remove(std::shared_ptr con void mtf::StubInputPlatform::register_dispatchable(std::shared_ptr const& queue) { - auto input_platform = stub_input_platform.load(); - if (!input_platform) - BOOST_THROW_EXCEPTION(std::runtime_error("No stub input platform available")); - - input_platform->platform_dispatchable->add_watch(queue); + platform_dispatchable->add_watch(queue); } void mtf::StubInputPlatform::unregister_dispatchable(std::shared_ptr const& queue) { - auto input_platform = stub_input_platform.load(); - if (!input_platform) - BOOST_THROW_EXCEPTION(std::runtime_error("No stub input platform available")); - - input_platform->platform_dispatchable->remove_watch(queue); + platform_dispatchable->remove_watch(queue); } - -std::atomic mtf::StubInputPlatform::stub_input_platform{nullptr}; -std::vector> mtf::StubInputPlatform::device_store; -std::mutex mtf::StubInputPlatform::device_store_guard; diff --git a/tests/mir_test_framework/stub_input_platform_accessor.cpp b/tests/mir_test_framework/stub_input_platform_accessor.cpp new file mode 100644 index 00000000000..467d6359bf0 --- /dev/null +++ b/tests/mir_test_framework/stub_input_platform_accessor.cpp @@ -0,0 +1,103 @@ +/* + * 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/stub_input_platform_accessor.h" + +namespace mir_test_framework +{ +class StaticDeviceStore : public mir_test_framework::DeviceStore +{ +public: + void foreach_device(std::function const&)> const& f) override + { + for (auto const& dev : device_store) + f(dev); + } + + void clear() override + { + std::lock_guard lk{device_store_guard}; + device_store.clear(); + } + + static std::mutex device_store_guard; + static std::vector> device_store; +}; + +std::vector> StaticDeviceStore::device_store; +std::mutex StaticDeviceStore::device_store_guard; +} + +mir::UniqueModulePtr mir_test_framework::StubInputPlatformAccessor::get( + std::shared_ptr const& input_device_registry) +{ + auto ptr = mir::make_module_ptr( + input_device_registry, std::make_shared()); + stub_input_platform = ptr.get(); + return ptr; +} + + +void mir_test_framework::StubInputPlatformAccessor::add(std::shared_ptr const& dev) +{ + auto input_platform = stub_input_platform.load(); + if (!input_platform) + { + std::lock_guard lk{StaticDeviceStore::device_store_guard}; + StaticDeviceStore::device_store.push_back(dev); + return; + } + + input_platform->add(dev); +} + +void mir_test_framework::StubInputPlatformAccessor::remove(std::shared_ptr const& dev) +{ + auto input_platform = stub_input_platform.load(); + if (!input_platform) + BOOST_THROW_EXCEPTION(std::runtime_error("No stub input platform available")); + + std::lock_guard lk{StaticDeviceStore::device_store_guard}; + StaticDeviceStore::device_store.erase( + std::remove_if(begin(StaticDeviceStore::device_store), + end(StaticDeviceStore::device_store), + [dev](auto weak_dev) + { + return (weak_dev.lock() == dev); + }), + end(StaticDeviceStore::device_store)); + input_platform->remove(dev); +} + +void mir_test_framework::StubInputPlatformAccessor::register_dispatchable(std::shared_ptr const& queue) +{ + auto input_platform = stub_input_platform.load(); + if (!input_platform) + BOOST_THROW_EXCEPTION(std::runtime_error("No stub input platform available")); + + input_platform->register_dispatchable(queue); +} + +void mir_test_framework::StubInputPlatformAccessor::unregister_dispatchable(std::shared_ptr const& queue) +{ + auto input_platform = stub_input_platform.load(); + if (!input_platform) + BOOST_THROW_EXCEPTION(std::runtime_error("No stub input platform available")); + + input_platform->unregister_dispatchable(queue); +} + +std::atomic mir_test_framework::StubInputPlatformAccessor::stub_input_platform{nullptr}; \ No newline at end of file diff --git a/tests/unit-tests/CMakeLists.txt b/tests/unit-tests/CMakeLists.txt index 4509b601476..fd641868536 100644 --- a/tests/unit-tests/CMakeLists.txt +++ b/tests/unit-tests/CMakeLists.txt @@ -183,7 +183,6 @@ target_link_libraries( mir_add_wrapped_executable(mir_umock_unit_tests NOINSTALL ${UMOCK_UNIT_TEST_SOURCES} $ - $ ${MIR_SERVER_OBJECTS} ${MIR_PLATFORM_OBJECTS} )