Files
firmware/test/TestUtil.cpp
T
Jonathan BennettandClaude Opus 5 e691bd3790 Claude/dmshell lock (#12024)
* Lock: give Portduino a real mutex instead of the empty fallback

Lock.cpp has a FreeRTOS implementation and an empty one, and Portduino takes
the empty one: every lock() and unlock() on a Linux build is a no-op, so
concurrency::Lock protects nothing there. TrafficManagementModule's cacheLock
and SPILock are both built on it, and native meshtasticd runs the radio and the
API on separate threads.

Add a pthread implementation under ARCH_PORTDUINO. The timed lock(uint32_t)
blocks rather than returning early, because there is no portable timed
pthread_mutex_lock across Linux and macOS and returning true without acquiring
would leave a caller such as SPILock unlocking a mutex it never took. Targets
that have neither FreeRTOS nor pthreads, such as STM32WL, keep the existing
empty implementation byte for byte.

Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REkPVFh6kvG4AZJ5A8AtM9

* test: create spiLock in the shared setup, which NodeDB needs and no test had

Making Portduino's Lock real turns a latent null dereference into a crash. spiLock
is a bare pointer that initSPI() fills in, and only main.cpp calls that, so in a
test binary it stays null. NodeDB's constructor reaches it through loadFromDisk(),
and while Lock::lock() was an empty function the call never touched `this`, so
23 suites have been calling a method on a null pointer and getting away with it.
With a pthread mutex behind it the same call reads through the null pointer and
takes SIGSEGV at offset 0x10, which is what test_phone_api_config_dump,
test_muted_source, test_nodeinfo_send_window and test_module_config hit.

Create it once in initializeTestEnvironment(), which every affected suite calls as
the first statement of setup(), before any of them constructs a NodeDB. The guard
is the idiom test_xmodem and test_nodedb_identity_hygiene already use; theirs stay
correct and become no-ops. test_safefile called initSPI() bare right after the
harness, which would now trip its assert, so that call goes away.

No firmware behaviour changes: main.cpp still calls initSPI() exactly once, and
nothing outside the test harness is touched.

Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REkPVFh6kvG4AZJ5A8AtM9

* test: create cryptLock where a suite reaches it without building a Router

Second instance of the same latent null dereference the previous commit fixed
for spiLock. cryptLock is a bare pointer that Router's constructor creates
(Router.cpp:246); AdminModule::setPassKey takes a LockGuard on it, and a suite
that exercises an admin path without standing up a Router leaves it null. While
Lock::lock() was empty on Portduino the guard never touched `this`; with a
pthread mutex it reads through null, which is test_tak_config's SIGSEGV in
handleGetModuleConfig.

It cannot go in initializeTestEnvironment() the way spiLock did, because Router
asserts cryptLock is unset before allocating its own, so creating it for every
suite would break the ones that do build a Router. It is a named helper instead,
testEnsureCryptLock(), called by the six suites that reach a cryptLock path with
no Router: test_ack_proof, test_admin_session_repro, test_fuzz_packets,
test_hop_scaling, test_module_config and test_tak_config. The three that define
setup() twice behind a PKI #if get the call only in the branch that compiles the
tests in.

No firmware behaviour changes; nothing outside test/ is touched.

Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REkPVFh6kvG4AZJ5A8AtM9

* test: create cryptLock in the harness instead of chasing suites

Router's constructor asserted cryptLock was unset, then allocated it. That
assert is why ten suites carry a mock-router destructor whose only job is to
delete the global and null it so the next router can be built. While
Lock::lock() was an empty function on Portduino a null cryptLock cost nothing,
so those null windows were invisible; with a real mutex, anything reaching
perhapsDecode() or the ack-proof paths after one of those destructors runs
dereferences null.

The fix is the idiom already on the next line of the same constructor, which
routingAuthCacheLock has used all along: reuse the lock if one exists. Nothing
in src/ ever deleted cryptLock, so a Router that finds one is finding the
process's only one. initializeTestEnvironment() can then create it for every
suite, the way it now does for spiLock, and the ten teardowns and the
per-suite helper from the previous commit all go away.

Replaces the six testEnsureCryptLock() call sites with one creation point, and
removes the null windows in test_admin_radio, test_mesh_beacon,
test_mesh_module, test_mqtt, test_nexthop_routing, test_nodeinfo_send_window,
test_traffic_management, test_event_channel_phone_api,
test_event_channel_router and test_phone_api_config_dump.

Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REkPVFh6kvG4AZJ5A8AtM9

---------

Co-authored-by: Claude <noreply@anthropic.com>
2026-10-01 21:33:14 +00:00

347 lines
12 KiB
C++

// First, in its own block so the include sorter keeps it there: configuration.h supplies the
// variant defines mesh-pb-constants.h needs (portduino resolves MAX_NUM_NODES at runtime).
#include "configuration.h"
#include "SPILock.h"
#include "SerialConsole.h"
#include "concurrency/OSThread.h"
#include "gps/RTC.h"
#include "mesh/CryptoEngine.h"
#include "TestUtil.h"
#if defined(ARDUINO)
#include <Arduino.h>
#else
#include <chrono>
#include <thread>
#endif
// The state checkpoint needs a POSIX directory walk, and only the host builds run these suites.
// Note ARDUINO *is* defined on portduino, so it is not the right guard here.
#if ARCH_PORTDUINO
#include "platform/portduino/PortduinoGlue.h"
#include <cstdint>
#include <cstdio>
#include <cstdlib>
#include <cstring>
#include <dirent.h>
#include <map>
#include <set>
#include <string>
#include <sys/stat.h>
#include <unistd.h>
#endif
#if ARCH_PORTDUINO
// A test binary must not be reachable from the network. main.cpp's setup()/loop() are compiled out
// under PIO_UNIT_TESTING, so the phone API, MQTT and the web server are never started - but that is
// a property of today's guards, not something anything checks. A suite that pulled in a service
// which binds a port would otherwise open one on the developer's machine, silently, for the length
// of the run. Assert the absence instead of trusting it.
//
// Listening sockets only: an outbound connection is a different (and louder) problem, and gethostby*
// opens transient sockets that would make an any-socket check flap.
// Linux-only: this check reads /proc; MinGW-w64 has no readlink() for fd links.
// Linux CI covers the check; native Windows uses a no-op.
#ifdef _WIN32
static void assertNoListeningSockets() {}
#else
static void assertNoListeningSockets()
{
// Socket fds appear as "socket:[inode]"; a listening TCP row in /proc/self/net carries st 0A.
std::set<std::string> ours;
if (DIR *fds = opendir("/proc/self/fd")) {
while (struct dirent *e = readdir(fds)) {
char path[64], target[128];
snprintf(path, sizeof(path), "/proc/self/fd/%s", e->d_name);
ssize_t n = readlink(path, target, sizeof(target) - 1);
if (n <= 0)
continue;
target[n] = '\0';
unsigned long inode = 0;
if (sscanf(target, "socket:[%lu]", &inode) == 1)
ours.insert(std::to_string(inode));
}
closedir(fds);
}
if (ours.empty())
return;
std::string offenders;
for (const char *table : {"/proc/self/net/tcp", "/proc/self/net/tcp6"}) {
FILE *f = fopen(table, "r");
if (!f)
continue;
char line[512];
bool header = true;
while (fgets(line, sizeof(line), f)) {
if (header) {
header = false;
continue;
}
// sl local_address rem_address st tx:rx tr:when retrnsmt uid timeout inode
char local[128] = {0};
unsigned st = 0, uid = 0;
unsigned long inode = 0;
if (sscanf(line, "%*d: %127s %*s %x %*s %*s %*s %u %*d %lu", local, &st, &uid, &inode) != 4)
continue;
if (st != 0x0A) // TCP_LISTEN
continue;
if (ours.count(std::to_string(inode)) == 0)
continue;
offenders += " ";
offenders += local;
}
fclose(f);
}
if (offenders.empty())
return;
// Before UNITY_BEGIN(), so there is no Unity failure to record - and a test binary that has
// opened a port is not a result worth collecting. Fail the suite outright and say why.
fprintf(stderr,
"FATAL: test binary is listening on%s\n"
"A unit-test run must not be reachable. Something started a network service - check what\n"
"the suite constructs, and whether it belongs behind main.cpp's PIO_UNIT_TESTING guard.\n",
offenders.c_str());
fflush(stderr);
exit(EXIT_FAILURE);
}
#endif
#endif
#if ARCH_PORTDUINO
static bool environmentBaselined = false;
// -s is how the harness keeps a test run off the host's radio: it makes portduinoSetup() skip the
// /etc/meshtasticd/config.yaml search and return before GPIO/SPI init. That job is done by the time
// any of this runs, and the flag's only remaining readers are behaviour we do want under test -
// wouldEncryptWithPKC() disables PKC while it is set. Clear it so suites exercise the production
// encode path; the radio choice is already made and is not revisited.
static void baselineEnvironment()
{
portduino_config.force_simradio = false;
assertNoListeningSockets();
environmentBaselined = true;
}
#endif
void testAssertEnvironmentIntact(const char *testName)
{
#if ARCH_PORTDUINO
// Not every suite calls initializeTestEnvironment() - test_atak does not - so the baseline
// cannot live only there, or those suites run with PKC off and skip the socket check. Establish
// it at the first RUN_TEST for whoever has not, and hold it from then on.
if (!environmentBaselined) {
baselineEnvironment();
return;
}
// Per test, not once per suite: a service that binds a port is opened by the code under test,
// not by the harness, so checking only at startup would miss every case that starts one.
assertNoListeningSockets();
if (!portduino_config.force_simradio)
return;
// Hard exit rather than TEST_FAIL: this runs between tests, outside any Unity test frame, so
// there is no failure to longjmp into. Repairing the flag silently would be worse - it would
// leave the suite that broke it passing.
for (FILE *out : {stdout, stderr})
fprintf(out,
"FATAL: force_simradio was set back on before %s\n"
"PKC is disabled while it is set, so the encode path under test falls back to channel\n"
"crypto and every later case asserts the wrong thing. A test that needs simradio must\n"
"restore the flag before it returns.\n",
testName ? testName : "(unknown test)");
fflush(stderr);
exit(EXIT_FAILURE);
#else
(void)testName;
#endif
}
void initializeTestEnvironment()
{
concurrency::hasBeenSetup = true;
consoleInit();
// NodeDB's constructor reaches spiLock through loadFromDisk(), and no test runs main.cpp, so
// nothing has created it. Suites got away with the null pointer while Lock::lock() was an empty
// function on Portduino; it is a real mutex now, so the call has to have something to lock.
if (!spiLock)
initSPI();
// Same story for cryptLock, which perhapsDecode() and the ack-proof paths take. Router's
// constructor makes one, but plenty of suites reach those paths without building a Router.
// Router reuses this one rather than allocating its own, so making it here is safe either way.
if (!cryptLock)
cryptLock = new concurrency::Lock();
#if ARCH_PORTDUINO
baselineEnvironment();
struct timeval tv;
tv.tv_sec = time(NULL);
tv.tv_usec = 0;
perhapsSetRTC(RTCQualityNTP, &tv);
#endif
concurrency::OSThread::setup();
// Baseline the sandbox before the first RUN_TEST, so writes made during suite setup are not
// charged to whichever test happens to run first.
testStateCheckpoint(nullptr, nullptr);
}
void testDelay(unsigned long ms)
{
#if defined(ARDUINO)
::delay(ms);
#else
std::this_thread::sleep_for(std::chrono::milliseconds(ms));
#endif
}
#if !ARCH_PORTDUINO
void testStateCheckpoint(const char *, const char *) {}
#else
namespace
{
/// MinGW-w64 has no lstat(): Windows has no POSIX symlink stat, and nothing in a test sandbox
/// creates a symlink, so stat() sees the same thing for every entry walk() can reach.
#ifdef _WIN32
inline int lstatCompat(const char *path, struct stat *st)
{
return stat(path, st);
}
#else
inline int lstatCompat(const char *path, struct stat *st)
{
return lstat(path, st);
}
#endif
/// Content fingerprint, used only to answer "did this file change?". FNV-1a rather than a real
/// digest because the answer is a boolean and the files are a few KB of protobuf; nothing here
/// records a hash as an expected value, which is what would make this a snapshot test.
uint64_t fileFingerprint(const std::string &path)
{
FILE *f = fopen(path.c_str(), "rb");
if (!f)
return 0;
uint64_t h = 1469598103934665603ULL;
unsigned char buf[4096];
size_t n;
while ((n = fread(buf, 1, sizeof(buf), f)) > 0) {
for (size_t i = 0; i < n; i++) {
h ^= buf[i];
h *= 1099511628211ULL;
}
}
fclose(f);
return h;
}
void walk(const std::string &root, const std::string &rel, std::map<std::string, uint64_t> &out)
{
const std::string dirPath = rel.empty() ? root : root + "/" + rel;
DIR *d = opendir(dirPath.c_str());
if (!d)
return;
while (struct dirent *e = readdir(d)) {
if (strcmp(e->d_name, ".") == 0 || strcmp(e->d_name, "..") == 0)
continue;
const std::string childRel = rel.empty() ? std::string(e->d_name) : rel + "/" + e->d_name;
const std::string childPath = root + "/" + childRel;
struct stat st;
if (lstatCompat(childPath.c_str(), &st) != 0)
continue;
if (S_ISDIR(st.st_mode))
walk(root, childRel, out);
else if (S_ISREG(st.st_mode))
out[childRel] = fileFingerprint(childPath);
}
closedir(d);
}
/// "test/test_admin_radio/test_main.cpp" -> "test_admin_radio". The suite name is not otherwise
/// available to a test program - PlatformIO passes it to the *build*, not to the run.
std::string suiteFromPath(const char *sourceFile)
{
if (!sourceFile)
return "";
std::string p(sourceFile);
const size_t lastSlash = p.find_last_of('/');
if (lastSlash == std::string::npos)
return "";
p.erase(lastSlash);
const size_t prevSlash = p.find_last_of('/');
return prevSlash == std::string::npos ? p : p.substr(prevSlash + 1);
}
struct StateWatch {
bool resolved = false;
bool active = false;
std::string root;
std::string report;
std::map<std::string, uint64_t> previous;
};
StateWatch &watch()
{
static StateWatch w;
return w;
}
} // namespace
void testStateCheckpoint(const char *testName, const char *sourceFile)
{
StateWatch &w = watch();
if (!w.resolved) {
w.resolved = true;
const char *report = getenv("MESHTASTIC_TEST_STATE_REPORT");
const char *home = getenv("HOME");
// No report path means nobody asked: a bare `pio test` behaves exactly as before.
w.active = report && *report && home && *home;
if (w.active) {
w.report = report;
w.root = home;
}
}
if (!w.active)
return;
std::map<std::string, uint64_t> current;
walk(w.root, "", current);
// The priming call from initializeTestEnvironment() has no test to attribute to.
if (testName) {
const std::string suite = suiteFromPath(sourceFile);
FILE *out = fopen(w.report.c_str(), "a");
if (out) {
for (const auto &entry : current) {
auto prior = w.previous.find(entry.first);
if (prior == w.previous.end())
fprintf(out, "%s\t%s\tadded\t%s\n", suite.c_str(), testName, entry.first.c_str());
else if (prior->second != entry.second)
fprintf(out, "%s\t%s\tmodified\t%s\n", suite.c_str(), testName, entry.first.c_str());
}
for (const auto &entry : w.previous) {
if (current.find(entry.first) == current.end())
fprintf(out, "%s\t%s\tremoved\t%s\n", suite.c_str(), testName, entry.first.c_str());
}
fclose(out);
}
}
w.previous.swap(current);
}
#endif