diff --git a/src/svxlink/modules/frn/CMakeLists.txt b/src/svxlink/modules/frn/CMakeLists.txt index 472e2ecd8..7161fc5b5 100644 --- a/src/svxlink/modules/frn/CMakeLists.txt +++ b/src/svxlink/modules/frn/CMakeLists.txt @@ -27,3 +27,9 @@ install(FILES ${MODNAME}.tcl DESTINATION ${SVX_SHARE_INSTALL_DIR}/events.d) install_if_not_exists(Module${MODNAME}.conf ${SVX_SYSCONF_INSTALL_DIR}/svxlink.d ) + +# Unit tests: every *Test.cpp here is auto-registered once the CTest +# suite (SvxLinkAddTests) is present; harmless without it. +if(COMMAND svxlink_add_tests) + svxlink_add_tests() +endif() diff --git a/src/svxlink/modules/frn/ModuleFrn.cpp b/src/svxlink/modules/frn/ModuleFrn.cpp index ed27c58b5..1d6ba9f84 100644 --- a/src/svxlink/modules/frn/ModuleFrn.cpp +++ b/src/svxlink/modules/frn/ModuleFrn.cpp @@ -129,6 +129,7 @@ ModuleFrn::ModuleFrn(void *dl_handle, Logic *logic, const string& cfg_name) , audio_valve(0) , audio_splitter(0) , audio_selector(0) + , audio_fifo(0) { cout << "\tModule Frn v" MODULE_FRN_VERSION " starting...\n"; @@ -229,6 +230,7 @@ bool ModuleFrn::initialize(void) if (!qso->initOk()) { delete qso; + qso = 0; cerr << "*** ERROR: Creation of Qso object failed\n"; return false; } @@ -241,10 +243,19 @@ bool ModuleFrn::initialize(void) void ModuleFrn::moduleCleanup() { AudioSource::clearHandler(); - audio_fifo->unregisterSource(); + if (audio_fifo != 0) + { + audio_fifo->unregisterSource(); + } - audio_splitter->removeSink(qso); - audio_valve->unregisterSink(); + if (audio_splitter != 0) + { + audio_splitter->removeSink(qso); + } + if (audio_valve != 0) + { + audio_valve->unregisterSink(); + } AudioSink::clearHandler(); delete qso; diff --git a/src/svxlink/modules/frn/ModuleFrnTest.cpp b/src/svxlink/modules/frn/ModuleFrnTest.cpp new file mode 100644 index 000000000..f84f8b712 --- /dev/null +++ b/src/svxlink/modules/frn/ModuleFrnTest.cpp @@ -0,0 +1,95 @@ +/** +@file ModuleFrnTest.cpp +@brief Unit test for ModuleFrn init-failure pointer lifecycle +@author Mark Rose +@date 2026-07-11 + +ModuleFrn::initialize() only assigns audio_fifo, audio_splitter and +audio_valve after the base Module::initialize() call succeeds. If +initialization fails (or is never called) before those members are +assigned, the ModuleFrn destructor still unconditionally calls +moduleCleanup(), which used to dereference audio_fifo unconditionally. +Because audio_fifo was not part of the constructor initializer list, it +held whatever garbage bits were already present in the object's storage, +so moduleCleanup() dereferenced an uninitialized/garbage pointer and +crashed. + +This test places a ModuleFrn instance into memory that has been poisoned +with a non-canonical bit pattern *before* construction -- exactly the +kind of "garbage" an uninitialized pointer member would contain -- then +destroys the object without ever calling initialize(). With the fix, +the constructor explicitly nulls audio_fifo and moduleCleanup() guards +every audio pipeline pointer with a null check, so destruction completes +cleanly. Without the fix, moduleCleanup() dereferences the poisoned +audio_fifo pointer and the process crashes (a non-canonical x86_64 +address reliably faults on first access), which CTest observes as a +failing (non-zero/signal-terminated) test. + +\verbatim +SvxLink - A Multi Purpose Voice Services System for Ham Radio Use +Copyright (C) 2026 Tobias Blomberg / SM0SVX + +This program is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 2 of the License, or +(at your option) any later version. +\endverbatim +*/ + +#include +#include +#include + +#include "ModuleFrn.h" + +using namespace std; + +namespace { + +int failures = 0; + +void check(bool cond, const string& msg) +{ + cout << (cond ? " ok " : " FAIL ") << msg << endl; + if (!cond) + { + ++failures; + } +} + +} /* anonymous namespace */ + + +int main(void) +{ + // Poison the storage for a ModuleFrn instance with a non-canonical + // x86_64 bit pattern before construction. This stands in for the + // uninitialized garbage that a pointer member would contain if it is + // never assigned by the constructor -- exactly the pre-fix bug for + // audio_fifo. Any real dereference of this pattern as a pointer + // reliably faults. + alignas(ModuleFrn) unsigned char storage[sizeof(ModuleFrn)]; + memset(storage, 0xAB, sizeof(storage)); + + // Construct in place. dl_handle and logic are both unused by the + // constructor and by moduleCleanup(), so passing null is safe as + // long as initialize() (which does dereference logic()) is never + // called -- exactly the failure mode this test targets: initialize() + // fails (or, as here, is never reached) before the audio pipeline + // members are assigned. + ModuleFrn *mod = new (storage) ModuleFrn(0, 0, "frn"); + + // Destroy without ever calling initialize(). This exercises + // moduleCleanup() with audio_fifo (and friends) left at whatever the + // constructor set them to. Pre-fix, audio_fifo is left as the + // poisoned garbage above and moduleCleanup() crashes dereferencing + // it. Post-fix, audio_fifo is explicitly nulled and every dereference + // in moduleCleanup() is null-guarded, so this returns cleanly. + mod->~ModuleFrn(); + + check(true, "destructor completed without dereferencing an " + "uninitialized audio_fifo pointer"); + + cout << (failures == 0 ? "ALL TESTS PASSED" : "TESTS FAILED") << endl; + return failures != 0; +} /* main */ diff --git a/src/svxlink/modules/frn/ModuleFrnTest.deps.cmake b/src/svxlink/modules/frn/ModuleFrnTest.deps.cmake new file mode 100644 index 000000000..3e62ce5bd --- /dev/null +++ b/src/svxlink/modules/frn/ModuleFrnTest.deps.cmake @@ -0,0 +1,19 @@ +# Extra sources/libs needed to link ModuleFrnTest. +# +# ModuleFrn.cpp requires QsoFrn.cpp/Utils.cpp (its module siblings) and +# Module.cpp (the svxlink core base class it derives from). Module.cpp +# in turn makes a few direct (non-virtual) calls into Logic:: and touches +# the LinkManager singleton pointer; ModuleFrnTestLinkStubs.cpp provides +# trivial link-only stand-ins for those so the real Logic/LinkManager +# subsystem (and the Async event loop / application wiring it needs) +# does not have to be pulled in for this standalone test. +set(ModuleFrnTest_EXTRA_SRCS + ModuleFrn.cpp + QsoFrn.cpp + Utils.cpp + ModuleFrnTestLinkStubs.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/../../svxlink/Module.cpp + ) +set(ModuleFrnTest_EXTRA_LIBS + asynccpp asynccore asyncaudio svxmisc ${GSM_LIBRARY} + ) diff --git a/src/svxlink/modules/frn/ModuleFrnTestLinkStubs.cpp b/src/svxlink/modules/frn/ModuleFrnTestLinkStubs.cpp new file mode 100644 index 000000000..f94642b82 --- /dev/null +++ b/src/svxlink/modules/frn/ModuleFrnTestLinkStubs.cpp @@ -0,0 +1,59 @@ +/** +@file ModuleFrnTestLinkStubs.cpp +@brief Link-only stand-ins for symbols ModuleFrnTest never executes +@author Mark Rose +@date 2026-07-11 + +ModuleFrnTest.cpp constructs and destroys a ModuleFrn without ever +calling initialize(), so it never exercises any Logic:: or LinkManager +functionality. However, Module.cpp (compiled in as part of the module +under test, since ModuleFrn derives from Module) contains a handful of +member functions -- reachable only from code paths this test never +runs -- that make *direct* (non-virtual) calls into Logic:: methods and +touch the LinkManager singleton pointer. Because those functions are +part of the same translation unit that is linked into this test binary, +the linker requires those symbols to be resolvable even though they are +never invoked at runtime by this test. + +Pulling in the real Logic.cpp/LinkManager.cpp to satisfy this would drag +in the full logic-core/event-handling/link-management subsystem, which +needs a live Async event loop and application wiring -- exactly what this +small unit test intentionally avoids. These stubs instead provide trivial +definitions for just the non-virtual symbols Module.cpp references, so +the parts of ModuleFrn actually under test (the constructor/destructor +pointer lifecycle) can be linked and run standalone. None of these bodies +are ever executed by ModuleFrnTest. + +\verbatim +SvxLink - A Multi Purpose Voice Services System for Ham Radio Use +Copyright (C) 2026 Tobias Blomberg / SM0SVX + +This program is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 2 of the License, or +(at your option) any later version. +\endverbatim +*/ + +#include "Logic.h" +#include "LinkManager.h" + +using namespace std; + + + // Non-virtual Logic:: member functions that Module.cpp calls directly + // (not through a vtable). Never invoked by this test -- ModuleFrn is + // destroyed before initialize() is ever called -- but the symbols must + // exist for the link to succeed. +void Logic::setEventVariable(const string&, const string&) {} +void Logic::sendDtmf(const string&) {} +Module *Logic::findModule(int) { return 0; } +bool Logic::isWritingMessage(void) { return false; } + + // Storage for the LinkManager singleton pointer. Module::activate()/ + // deactivate() check LinkManager::hasInstance() (inline, reads this + // pointer) before touching the singleton; leaving it null means that + // guard is always false and the singleton is never dereferenced. +LinkManager *LinkManager::_instance = 0; + +void LinkManager::setLogicMute(const LogicBase*, bool) {}