From 233049618a0c776d2a8c398a35952b2816977fc6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Damian=20Kr=C3=B3lik?=
 <66667989+Damian-Nordic@users.noreply.github.com>
Date: Mon, 15 Nov 2021 19:37:27 +0100
Subject: [PATCH] [platform] Fix crashes in ConfigurationManager (#11791)

Now that ConfigurationManager is pure virtual it's no longer
safe to include or exclude its interface methods using
pre-compiler directives as building the ConfigurationManager
header using different settings may lead to vtable index
mismatch and hard to debug crashes.

In fact, the crashes occurred on platforms that use several
build systems, e.g. ESP32 (see #11775) or nRF Connect, as
GN sets NDEBUG for release builds while nRF Connect examples
are by default built with release configuration AND enabled
assertions, too.
---
 src/include/platform/ConfigurationManager.h       |  6 +-----
 .../internal/GenericConfigurationManagerImpl.cpp  | 15 ++++++++++-----
 .../internal/GenericConfigurationManagerImpl.h    |  4 ----
 3 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/src/include/platform/ConfigurationManager.h b/src/include/platform/ConfigurationManager.h
index 18ffe60efb7beb..9390fff692fada 100644
--- a/src/include/platform/ConfigurationManager.h
+++ b/src/include/platform/ConfigurationManager.h
@@ -83,10 +83,8 @@ class ConfigurationManager
     virtual CHIP_ERROR GetFirmwareRevision(uint16_t & firmwareRev)                                  = 0;
     virtual CHIP_ERROR GetSetupPinCode(uint32_t & setupPinCode)                                     = 0;
     virtual CHIP_ERROR GetSetupDiscriminator(uint16_t & setupDiscriminator)                         = 0;
-#if CHIP_ENABLE_ROTATING_DEVICE_ID
     // Lifetime counter is monotonic counter that is incremented only in the case of a factory reset
-    virtual CHIP_ERROR GetLifetimeCounter(uint16_t & lifetimeCounter) = 0;
-#endif
+    virtual CHIP_ERROR GetLifetimeCounter(uint16_t & lifetimeCounter)                  = 0;
     virtual CHIP_ERROR GetRegulatoryLocation(uint32_t & location)                      = 0;
     virtual CHIP_ERROR GetCountryCode(char * buf, size_t bufSize, size_t & codeLen)    = 0;
     virtual CHIP_ERROR GetBreadcrumb(uint64_t & breadcrumb)                            = 0;
@@ -109,9 +107,7 @@ class ConfigurationManager
 
     virtual CHIP_ERROR GetBLEDeviceIdentificationInfo(Ble::ChipBLEDeviceIdentificationInfo & deviceIdInfo) = 0;
 
-#if !defined(NDEBUG)
     virtual CHIP_ERROR RunUnitTests() = 0;
-#endif
 
     virtual bool IsFullyProvisioned()   = 0;
     virtual void InitiateFactoryReset() = 0;
diff --git a/src/include/platform/internal/GenericConfigurationManagerImpl.cpp b/src/include/platform/internal/GenericConfigurationManagerImpl.cpp
index ef8f91f08742eb..4ea6ff3515640f 100644
--- a/src/include/platform/internal/GenericConfigurationManagerImpl.cpp
+++ b/src/include/platform/internal/GenericConfigurationManagerImpl.cpp
@@ -379,20 +379,26 @@ CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::StoreBootReasons(uint32_t
     return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
 }
 
-#if CHIP_ENABLE_ROTATING_DEVICE_ID
 template <class ImplClass>
 CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::GetLifetimeCounter(uint16_t & lifetimeCounter)
 {
+#if CHIP_ENABLE_ROTATING_DEVICE_ID
     lifetimeCounter = static_cast<uint16_t>(mLifetimePersistedCounter.GetValue());
     return CHIP_NO_ERROR;
+#else
+    return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
+#endif
 }
 
 template <class ImplClass>
 CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::_IncrementLifetimeCounter()
 {
+#if CHIP_ENABLE_ROTATING_DEVICE_ID
     return mLifetimePersistedCounter.Advance();
-}
+#else
+    return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
 #endif
+}
 
 template <class ImplClass>
 CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::GetFailSafeArmed(bool & val)
@@ -490,16 +496,15 @@ CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::GetSecondaryPairingInstru
     return CHIP_NO_ERROR;
 }
 
-#if !defined(NDEBUG)
 template <class ImplClass>
 CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::RunUnitTests()
 {
+#if !defined(NDEBUG)
     ChipLogProgress(DeviceLayer, "Running configuration unit test");
     Impl()->RunConfigUnitTest();
-
+#endif
     return CHIP_NO_ERROR;
 }
-#endif
 
 template <class ImplClass>
 void GenericConfigurationManagerImpl<ImplClass>::LogDeviceConfig()
diff --git a/src/include/platform/internal/GenericConfigurationManagerImpl.h b/src/include/platform/internal/GenericConfigurationManagerImpl.h
index 42a64c7781a32f..26c9e10a5b6604 100644
--- a/src/include/platform/internal/GenericConfigurationManagerImpl.h
+++ b/src/include/platform/internal/GenericConfigurationManagerImpl.h
@@ -76,10 +76,8 @@ class GenericConfigurationManagerImpl : public ConfigurationManager
     CHIP_ERROR StoreSetupPinCode(uint32_t setupPinCode) override;
     CHIP_ERROR GetSetupDiscriminator(uint16_t & setupDiscriminator) override;
     CHIP_ERROR StoreSetupDiscriminator(uint16_t setupDiscriminator) override;
-#if CHIP_ENABLE_ROTATING_DEVICE_ID
     CHIP_ERROR GetLifetimeCounter(uint16_t & lifetimeCounter) override;
     CHIP_ERROR _IncrementLifetimeCounter();
-#endif
     CHIP_ERROR GetFailSafeArmed(bool & val) override;
     CHIP_ERROR SetFailSafeArmed(bool val) override;
     CHIP_ERROR GetBLEDeviceIdentificationInfo(Ble::ChipBLEDeviceIdentificationInfo & deviceIdInfo) override;
@@ -103,9 +101,7 @@ class GenericConfigurationManagerImpl : public ConfigurationManager
     CHIP_ERROR StoreTotalOperationalHours(uint32_t totalOperationalHours) override;
     CHIP_ERROR GetBootReasons(uint32_t & bootReasons) override;
     CHIP_ERROR StoreBootReasons(uint32_t bootReasons) override;
-#if !defined(NDEBUG)
     CHIP_ERROR RunUnitTests(void) override;
-#endif
     bool IsFullyProvisioned() override;
     void InitiateFactoryReset() override;
     void LogDeviceConfig() override;