ci: enable AddressSanitizer

Author: mattkae

.github/workflows/spread.yml

@@ -30,9 +30,7 @@ jobs:
         set -euo pipefail
 
         if ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }}; then
-          TASKS='"lxd:ubuntu-24.04:spread/build/ubuntu:asan"
-                 "lxd:ubuntu-24.04:spread/build/ubuntu:tsan"
-                 "lxd:ubuntu-24.04:spread/build/ubuntu:asan_clang"
+          TASKS='"lxd:ubuntu-24.04:spread/build/ubuntu:tsan"
                  "lxd:ubuntu-24.04:spread/build/ubuntu:tsan_clang"'
         fi
 
@@ -49,6 +47,8 @@ jobs:
                 "lxd:fedora-rawhide:spread/build/fedora:amd64"
                 "lxd:ubuntu-24.04:spread/build/sbuild:ubuntu_devel"
                 "lxd:ubuntu-24.04:spread/build/sbuild:ubuntu_proposed"
+                "lxd:ubuntu-24.04:spread/build/ubuntu:asan"
+                "lxd:ubuntu-24.04:spread/build/ubuntu:asan_clang"
                 "lxd:ubuntu-24.04:spread/build/ubuntu:ubsan"
                 "lxd:ubuntu-24.04:spread/build/ubuntu:ubsan_clang"'
 

spread.yaml

@@ -37,8 +37,8 @@ environment:
     GCC_VERSION:
     GCC_VERSION/ubuntu_devel: 14
     DEB_BUILD_EXTRA:
-    DEB_BUILD_EXTRA/ubsan,ubsan_clang: nostrip
-    DEB_BUILD_EXTRA/asan,asan_clang,tsan,tsan_clang: nostrip nocheck
+    DEB_BUILD_EXTRA/asan,asan_clang,ubsan,ubsan_clang: nostrip optimize=-lto
+    DEB_BUILD_EXTRA/tsan,tsan_clang: nostrip nocheck
     DEBOOTSTRAP_OPTS:
     DEBOOTSTRAP_OPTS/debian_sid,ubuntu_devel: --no-merged-usr
     CTEST_OUTPUT_ON_FAILURE: 1

spread/build/ubuntu/task.yaml

@@ -53,6 +53,6 @@ execute: |
     echo "OVERRIDE_CONFIGURE_OPTIONS += -DCMAKE_CXX_COMPILER_LAUNCHER=ccache" >> debian/opts.mk
 
     # build and run tests
-    UBSAN_OPTIONS=halt_on_error=1 debian/rules build
+    UBSAN_OPTIONS=halt_on_error=1 ASAN_OPTIONS=detect_leaks=0,verify_asan_link_order=0,detect_odr_violation=0 debian/rules build
 
     ccache --show-stats --zero-stats > ${CCACHE_DIR}/ccache.stats

src/include/server/mir/scene/text_input_hub.h

@@ -23,6 +23,8 @@
 #include <memory>
 #include <optional>
 #include <string>
+#include <wayland-server.h>
+#include <algorithm>
 
 struct wl_array;
 
@@ -97,6 +99,39 @@ struct TextInputState
     std::optional<TextInputContentPurpose> content_purpose;
 };
 
