From be4696e238a5ffc4f68814045b2ae654d2d9fb5d Mon Sep 17 00:00:00 2001 From: Ben Meadors Date: Fri, 25 Jul 2025 12:32:58 -0500 Subject: [PATCH] Revert "Suppress false positive" This reverts commit bead96eaee31f11ed631c016eb0424055e923a29. --- .github/workflows/test_native.yml | 2 - asan_suppressions.txt | 47 ------------------------ test/test_mqtt/MQTT.cpp | 8 +++- variants/native/portduino/platformio.ini | 4 +- 4 files changed, 8 insertions(+), 53 deletions(-) delete mode 100644 asan_suppressions.txt diff --git a/.github/workflows/test_native.yml b/.github/workflows/test_native.yml index a034bb106..dc05959fd 100644 --- a/.github/workflows/test_native.yml +++ b/.github/workflows/test_native.yml @@ -90,8 +90,6 @@ jobs: run: sed -i 's/-DBUILD_EPOCH=$UNIX_TIME/#-DBUILD_EPOCH=$UNIX_TIME/' platformio.ini - name: PlatformIO Tests - env: - ASAN_OPTIONS: suppressions=asan_suppressions.txt:halt_on_error=1:abort_on_error=1 run: platformio test -e coverage -v --junit-output-path testreport.xml - name: Save test results diff --git a/asan_suppressions.txt b/asan_suppressions.txt deleted file mode 100644 index c8fb221ef..000000000 --- a/asan_suppressions.txt +++ /dev/null @@ -1,47 +0,0 @@ -# AddressSanitizer suppressions file for Meshtastic firmware tests -# This file suppresses known memory leaks that are not actual bugs - -# MQTT test memory leak - ClientNotification allocations in test framework -# ======================================================================== -# -# BACKGROUND: -# The MQTT unit tests show memory leaks when AddressSanitizer is enabled. -# These are not real memory leaks in production code, but rather artifacts -# of the test framework architecture. -# -# ROOT CAUSE: -# The issue occurs because sendClientNotification() is not a virtual method -# in the base MeshService class. This means MockMeshService cannot override -# it to properly track and clean up ClientNotification allocations during tests. -# When MQTT validation fails, ClientNotification objects are allocated via -# MemoryDynamic::allocate() but cannot be intercepted by the mock framework. -# -# PRODUCTION IMPACT: -# None. This only affects unit tests, not production firmware. -# -# SOLUTION ATTEMPTED: -# Tried to override sendClientNotification in MockMeshService but failed -# because the method is not virtual in the base class. -# -# SUPPRESSIONS: -# These suppressions allow the CI tests to pass while documenting that -# the "leaks" are test artifacts, not real memory issues. -# -# TO FIX PROPERLY (future work): -# - Make sendClientNotification() virtual in MeshService base class, or -# - Refactor test framework to use dependency injection for better mocking - -# Suppress all memory leaks in MQTT test files -leak:MQTT.cpp -leak:test_mqtt - -# Suppress leaks related to ClientNotification allocation patterns -leak:ClientNotification -leak:MemoryDynamic -leak:allocFromPool -leak:Allocator - -# Suppress leaks in mock services that cannot properly clean up -leak:MockMeshService -leak:sendMqttMessageToClientProxy -leak:releaseMqttClientProxyMessageToPool diff --git a/test/test_mqtt/MQTT.cpp b/test/test_mqtt/MQTT.cpp index 723d11627..b5b09d97d 100644 --- a/test/test_mqtt/MQTT.cpp +++ b/test/test_mqtt/MQTT.cpp @@ -56,7 +56,13 @@ class MockMeshService : public MeshService messages_.emplace_back(*m); releaseMqttClientProxyMessageToPool(m); } - std::list messages_; // Messages received from the MeshService. + void sendClientNotification(meshtastic_ClientNotification *n) override + { + notifications_.emplace_back(*n); + releaseClientNotificationToPool(n); + } + std::list messages_; // Messages received from the MeshService. + std::list notifications_; // Notifications received from the MeshService. }; // Minimal NodeDB needed to return values from getMeshNode. diff --git a/variants/native/portduino/platformio.ini b/variants/native/portduino/platformio.ini index c175e7123..732b2a1d4 100644 --- a/variants/native/portduino/platformio.ini +++ b/variants/native/portduino/platformio.ini @@ -105,6 +105,4 @@ build_src_filter = ${env:native-tft.build_src_filter} [env:coverage] extends = env:native -build_flags = -lgcov --coverage -fsanitize=address ${env:native.build_flags} -test_build_src = yes -test_filter = * +build_flags = -lgcov --coverage -fprofile-abs-path -fsanitize=address ${env:native.build_flags}