[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.
project-chip · Jan 4, 2022 · 9007075 · 9007075
1 parent a5d38ec
commit 9007075
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 14 deletions.
diff --git 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
@@ -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
@@ -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;