From 0cbbd948c0ff1f946f51435e57686b417f1ebeae Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Wed, 11 Mar 2020 10:59:27 +0100 Subject: [PATCH 1/4] nvs: clean coverage files on rebuild Fixes errors reported by libgcov related to merging debug information. --- components/nvs_flash/test_nvs_host/Makefile | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/components/nvs_flash/test_nvs_host/Makefile b/components/nvs_flash/test_nvs_host/Makefile index 20c2966e24..68c2161f9a 100644 --- a/components/nvs_flash/test_nvs_host/Makefile +++ b/components/nvs_flash/test_nvs_host/Makefile @@ -39,7 +39,7 @@ COVERAGE_FILES = $(OBJ_FILES:.o=.gc*) $(OBJ_FILES): %.o: %.cpp -$(TEST_PROGRAM): $(OBJ_FILES) +$(TEST_PROGRAM): $(OBJ_FILES) clean-coverage $(MAKE) -C ../../mbedtls/mbedtls/ lib g++ $(LDFLAGS) -o $(TEST_PROGRAM) $(OBJ_FILES) ../../mbedtls/mbedtls/library/libmbedcrypto.a @@ -62,12 +62,14 @@ coverage_report: coverage.info genhtml coverage.info --output-directory coverage_report @echo "Coverage report is in coverage_report/index.html" -clean: - $(MAKE) -C ../../mbedtls/mbedtls/ clean - rm -f $(OBJ_FILES) $(TEST_PROGRAM) +clean-coverage: rm -f $(COVERAGE_FILES) *.gcov rm -rf coverage_report/ rm -f coverage.info + +clean: clean-coverage + $(MAKE) -C ../../mbedtls/mbedtls/ clean + rm -f $(OBJ_FILES) $(TEST_PROGRAM) rm -f ../nvs_partition_generator/partition_single_page.bin rm -f ../nvs_partition_generator/partition_multipage_blob.bin rm -f ../nvs_partition_generator/partition_encrypted.bin @@ -80,4 +82,4 @@ clean: -.PHONY: clean all test long-test +.PHONY: clean clean-coverage all test long-test From db34a4d031cb0902c28a0a6a01cb4345f30f7d9e Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Wed, 11 Mar 2020 11:28:30 +0100 Subject: [PATCH 2/4] nvs: add test for erase cycles distribution --- .../test_nvs_host/spi_flash_emulation.h | 7 +++++ .../nvs_flash/test_nvs_host/test_nvs.cpp | 31 ++++++++++++++++++- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/components/nvs_flash/test_nvs_host/spi_flash_emulation.h b/components/nvs_flash/test_nvs_host/spi_flash_emulation.h index f17573a02e..db929fe910 100644 --- a/components/nvs_flash/test_nvs_host/spi_flash_emulation.h +++ b/components/nvs_flash/test_nvs_host/spi_flash_emulation.h @@ -36,6 +36,7 @@ public: SpiFlashEmulator(size_t sectorCount) : mUpperSectorBound(sectorCount) { mData.resize(sectorCount * SPI_FLASH_SEC_SIZE / 4, 0xffffffff); + mEraseCnt.resize(sectorCount); spi_flash_emulator_set(this); } @@ -124,6 +125,7 @@ public: std::fill_n(begin(mData) + offset, SPI_FLASH_SEC_SIZE / 4, 0xffffffff); ++mEraseOps; + mEraseCnt[sectorNumber]++; mTotalTime += getEraseOpTime(); return true; } @@ -217,6 +219,10 @@ public: mFailCountdown = count; } + size_t getSectorEraseCount(uint32_t sector) const { + return mEraseCnt[sector]; + } + protected: static size_t getReadOpTime(uint32_t bytes); static size_t getWriteOpTime(uint32_t bytes); @@ -224,6 +230,7 @@ protected: std::vector mData; + std::vector mEraseCnt; mutable size_t mReadOps = 0; mutable size_t mWriteOps = 0; diff --git a/components/nvs_flash/test_nvs_host/test_nvs.cpp b/components/nvs_flash/test_nvs_host/test_nvs.cpp index 648b6c900b..8be5a955a6 100644 --- a/components/nvs_flash/test_nvs_host/test_nvs.cpp +++ b/components/nvs_flash/test_nvs_host/test_nvs.cpp @@ -258,7 +258,7 @@ TEST_CASE("Page validates blob size", "[nvs]") Page page; TEST_ESP_OK(page.load(0)); - char buf[2048] = { 0 }; + char buf[4096] = { 0 }; // There are two potential errors here: // - not enough space in the page (because one value has been written already) // - value is too long @@ -527,6 +527,35 @@ TEST_CASE("can modify an item on a page which will be erased", "[nvs]") } } +TEST_CASE("erase operations are distributed among sectors", "[nvs]") +{ + const size_t sectors = 6; + SpiFlashEmulator emu(sectors); + Storage storage; + CHECK(storage.init(0, sectors) == ESP_OK); + + /* Fill some part of storage with static values */ + const size_t static_sectors = 2; + for (size_t i = 0; i < static_sectors * Page::ENTRY_COUNT; ++i) { + char name[Item::MAX_KEY_LENGTH]; + snprintf(name, sizeof(name), "static%d", (int) i); + REQUIRE(storage.writeItem(1, name, i) == ESP_OK); + } + + /* Now perform many write operations */ + const size_t write_ops = 2000; + for (size_t i = 0; i < write_ops; ++i) { + REQUIRE(storage.writeItem(1, "value", i) == ESP_OK); + } + + /* Check that erase counts are distributed between the remaining sectors */ + const size_t max_erase_cnt = write_ops / Page::ENTRY_COUNT / (sectors - static_sectors) + 1; + for (size_t i = 0; i < sectors; ++i) { + auto erase_cnt = emu.getSectorEraseCount(i); + INFO("Sector " << i << " erased " << erase_cnt); + CHECK(erase_cnt <= max_erase_cnt); + } +} TEST_CASE("can erase items", "[nvs]") { From d2526e6dda555f9b119f4e5d5d408cb23512e04c Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Wed, 11 Mar 2020 11:28:44 +0100 Subject: [PATCH 3/4] nvs: fix out of bounds array access in host test --- components/nvs_flash/test_nvs_host/spi_flash_emulation.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/components/nvs_flash/test_nvs_host/spi_flash_emulation.cpp b/components/nvs_flash/test_nvs_host/spi_flash_emulation.cpp index 914efc1452..9881203083 100644 --- a/components/nvs_flash/test_nvs_host/spi_flash_emulation.cpp +++ b/components/nvs_flash/test_nvs_host/spi_flash_emulation.cpp @@ -71,10 +71,11 @@ static size_t blockEraseTime = 37142; static size_t timeInterp(uint32_t bytes, size_t* lut) { + const int lut_size = sizeof(readTimes)/sizeof(readTimes[0]); int lz = __builtin_clz(bytes / 4); int log_size = 32 - lz; size_t x2 = 1 << (log_size + 2); - size_t y2 = lut[log_size]; + size_t y2 = lut[std::min(log_size, lut_size - 1)]; size_t x1 = 1 << (log_size + 1); size_t y1 = lut[log_size - 1]; return (bytes - x1) * (y2 - y1) / (x2 - x1) + y1; From 6afc1160361de13ac73ed5bfc13d15df15157433 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Wed, 11 Mar 2020 11:32:56 +0100 Subject: [PATCH 4/4] nvs: enable address sanitizer in host tests when building with clang --- components/nvs_flash/test_nvs_host/Makefile | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/components/nvs_flash/test_nvs_host/Makefile b/components/nvs_flash/test_nvs_host/Makefile index 68c2161f9a..5c20d8f20c 100644 --- a/components/nvs_flash/test_nvs_host/Makefile +++ b/components/nvs_flash/test_nvs_host/Makefile @@ -28,11 +28,23 @@ SOURCE_FILES = \ crc.cpp \ main.cpp +ifeq ($(shell $(CC) -v 2>&1 | grep -c "clang version"), 1) +COMPILER := clang +else +COMPILER := gcc +endif + CPPFLAGS += -I../include -I../src -I./ -I../../esp_common/include -I../../esp32/include -I ../../mbedtls/mbedtls/include -I ../../spi_flash/include -I ../../soc/include -I ../../xtensa/include -I ../../../tools/catch -fprofile-arcs -ftest-coverage CFLAGS += -fprofile-arcs -ftest-coverage CXXFLAGS += -std=c++11 -Wall -Werror LDFLAGS += -lstdc++ -Wall -fprofile-arcs -ftest-coverage +ifeq ($(COMPILER),clang) +CFLAGS += -fsanitize=address +CXXFLAGS += -fsanitize=address +LDFLAGS += -fsanitize=address +endif + OBJ_FILES = $(SOURCE_FILES:.cpp=.o) COVERAGE_FILES = $(OBJ_FILES:.o=.gc*)