+class CopyableWlArray
+{
+public:
+    explicit CopyableWlArray(wl_array const* in_data)
+    {
+        wl_array_init(&data_);
+        wl_array_copy(&data_, const_cast<wl_array*>(in_data));
+    }
+    
+    CopyableWlArray(CopyableWlArray const& other)
+        : CopyableWlArray{&other.data_}
+    {
+    }
+
+    CopyableWlArray& operator=(CopyableWlArray other)
+    {
+        std::swap(data_, other.data_);
+        return *this;
+    }
+
+    ~CopyableWlArray()
+    {
+        wl_array_release(&data_);
+    }
+
+    [[nodiscard]] wl_array* data() const
+    {
+        return const_cast<wl_array*>(&data_);
+    }
+private:
+    wl_array data_;
+};
+
 /// See text-input-unstable-v3.xml for details
 struct TextInputChange
 {
@@ -142,7 +177,7 @@ struct TextInputChange
     std::optional<TextInputKeySym> keysym;
 
     /// \remark Defined for text input v1 and v2, not v3.
-    std::optional<wl_array*> modifier_map;
+    std::optional<CopyableWlArray> modifier_map;
 
     /// \remark Defined for text input v1 and v2, not v3.
     std::optional<CursorPosition> cursor_position;

src/server/frontend_wayland/input_method_v1.cpp

@@ -327,7 +327,7 @@ class mf::InputMethodV1::Instance : wayland::InputMethodV1
 
         void modifiers_map(struct wl_array *map) override
         {
-            change.pending_change.modifier_map = map;
+            change.pending_change.modifier_map = scene::CopyableWlArray(map);
             change.waiting_status = InputMethodV1ChangeWaitingStatus::none;
         }
 

src/server/frontend_wayland/text_input_v1.cpp

@@ -289,7 +289,7 @@ void TextInputV1::send_text_change(ms::TextInputChange const& change)
     }
     if (change.modifier_map)
     {
-        send_modifiers_map_event(change.modifier_map.value());
+        send_modifiers_map_event(change.modifier_map.value().data());
     }
     if (change.direction)
     {

src/server/frontend_wayland/text_input_v2.cpp

@@ -303,7 +303,7 @@ void mf::TextInputV2::send_text_change(ms::TextInputChange const& change)
     }
     if (change.modifier_map)
     {
-        send_modifiers_map_event(change.modifier_map.value());
+        send_modifiers_map_event(change.modifier_map.value().data());
     }
     if (change.direction)
     {

src/server/symbols.map

@@ -1344,12 +1344,12 @@ local: *;
 MIR_SERVER_INTERNAL_2.18 {
 global:
   extern "C++" {
-    mir::DefaultServerConfiguration::the_led_observer_registrar*;
     mir::DecorationStrategy::?DecorationStrategy*;
     mir::DecorationStrategy::DecorationStrategy*;
     mir::DecorationStrategy::operator*;
     mir::DefaultServerConfiguration::set_the_decoration_strategy*;
     mir::DefaultServerConfiguration::the_decoration_strategy*;
+    mir::DefaultServerConfiguration::the_led_observer_registrar*;
     mir::Server::set_the_decoration_strategy*;
     mir::Server::the_decoration_strategy*;
     mir::Server::the_idle_handler*;
@@ -1367,8 +1367,17 @@ global:
     mir::input::receiver::XKBMapperRegistrar::set_keymap_for_device*;
     mir::input::receiver::XKBMapperRegistrar::unregister_interest*;
     mir::input::receiver::XKBMapperRegistrar::xkb_modifiers*;
+    mir::scene::CopyableWlArray::?CopyableWlArray*;
+    mir::scene::CopyableWlArray::CopyableWlArray*;
+    mir::scene::CopyableWlArray::data*;
+    mir::scene::CopyableWlArray::operator*;
+    mir::scene::TextInputChange::?TextInputChange*;
+    mir::scene::TextInputChange::operator*;
     mir::shell::IdleHandlerObserver::?IdleHandlerObserver*;
     mir::shell::IdleHandlerObserver::IdleHandlerObserver*;
+    non-virtual?thunk?to?mir::DecorationStrategy::?DecorationStrategy*;
+    non-virtual?thunk?to?mir::DefaultServerConfiguration::set_the_decoration_strategy*;
+    non-virtual?thunk?to?mir::DefaultServerConfiguration::the_decoration_strategy*;
     non-virtual?thunk?to?mir::DefaultServerConfiguration::the_led_observer_registrar*;
     non-virtual?thunk?to?mir::input::receiver::XKBMapperRegistrar::clear_all_keymaps*;
     non-virtual?thunk?to?mir::input::receiver::XKBMapperRegistrar::clear_keymap_for_device*;
@@ -1382,19 +1391,17 @@ global:
     non-virtual?thunk?to?mir::input::receiver::XKBMapperRegistrar::unregister_interest*;
     non-virtual?thunk?to?mir::input::receiver::XKBMapperRegistrar::xkb_modifiers*;
     non-virtual?thunk?to?mir::shell::IdleHandlerObserver::?IdleHandlerObserver*;
-    typeinfo?for?mir::input::receiver::XKBMapperRegistrar;
-    typeinfo?for?mir::shell::IdleHandlerObserver;
-    vtable?for?mir::input::receiver::XKBMapperRegistrar;
-    vtable?for?mir::shell::IdleHandlerObserver;
-    non-virtual?thunk?to?mir::DecorationStrategy::?DecorationStrategy*;
-    non-virtual?thunk?to?mir::DefaultServerConfiguration::set_the_decoration_strategy*;
-    non-virtual?thunk?to?mir::DefaultServerConfiguration::the_decoration_strategy*;
     non-virtual?thunk?to?mir::shell::IdleHandlerObserver::?IdleHandlerObserver*;
     typeinfo?for?mir::DecorationStrategy;
+    typeinfo?for?mir::input::receiver::XKBMapperRegistrar;
+    typeinfo?for?mir::scene::CopyableWlArray;
+    typeinfo?for?mir::shell::IdleHandlerObserver;
     typeinfo?for?mir::shell::IdleHandlerObserver;
     virtual?thunk?to?mir::DefaultServerConfiguration::set_the_decoration_strategy*;
     virtual?thunk?to?mir::DefaultServerConfiguration::the_decoration_strategy*;
     vtable?for?mir::DecorationStrategy;
+    vtable?for?mir::input::receiver::XKBMapperRegistrar;
+    vtable?for?mir::shell::IdleHandlerObserver;
     vtable?for?mir::shell::IdleHandlerObserver;
   };
 } MIR_SERVER_INTERNAL_2.17;

tests/include/mir/test/barrier.h

@@ -38,7 +38,7 @@ class Barrier
     void ready()
     {
         std::unique_lock lock(mutex);
-        if (--wait_threads)
+        if ((--wait_threads) > 0)
         {
             if (!cv.wait_for(lock, std::chrono::minutes(1), [&]{ return wait_threads == 0; }))
                 throw std::runtime_error("Timeout");

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<void(std::weak_ptr<mir::input::InputDevice> const&)> const&) = 0;
+    virtual void clear() = 0;
+};
+
 class FakeInputDevice;
 class StubInputPlatform : public mir::input::Platform
 {
 public:
-    explicit StubInputPlatform(std::shared_ptr<mir::input::InputDeviceRegistry> const& input_device_registry);
+    explicit StubInputPlatform(
+        std::shared_ptr<mir::input::InputDeviceRegistry> const& input_device_registry,
+        std::shared_ptr<DeviceStore> const& device_store);
     ~StubInputPlatform();
 
     std::shared_ptr<mir::dispatch::Dispatchable> 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<mir::input::InputDevice> const& dev);
-    static void remove(std::shared_ptr<mir::input::InputDevice> const& dev);
-    static void register_dispatchable(std::shared_ptr<mir::dispatch::Dispatchable> const& queue);
-    static void unregister_dispatchable(std::shared_ptr<mir::dispatch::Dispatchable> const& queue);
+    void add(std::shared_ptr<mir::input::InputDevice> const& dev);
+    void remove(std::shared_ptr<mir::input::InputDevice> const& dev);
+    void register_dispatchable(std::shared_ptr<mir::dispatch::Dispatchable> const& queue);
+    void unregister_dispatchable(std::shared_ptr<mir::dispatch::Dispatchable> const& queue);
 
 private:
     std::shared_ptr<mir::dispatch::MultiplexingDispatchable> const platform_dispatchable;
     std::shared_ptr<mir::dispatch::ActionQueue> const platform_queue;
     std::shared_ptr<mir::input::InputDeviceRegistry> const registry;
-    static std::atomic<StubInputPlatform*> stub_input_platform;
-    static std::vector<std::weak_ptr<mir::input::InputDevice>> device_store;
-    static std::mutex device_store_guard;
+    std::shared_ptr<DeviceStore> const device_store;
 };
 
 }

