diff --git a/.github/workflows/run-unit-tests.yml b/.github/workflows/run-unit-tests.yml index 42ad32742..795f6dba3 100644 --- a/.github/workflows/run-unit-tests.yml +++ b/.github/workflows/run-unit-tests.yml @@ -126,6 +126,7 @@ jobs: python3 -B test/test_companion_discovery_correlation.py -v python3 -B test/test_pico_ble_queues.py -v python3 -B test/test_companion_preferences_transaction.py -v + python3 -B test/test_companion_prefs_values.py -v python3 -B test/test_companion_prefs_tail.py -v python3 -B test/test_companion_radio_settings_transaction.py -v python3 -B test/test_companion_prefs_transactions.py -v diff --git a/examples/companion_radio/DataStore.cpp b/examples/companion_radio/DataStore.cpp index 085a6eb6c..dd82264ec 100644 --- a/examples/companion_radio/DataStore.cpp +++ b/examples/companion_radio/DataStore.cpp @@ -998,7 +998,11 @@ bool DataStore::loadPrefsInt(const char *filename, double& node_lon) { File file = openRead(_fs, filename); if (file) { - CompanionNodePrefs loaded_prefs = _prefs; + CompanionNodePrefs loaded_prefs; + if (!loaded_prefs.copyPersistedValuesFrom(_prefs)) { + file.close(); + return false; + } // Files written before this tail existed always started Bluetooth at boot. // Do not inherit a runtime off value when loading one of those images. loaded_prefs.bluetooth_enabled = 1; @@ -1230,7 +1234,7 @@ bool DataStore::loadPrefsInt(const char *filename, file.close(); if (!success) return false; loaded_prefs.usb_debug_enabled = loaded_prefs.usb_debug_enabled == 1 ? 1 : 0; - _prefs = loaded_prefs; + if (!_prefs.copyPersistedValuesFrom(loaded_prefs)) return false; node_lat = loaded_lat; node_lon = loaded_lon; return true; diff --git a/examples/companion_radio/NodePrefs.h b/examples/companion_radio/NodePrefs.h index 67d065c2c..e282fe97d 100644 --- a/examples/companion_radio/NodePrefs.h +++ b/examples/companion_radio/NodePrefs.h @@ -247,6 +247,44 @@ private: public: CompanionNodePrefs() : radio(this), custom(&radio) { } + // Only the scalar/array value prefix is a snapshot. Copying the complete + // object also copies adapter owner/fallback pointers and costs two large + // memberwise copies in the transactional loader. Keep both live adapters + // untouched, and leave a normally constructed snapshot's adapters local. + bool copyPersistedValuesFrom(const CompanionNodePrefs& source) { + if (&source == this) return true; + static_assert(sizeof(usb_debug_enabled) == sizeof(uint8_t), + "The published USB debug preference is one byte"); + const uintptr_t first = reinterpret_cast(&airtime_factor); + const uintptr_t last = reinterpret_cast(&usb_debug_enabled); + if (last < first + || last - first > sizeof(*this) - sizeof(usb_debug_enabled)) { + return false; + } + const size_t size = static_cast(last - first) + + sizeof(usb_debug_enabled); + const auto bounded = [size](const CompanionNodePrefs& values) -> bool { + const uintptr_t object = reinterpret_cast(&values); + const uintptr_t begin = + reinterpret_cast(&values.airtime_factor); + const uintptr_t end = + reinterpret_cast(&values.usb_debug_enabled); + const uintptr_t radio_begin = reinterpret_cast(&values.radio); + const uintptr_t custom_begin = reinterpret_cast(&values.custom); + // Subtractions happen only after the corresponding ordering checks. + // Never include either runtime adapter, even if this layout is changed. + return begin >= object && begin - object <= sizeof(values) + && size <= sizeof(values) - static_cast(begin - object) + && end >= begin + && end - begin == size - sizeof(values.usb_debug_enabled) + && radio_begin >= begin && size <= radio_begin - begin + && custom_begin >= begin && size <= custom_begin - begin; + }; + if (!bounded(*this) || !bounded(source)) return false; + memcpy(&airtime_factor, &source.airtime_factor, size); + return true; + } + bool isRepeatEn() const { return client_repeat != 0; } void setRepeatEn(bool enabled) { client_repeat = enabled ? 1 : 0; } diff --git a/test/test_companion_prefs_values.py b/test/test_companion_prefs_values.py new file mode 100644 index 000000000..b3258505a --- /dev/null +++ b/test/test_companion_prefs_values.py @@ -0,0 +1,240 @@ +"""Exercise value-only snapshots using the real adapter-bearing preferences.""" +from pathlib import Path +import os +import re +import subprocess +import tempfile +import unittest + +from test_companion_preferences_transaction import HARNESS, esp_recovery_helpers +from test_radio_receive_contract import method + +ROOT = Path(__file__).resolve().parents[1] + +TESTS = r''' +#include +#include +@FIELD_CHECKS@ + +static void seedValues(CompanionNodePrefs& prefs, uint32_t& random) { + auto fill = [&](void* destination, size_t size) { + auto* bytes = static_cast(destination); + for (size_t i = 0; i < size; ++i) { + random = random * 1664525u + 1013904223u; + bytes[i] = static_cast(random >> 24); + } + }; + @SEED_FIELDS@ +} + +static void equalValues(const CompanionNodePrefs& a, + const CompanionNodePrefs& b) { + @COMPARE_FIELDS@ +} + +static size_t adapterOffset(const CompanionNodePrefs& prefs) { + // The value helper does not copy the adapters or their inter-field padding. + // Use the complete object's byte representation for bounded observation. + const uintptr_t first = reinterpret_cast(&prefs); + const uintptr_t last = reinterpret_cast(&prefs.usb_debug_enabled); + assert(last >= first && last - first < sizeof(prefs)); + return static_cast(last - first) + sizeof(prefs.usb_debug_enabled); +} + +static std::vector adapterBytes(const CompanionNodePrefs& prefs) { + const auto* bytes = reinterpret_cast(&prefs); + return {bytes + adapterOffset(prefs), bytes + sizeof(prefs)}; +} + +static void expectTag(CompanionNodePrefs& prefs, const char* expected) { + char value[32] = {}; + assert(prefs.getCustom()->getByKey("snapshot_tag", value, sizeof(value))); + assert(!strcmp(value, expected)); +} + +int main() { + uint32_t random = 0x19283746; + for (unsigned trial = 0; trial < 64; ++trial) { + CompanionNodePrefs source, snapshot; + seedValues(source, random); + source.getRadioPrefs()->setFreq(915.25f); + assert(source.getCustom()->setByKey("snapshot_tag", "source")); + assert(snapshot.getCustom()->setByKey("snapshot_tag", "snapshot")); + snapshot.clearDirty(); + assert(source.isDirty() && !snapshot.isDirty()); + auto source_adapters = adapterBytes(source); + auto snapshot_adapters = adapterBytes(snapshot); + assert(snapshot.copyPersistedValuesFrom(source)); + equalValues(source, snapshot); + assert(adapterBytes(source) == source_adapters); + assert(adapterBytes(snapshot) == snapshot_adapters); + assert(source.isDirty() && !snapshot.isDirty()); + expectTag(source, "source"); expectTag(snapshot, "snapshot"); + const auto* bytes = reinterpret_cast(&snapshot); + std::vector before_self(bytes, bytes + sizeof(snapshot)); + assert(snapshot.copyPersistedValuesFrom(snapshot)); + assert(!memcmp(&snapshot, before_self.data(), before_self.size())); + // A normally constructed scratch adapter must still own the scratch + // object, including its dynamic-config fallback after copying values. + snapshot.getRadioPrefs()->setFreq(910.5f); + assert(snapshot.freq == 910.5f && source.freq == 915.25f); + source.radio_fem_txgain = 0; + assert(snapshot.getCustom()->setByKey("fem_txgain", "1")); + assert(snapshot.radio_fem_txgain == 1 && source.radio_fem_txgain == 0); + source.getRadioPrefs()->setFreq(920.5f); + assert(source.freq == 920.5f && snapshot.freq == 910.5f); + } + + unsigned comparisons = 0; + for (unsigned trial = 0; trial < 3; ++trial) { + CompanionNodePrefs saved; + seedValues(saved, random); + saved.bluetooth_enabled = static_cast(trial); + DataStore image_store; + assert(image_store.savePrefs(saved, 47.125, -122.25)); + const auto image = image_store.fs.files["/new_prefs"]; + // Test every possible old/torn image length, including every officially + // supported append boundary and unknown lengths beyond the current tail. + for (size_t length = 0; length <= image.size() + 5; ++length) { + auto disk = image; + disk.resize(length); + for (size_t quota = 0; quota <= length; ++quota) { + DataStore actual_store, legacy_store; + actual_store.fs.files["/new_prefs"] = disk; + legacy_store.fs.files["/new_prefs"] = disk; + actual_store.fs.fail_read_after = static_cast(quota); + legacy_store.fs.fail_read_after = static_cast(quota); + CompanionNodePrefs actual, legacy; + seedValues(actual, random); + assert(legacy.copyPersistedValuesFrom(actual)); + actual.getRadioPrefs()->setFreq(915.25f); + legacy.getRadioPrefs()->setFreq(915.25f); + assert(actual.getCustom()->setByKey("snapshot_tag", "actual")); + assert(legacy.getCustom()->setByKey("snapshot_tag", "legacy")); + auto actual_adapters = adapterBytes(actual); + auto legacy_adapters = adapterBytes(legacy); + double lat = 1, lon = 2, legacy_lat = 1, legacy_lon = 2; + const bool expected = legacy_store.loadPrefsLegacy( + "/new_prefs", legacy, legacy_lat, legacy_lon); + const bool got = actual_store.loadPrefsInt( + "/new_prefs", actual, lat, lon); + assert(got == expected); + equalValues(actual, legacy); + assert(!memcmp(&lat, &legacy_lat, sizeof(lat))); + assert(!memcmp(&lon, &legacy_lon, sizeof(lon))); + assert(actual_store.fs.files["/new_prefs"] == disk); + assert(legacy_store.fs.files["/new_prefs"] == disk); + assert(adapterBytes(actual) == actual_adapters); + assert(adapterBytes(legacy) == legacy_adapters); + assert(actual.isDirty() && legacy.isDirty()); + expectTag(actual, "actual"); expectTag(legacy, "legacy"); + auto legacy_txgain = legacy.radio_fem_txgain; + assert(actual.getCustom()->setByKey("fem_txgain", "1")); + assert(actual.radio_fem_txgain == 1 + && legacy.radio_fem_txgain == legacy_txgain); + ++comparisons; + } + } + } + printf("PASS: %u actual-loader value/adapter/failure comparisons\n", comparisons); +} +''' + + +class CompanionPrefsValuesTests(unittest.TestCase): + def test_value_only_copy_and_full_loader_match_legacy(self): + source = (ROOT / 'examples/companion_radio/DataStore.cpp').read_text() + loader = method(source, 'bool DataStore::loadPrefsInt(') + legacy = loader.replace('DataStore::loadPrefsInt(', 'DataStore::loadPrefsLegacy(', 1) + legacy = legacy.replace(''' CompanionNodePrefs loaded_prefs; + if (!loaded_prefs.copyPersistedValuesFrom(_prefs)) { + file.close(); + return false; + }''', ' CompanionNodePrefs loaded_prefs = _prefs;') + legacy = legacy.replace( + ' if (!_prefs.copyPersistedValuesFrom(loaded_prefs)) return false;', + ' _prefs = loaded_prefs;') + self.assertNotIn('copyPersistedValuesFrom', legacy) + methods = esp_recovery_helpers(source) + '\n'.join((loader, legacy, + method(source, 'bool DataStore::savePrefs('))) + code = HARNESS.split('int main(')[0].replace('@METHODS@', methods) + code = code.replace(' bool loadPrefsInt(const char*, CompanionNodePrefs&, double&, double&);', + ' bool loadPrefsInt(const char*, CompanionNodePrefs&, double&, double&);\n' + ' bool loadPrefsLegacy(const char*, CompanionNodePrefs&, double&, double&);') + prefs = (ROOT / 'examples/companion_radio/NodePrefs.h').read_text() + prefix = prefs[prefs.index('class CompanionNodePrefs {'):prefs.index('\nprivate:')] + declarations = re.findall( + r'^\s*(?:float|char|u?int(?:8|16|32)_t)\s+([A-Za-z_][A-Za-z_0-9]*)', + prefix, re.M) + self.assertEqual(declarations[0], 'airtime_factor') + self.assertEqual(declarations[-1], 'usb_debug_enabled') + optional = { + 'fan_mode': 'TBEAM_1W', 'fan_lo': 'TBEAM_1W', 'fan_hi': 'TBEAM_1W', + 'wifi_ssid': 'defined(RP2040_PLATFORM) && defined(ENABLE_WIFI_INTERFACE)', + 'wifi_pwd': 'defined(RP2040_PLATFORM) && defined(ENABLE_WIFI_INTERFACE)', + } + + def fields(template): + rows = [] + for name in declarations: + row = template.format(name=name) + if name in optional: + condition = optional[name] + if not condition.startswith('defined('): + condition = 'defined(' + condition + ')' + row = '#if ' + condition + '\n' + row + '\n#endif' + rows.append(row) + return '\n'.join(rows) + + tests = TESTS.replace('@FIELD_CHECKS@', fields( + 'static_assert(std::is_trivially_copyable::value,\n' + ' "A persisted snapshot member must remain a value");')) + tests = tests.replace('@SEED_FIELDS@', fields('fill(&prefs.{name}, sizeof(prefs.{name}));')) + tests = tests.replace('@COMPARE_FIELDS@', fields( + 'assert(!memcmp(&a.{name}, &b.{name}, sizeof(a.{name})));')) + with tempfile.TemporaryDirectory(prefix='meshcore-prefs-values-') as directory: + work = Path(directory) + (work / 'Utils.h').write_text('''#pragma once +#include +#include +namespace mesh { struct Utils { + static void printHex(Stream&,const uint8_t*,size_t){assert(false);} + static void fromHex(uint8_t*,size_t,const char*){assert(false);} +}; } +''') + (work / 'platform_shim.h').write_text('''#include +#include +inline char* utoa(unsigned int value,char* output,int base){ + if(base!=10)abort();sprintf(output,"%u",value);return output; +} +''') + transaction = (ROOT / 'src/helpers/ContactFileTransaction.h').read_text() + (work / 'ContactFileTransaction.h').write_text(transaction.replace( + '#include "IdentityStore.h"', '#include ')) + cpp = work / 'test.cpp' + cpp.write_text(code + tests) + for platform in ('STM32_PLATFORM', 'NRF52_PLATFORM', 'ESP32_PLATFORM', 'RP2040_PLATFORM', + 'ESP32_PLATFORM,TBEAM_1W', 'RP2040_PLATFORM,ENABLE_WIFI_INTERFACE'): + with self.subTest(platform=platform): + binary = work / 'test' + sanitizers = [] if os.name == 'nt' else [ + '-fsanitize=address,undefined', '-fno-sanitize-recover=all', + '-fno-pie', '-no-pie'] + built = subprocess.run([os.environ.get('CXX', 'g++'), '-std=c++17', + *['-D' + flag + '=1' for flag in platform.split(',')], + *sanitizers, '-include', str(work / 'platform_shim.h'), + '-I', str(work), '-I', str(ROOT / 'test/fixtures/radio_profiles/mocks'), + '-I', str(ROOT / 'test/mocks'), '-I', str(ROOT / 'src'), + '-I', str(ROOT / 'src/helpers'), '-I', str(ROOT), str(cpp), + *[str(ROOT / 'src/helpers' / name) for name in ( + 'ConfigSerializer.cpp', 'DynamicConfigSerializer.cpp', + 'CommonRadioPrefs.cpp', 'TxtDataHelpers.cpp')], + '-o', str(binary)], capture_output=True, text=True) + self.assertEqual(built.returncode, 0, built.stdout + built.stderr) + self.assertNotIn('offsetof', built.stderr) + ran = subprocess.run([str(binary)], capture_output=True, text=True, timeout=60) + self.assertEqual(ran.returncode, 0, ran.stdout + ran.stderr) + + +if __name__ == '__main__': + unittest.main() diff --git a/test/test_identity_and_settings_recovery.py b/test/test_identity_and_settings_recovery.py index 9385b4dc4..e23cd6644 100644 --- a/test/test_identity_and_settings_recovery.py +++ b/test/test_identity_and_settings_recovery.py @@ -14,21 +14,39 @@ def fixture_prefix(): return text[:text.index('struct Host {')] -def compile_run(program, platform): +def compile_run(program, platform, actual_node_prefs=False): with tempfile.TemporaryDirectory(prefix='mesh-storage-recovery-') as directory: directory = Path(directory) (directory / 'FS.h').write_text('#pragma once\nnamespace fs { using FS = FakeFilesystem; }\n') (directory / 'Adafruit_LittleFS.h').write_text( '#pragma once\nusing Adafruit_LittleFS = FakeFilesystem;\n' 'namespace Adafruit_LittleFS_Namespace {}\n') + helper_sources = [] + if actual_node_prefs: + # Keep the actual owner-bearing adapters in the preferences + # recovery test, rather than replacing the new snapshot API. + (directory / 'Arduino.h').write_text( + '#include "' + str(ROOT / 'test/mocks/Arduino.h') + '"\n') + utils = (ROOT / 'test/fixtures/contact_cache/mocks/Utils.h').read_text() + utils = utils.replace('public:', '''public: + static void printHex(Stream&, const uint8_t*, size_t) { assert(false); } + static void fromHex(uint8_t*, size_t, const char*) { assert(false); } +''', 1) + (directory / 'Utils.h').write_text('#include \n' + utils) + helper_sources = [str(ROOT / 'src/helpers' / name) for name in ( + 'ConfigSerializer.cpp', 'DynamicConfigSerializer.cpp', + 'CommonRadioPrefs.cpp', 'TxtDataHelpers.cpp')] cpp, exe = directory / 'test.cpp', directory / 'test' cpp.write_text(program) compiled = subprocess.run([ 'c++', '-std=c++17', '-O1', '-g', '-fsanitize=address,undefined', - '-fno-pie', '-no-pie', '-DMESH_CONTACT_CACHE=0', '-D' + platform + '=1', + '-fno-sanitize-recover=all', '-fno-pie', '-no-pie', + '-DMESH_CONTACT_CACHE=0', '-D' + platform + '=1', '-I', str(directory), '-I', str(ROOT / 'test/fixtures/contact_cache/mocks'), + '-I', str(ROOT / 'test/mocks'), '-I', str(ROOT / 'src'), '-I', str(ROOT / 'lib/ed25519'), - '-I', str(ROOT / 'examples/companion_radio'), str(cpp), '-o', str(exe), + '-I', str(ROOT / 'examples/companion_radio'), str(cpp), *helper_sources, + '-o', str(exe), ], capture_output=True, text=True, timeout=60) if compiled.returncode: raise AssertionError(compiled.stdout + compiled.stderr) @@ -233,12 +251,8 @@ int main() { def test_esp32_preferences_and_channels_recover_or_reset(self): store = (ROOT / 'examples/companion_radio/DataStore.cpp').read_text() - prefs = (ROOT / 'examples/companion_radio/NodePrefs.h').read_text() - fields = prefs[prefs.index('class CompanionNodePrefs {'):prefs.index('\nprivate:')] program = fixture_prefix() + r''' -#include -#include "BluetoothName.h" -''' + fields + '\n};\n' + r''' +#include "NodePrefs.h" struct ChannelDetails { struct { uint8_t secret[32] = {}; uint8_t tx_radio = 0; } channel; char name[32] = {}; }; #define MAX_GROUP_CHANNELS 40 struct DataStoreHost { @@ -330,7 +344,7 @@ int main() { } } ''' - compile_run(program, 'ESP32_PLATFORM') + compile_run(program, 'ESP32_PLATFORM', actual_node_prefs=True) if __name__ == '__main__':