tests/include/mir_test_framework/stub_input_platform_accessor.h

@@ -0,0 +1,40 @@
+/*
+ * 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 <http://www.gnu.org/licenses/>.
+ */
+
+#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<mir::input::Platform> get(std::shared_ptr<mir::input::InputDeviceRegistry> const& input_device_registry);
+    static void add(std::shared_ptr<mir::input::InputDevice> const& dev);
+    static void remove(std::shared_ptr<mir::input::InputDevice> const& dev);
+    static void register_dispatchable(std::shared_ptr<mir::dispatch::Dispatchable> const& queue);
+    static void unregister_dispatchable(std::shared_ptr<mir::dispatch::Dispatchable> const& queue);
+    static void clear();
+
+private:
+    static std::atomic<StubInputPlatform*> stub_input_platform;
+
+};
+}
+
+#endif //MIR_TEST_FRAMEWORK_STUB_INPUT_PLATFORM_ACCESSOR_H

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
   $<TARGET_OBJECTS:mir-public-test-framework>
+  $<TARGET_OBJECTS:mir-stub-input-platform>
   $<TARGET_OBJECTS:mir-protected-test-framework>
   $<TARGET_OBJECTS:mir-public-test>
 )
@@ -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_OBJECTS:mir-test-input-framework>
-  $<TARGET_OBJECTS:mirevdevutilsobjects>)
+  $<TARGET_OBJECTS:mirevdevutilsobjects>
+  $<TARGET_OBJECTS:mir-stub-input-platform>)
 target_link_libraries(mirplatforminputstub mircommon)
 
 set_target_properties(

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<md::ActionQueue>()}, device{std::make_shared<InputDevice>(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

tests/mir_test_framework/stub_input.cpp

@@ -14,7 +14,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#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<mi::Platform> create_input_platform(
     std::shared_ptr<mi::InputReport> const& /*report*/)
 {
     mir::assert_entry_point_signature<mi::CreatePlatform>(&create_input_platform);
-    return mir::make_module_ptr<mtf::StubInputPlatform>(input_device_registry);
+    return mtf::StubInputPlatformAccessor::get(input_device_registry);
 }
 
 void add_input_platform_options(