Darwin: Prohibit static initializers in Matter.framework (#34168)

* Darwin: Prohibit static initializers in Matter.framework Globals should either be "constinit" (i.e. use a constrexpr constructor) and trivially destructible, or use the Global<> / AtomicGlobal<> helpers. * Update src/messaging/ReliableMessageProtocolConfig.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Use correct value in ReliableMessageProtocolConfig unit test override * Enable -no_inits for release builds only ASAN and TSAN both use initializers, so enabling it for Debug builds breaks those in CI. Ideally we could just turn it off for builds that actually use *SAN but that probably requires migrating the project to use xcconfig files. --------- Co-authored-by: Boris Zbarsky <[email protected]>
project-chip · Jul 4, 2024 · f199ba4 · f199ba4
1 parent 5e37260
commit f199ba4
Show file tree

Hide file tree

Showing 14 changed files with 80 additions and 47 deletions.
diff --git a/src/app/AttributeAccessInterfaceCache.h b/src/app/AttributeAccessInterfaceCache.h
@@ -48,7 +48,7 @@ class AttributeAccessInterfaceCache
         kDefinitelyUsed
     };
 
-    AttributeAccessInterfaceCache() { Invalidate(); }
+    constexpr AttributeAccessInterfaceCache() = default;
 
     /**
      * @brief Invalidate the whole cache. Must be called every time list of AAI registrations changes.
@@ -106,6 +106,8 @@ class AttributeAccessInterfaceCache
 private:
     struct AttributeAccessCacheEntry
     {
+        constexpr AttributeAccessCacheEntry() = default;
+
         EndpointId endpointId               = kInvalidEndpointId;
         ClusterId clusterId                 = kInvalidClusterId;
         AttributeAccessInterface * accessor = nullptr;
@@ -137,8 +139,8 @@ class AttributeAccessInterfaceCache
         return &mCacheSlots[0];
     }
 
-    AttributeAccessCacheEntry mCacheSlots[1];
-    AttributeAccessCacheEntry mLastUnusedEntry;
+    AttributeAccessCacheEntry mCacheSlots[1] = {};
+    AttributeAccessCacheEntry mLastUnusedEntry{};
 };
 
 } // namespace app

diff --git a/src/app/EventLoggingTypes.h b/src/app/EventLoggingTypes.h
@@ -100,7 +100,7 @@ struct Timestamp
         kSystem = 0,
         kEpoch
     };
-    Timestamp() {}
+    constexpr Timestamp() = default;
     Timestamp(Type aType, uint64_t aValue) : mType(aType), mValue(aValue) {}
     Timestamp(System::Clock::Timestamp aValue) : mType(Type::kSystem), mValue(aValue.count()) {}
     static Timestamp Epoch(System::Clock::Timestamp aValue)

diff --git a/src/app/EventManagement.cpp b/src/app/EventManagement.cpp
@@ -32,7 +32,7 @@ using namespace chip::TLV;
 
 namespace chip {
 namespace app {
-static EventManagement sInstance;
+EventManagement EventManagement::sInstance;
 
 /**
  * @brief

diff --git a/src/app/EventManagement.h b/src/app/EventManagement.h
@@ -73,7 +73,7 @@ namespace app {
 inline constexpr const uint32_t kEventManagementProfile = 0x1;
 inline constexpr const uint32_t kFabricIndexTag         = 0x1;
 inline constexpr size_t kMaxEventSizeReserve            = 512;
-constexpr uint16_t kRequiredEventField =
+inline constexpr uint16_t kRequiredEventField =
     (1 << to_underlying(EventDataIB::Tag::kPriority)) | (1 << to_underlying(EventDataIB::Tag::kPath));
 
 /**
@@ -388,6 +388,9 @@ class EventManagement
     void SetScheduledEventInfo(EventNumber & aEventNumber, uint32_t & aInitialWrittenEventBytes) const;
 
 private:
+    constexpr EventManagement() = default;
+    static EventManagement sInstance;
+
     /**
      * @brief
      *  Internal structure for traversing events.
@@ -555,9 +558,9 @@ class EventManagement
     MonotonicallyIncreasingCounter<EventNumber> * mpEventNumberCounter = nullptr;
 
     EventNumber mLastEventNumber = 0; ///< Last event Number vended
-    Timestamp mLastEventTimestamp;    ///< The timestamp of the last event in this buffer
+    Timestamp mLastEventTimestamp{};  ///< The timestamp of the last event in this buffer
 
-    System::Clock::Milliseconds64 mMonotonicStartupTime;
+    System::Clock::Milliseconds64 mMonotonicStartupTime{};
 };
 } // namespace app
 } // namespace chip
diff --git a/src/app/clusters/descriptor/descriptor.cpp b/src/app/clusters/descriptor/descriptor.cpp
@@ -27,6 +27,7 @@
 #include <app/AttributeAccessInterfaceRegistry.h>
 #include <app/util/attribute-storage.h>
 #include <app/util/endpoint-config-api.h>
+#include <lib/core/Global.h>
 #include <lib/support/CodeUtils.h>
 #include <lib/support/logging/CHIPLogging.h>
 
@@ -205,7 +206,9 @@ CHIP_ERROR DescriptorAttrAccess::ReadClusterRevision(EndpointId endpoint, Attrib
     return aEncoder.Encode(kClusterRevision);
 }
 
-DescriptorAttrAccess gAttrAccess;
+namespace {
+Global<DescriptorAttrAccess> gAttrAccess;
+}
 
 CHIP_ERROR DescriptorAttrAccess::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder)
 {
@@ -244,5 +247,5 @@ CHIP_ERROR DescriptorAttrAccess::Read(const ConcreteReadAttributePath & aPath, A
 
 void MatterDescriptorPluginServerInitCallback()
 {
-    registerAttributeAccessOverride(&gAttrAccess);
+    registerAttributeAccessOverride(&gAttrAccess.get());
 }
diff --git a/src/darwin/Framework/Matter.xcodeproj/project.pbxproj b/src/darwin/Framework/Matter.xcodeproj/project.pbxproj
@@ -2497,6 +2497,7 @@
 					"-Wl,-unexported_symbol,\"__Unwind_*\"",
 					"-Wl,-unexported_symbol,\"_unw_*\"",
 					"-Wl,-hidden-lCHIP",
+					"-Wl,-no_inits",
 				);
 				"OTHER_LDFLAGS[sdk=macosx*]" = (
 					"-framework",
@@ -2515,6 +2516,7 @@
 					"-Wl,-unexported_symbol,\"__Unwind_*\"",
 					"-Wl,-unexported_symbol,\"_unw_*\"",
 					"-Wl,-hidden-lCHIP",
+					"-Wl,-no_inits",
 				);
 				PRODUCT_BUNDLE_IDENTIFIER = com.csa.matter;
 				PRODUCT_NAME = "$(TARGET_NAME:c99extidentifier)";

diff --git a/src/include/platform/internal/GenericConfigurationManagerImpl.ipp b/src/include/platform/internal/GenericConfigurationManagerImpl.ipp
@@ -49,14 +49,18 @@
 #include <platform/ThreadStackManager.h>
 #endif
 
+#include <optional>
+
 // TODO : may be we can make it configurable
 #define BLE_ADVERTISEMENT_VERSION 0
 
 namespace chip {
 namespace DeviceLayer {
 namespace Internal {
 
-static Optional<System::Clock::Seconds32> sFirmwareBuildChipEpochTime;
+namespace {
+std::optional<System::Clock::Seconds32> gFirmwareBuildChipEpochTime;
+}
 
 #if CHIP_USE_TRANSITIONAL_COMMISSIONABLE_DATA_PROVIDER
 
@@ -288,9 +292,9 @@ template <class ConfigClass>
 CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::GetFirmwareBuildChipEpochTime(System::Clock::Seconds32 & chipEpochTime)
 {
     // If the setter was called and we have a value in memory, return this.
-    if (sFirmwareBuildChipEpochTime.HasValue())
+    if (gFirmwareBuildChipEpochTime.has_value())
     {
-        chipEpochTime = sFirmwareBuildChipEpochTime.Value();
+        chipEpochTime = gFirmwareBuildChipEpochTime.value();
         return CHIP_NO_ERROR;
     }
 #ifdef CHIP_DEVICE_CONFIG_FIRMWARE_BUILD_TIME_MATTER_EPOCH_S
@@ -323,7 +327,7 @@ CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::SetFirmwareBuildChipEpo
     //
     // Implementations that can't use the hard-coded time for whatever reason
     // should set this at each init.
-    sFirmwareBuildChipEpochTime.SetValue(chipEpochTime);
+    gFirmwareBuildChipEpochTime = chipEpochTime;
     return CHIP_NO_ERROR;
 }
 

diff --git a/src/lib/support/IntrusiveList.h b/src/lib/support/IntrusiveList.h
@@ -84,7 +84,7 @@ enum class IntrusiveMode
 class IntrusiveListNodePrivateBase
 {
 public:
-    IntrusiveListNodePrivateBase() : mPrev(nullptr), mNext(nullptr) {}
+    constexpr IntrusiveListNodePrivateBase() : mPrev(nullptr), mNext(nullptr) {}
     ~IntrusiveListNodePrivateBase() { VerifyOrDie(!IsInList()); }
 
     // Note: The copy construct/assignment is not provided because the list node state is not copyable.
@@ -98,7 +98,7 @@ class IntrusiveListNodePrivateBase
 
 private:
     friend class IntrusiveListBase;
-    IntrusiveListNodePrivateBase(IntrusiveListNodePrivateBase * prev, IntrusiveListNodePrivateBase * next) :
+    constexpr IntrusiveListNodePrivateBase(IntrusiveListNodePrivateBase * prev, IntrusiveListNodePrivateBase * next) :
         mPrev(prev), mNext(next)
     {}
 
@@ -284,7 +284,7 @@ class IntrusiveListBase
     //   ^                                           |
     //    \------------------------------------------/
     //
-    IntrusiveListBase() : mNode(&mNode, &mNode) {}
+    constexpr IntrusiveListBase() : mNode(&mNode, &mNode) {}
     ~IntrusiveListBase()
     {
         VerifyOrDie(Empty());
@@ -399,7 +399,7 @@ template <typename T, IntrusiveMode Mode = IntrusiveMode::Strict, typename Hook
 class IntrusiveList : public IntrusiveListBase
 {
 public:
-    IntrusiveList() : IntrusiveListBase() {}
+    constexpr IntrusiveList() : IntrusiveListBase() {}
 
     IntrusiveList(IntrusiveList &&)             = default;
     IntrusiveList & operator=(IntrusiveList &&) = default;

diff --git a/src/messaging/ReliableMessageProtocolConfig.cpp b/src/messaging/ReliableMessageProtocolConfig.cpp
@@ -32,28 +32,33 @@
 #include <app/icd/server/ICDConfigurationData.h> // nogncheck
 #endif
 
+#include <optional>
+
 namespace chip {
 
 using namespace System::Clock::Literals;
 
 #if CONFIG_BUILD_FOR_HOST_UNIT_TEST
-static Optional<System::Clock::Timeout> idleRetransTimeoutOverride   = NullOptional;
-static Optional<System::Clock::Timeout> activeRetransTimeoutOverride = NullOptional;
-static Optional<System::Clock::Timeout> activeThresholdTimeOverride  = NullOptional;
+namespace {
+// Use std::optional for globals to avoid static initializers
+std::optional<System::Clock::Timeout> gIdleRetransTimeoutOverride;
+std::optional<System::Clock::Timeout> gActiveRetransTimeoutOverride;
+std::optional<System::Clock::Timeout> gActiveThresholdTimeOverride;
+} // namespace
 
 void OverrideLocalMRPConfig(System::Clock::Timeout idleRetransTimeout, System::Clock::Timeout activeRetransTimeout,
                             System::Clock::Timeout activeThresholdTime)
 {
-    idleRetransTimeoutOverride.SetValue(idleRetransTimeout);
-    activeRetransTimeoutOverride.SetValue(activeRetransTimeout);
-    activeThresholdTimeOverride.SetValue(activeThresholdTime);
+    gIdleRetransTimeoutOverride   = idleRetransTimeout;
+    gActiveRetransTimeoutOverride = activeRetransTimeout;
+    gActiveThresholdTimeOverride  = activeThresholdTime;
 }
 
 void ClearLocalMRPConfigOverride()
 {
-    activeRetransTimeoutOverride.ClearValue();
-    idleRetransTimeoutOverride.ClearValue();
-    activeThresholdTimeOverride.ClearValue();
+    gActiveRetransTimeoutOverride = std::nullopt;
+    gIdleRetransTimeoutOverride   = std::nullopt;
+    gActiveThresholdTimeOverride  = std::nullopt;
 }
 #endif
 
@@ -62,14 +67,15 @@ namespace {
 
 // This is not a static member of ReliableMessageProtocolConfig because the free
 // function GetLocalMRPConfig() needs access to it.
-Optional<ReliableMessageProtocolConfig> sDynamicLocalMPRConfig;
+// Use std::optional to avoid a static initializer
+std::optional<ReliableMessageProtocolConfig> sDynamicLocalMPRConfig;
 
 } // anonymous namespace
 
 bool ReliableMessageProtocolConfig::SetLocalMRPConfig(const Optional<ReliableMessageProtocolConfig> & localMRPConfig)
 {
     auto oldConfig         = GetLocalMRPConfig();
-    sDynamicLocalMPRConfig = localMRPConfig;
+    sDynamicLocalMPRConfig = localMRPConfig.std_optional();
     return oldConfig != GetLocalMRPConfig();
 }
 #endif // CHIP_DEVICE_CONFIG_ENABLE_DYNAMIC_MRP_CONFIG
@@ -89,9 +95,9 @@ Optional<ReliableMessageProtocolConfig> GetLocalMRPConfig()
     ReliableMessageProtocolConfig config(CHIP_CONFIG_MRP_LOCAL_IDLE_RETRY_INTERVAL, CHIP_CONFIG_MRP_LOCAL_ACTIVE_RETRY_INTERVAL);
 
 #if CHIP_DEVICE_CONFIG_ENABLE_DYNAMIC_MRP_CONFIG
-    if (sDynamicLocalMPRConfig.HasValue())
+    if (sDynamicLocalMPRConfig.has_value())
     {
-        config = sDynamicLocalMPRConfig.Value();
+        config = sDynamicLocalMPRConfig.value();
     }
 #endif // CHIP_DEVICE_CONFIG_ENABLE_DYNAMIC_MRP_CONFIG
 
@@ -105,19 +111,19 @@ Optional<ReliableMessageProtocolConfig> GetLocalMRPConfig()
 #endif
 
 #if CONFIG_BUILD_FOR_HOST_UNIT_TEST
-    if (idleRetransTimeoutOverride.HasValue())
+    if (gIdleRetransTimeoutOverride.has_value())
     {
-        config.mIdleRetransTimeout = idleRetransTimeoutOverride.Value();
+        config.mIdleRetransTimeout = gIdleRetransTimeoutOverride.value();
     }
 
-    if (activeRetransTimeoutOverride.HasValue())
+    if (gActiveRetransTimeoutOverride.has_value())
     {
-        config.mActiveRetransTimeout = activeRetransTimeoutOverride.Value();
+        config.mActiveRetransTimeout = gActiveRetransTimeoutOverride.value();
     }
 
-    if (activeThresholdTimeOverride.HasValue())
+    if (gActiveThresholdTimeOverride.has_value())
     {
-        config.mActiveThresholdTime = activeRetransTimeoutOverride.Value();
+        config.mActiveThresholdTime = gActiveThresholdTimeOverride.value();
     }
 #endif
 

diff --git a/src/messaging/ReliableMessageProtocolConfig.h b/src/messaging/ReliableMessageProtocolConfig.h
@@ -191,8 +191,9 @@ inline constexpr System::Clock::Milliseconds32 kDefaultActiveTime = System::Cloc
  */
 struct ReliableMessageProtocolConfig
 {
-    ReliableMessageProtocolConfig(System::Clock::Milliseconds32 idleInterval, System::Clock::Milliseconds32 activeInterval,
-                                  System::Clock::Milliseconds16 activeThreshold = kDefaultActiveTime) :
+    constexpr ReliableMessageProtocolConfig(System::Clock::Milliseconds32 idleInterval,
+                                            System::Clock::Milliseconds32 activeInterval,
+                                            System::Clock::Milliseconds16 activeThreshold = kDefaultActiveTime) :
         mIdleRetransTimeout(idleInterval),
         mActiveRetransTimeout(activeInterval), mActiveThresholdTime(activeThreshold)
     {}

diff --git a/src/platform/Darwin/ConfigurationManagerImpl.cpp b/src/platform/Darwin/ConfigurationManagerImpl.cpp
@@ -26,6 +26,7 @@
 #include <platform/Darwin/ConfigurationManagerImpl.h>
 
 #include <lib/core/CHIPVendorIdentifiers.hpp>
+#include <lib/core/Global.h>
 #include <platform/CHIPDeviceConfig.h>
 #include <platform/ConfigurationManager.h>
 #include <platform/Darwin/DiagnosticDataProviderImpl.h>
@@ -138,10 +139,13 @@ CHIP_ERROR GetMACAddressFromInterfaces(io_iterator_t primaryInterfaceIterator, u
 }
 #endif // TARGET_OS_OSX
 
+namespace {
+AtomicGlobal<ConfigurationManagerImpl> gInstance;
+}
+
 ConfigurationManagerImpl & ConfigurationManagerImpl::GetDefaultInstance()
 {
-    static ConfigurationManagerImpl sInstance;
-    return sInstance;
+    return gInstance.get();
 }
 
 CHIP_ERROR ConfigurationManagerImpl::Init()

diff --git a/src/platform/Darwin/ConfigurationManagerImpl.h b/src/platform/Darwin/ConfigurationManagerImpl.h
@@ -34,7 +34,7 @@
 namespace chip {
 namespace DeviceLayer {
 
-static constexpr int kCountryCodeLength = 2;
+inline constexpr int kCountryCodeLength = 2;
 
 /**
  * Concrete implementation of the ConfigurationManager singleton object for the Darwin platform.

diff --git a/src/platform/Darwin/DeviceInstanceInfoProviderImpl.cpp b/src/platform/Darwin/DeviceInstanceInfoProviderImpl.cpp
@@ -18,6 +18,7 @@
 
 #include "DeviceInstanceInfoProviderImpl.h"
 
+#include <lib/core/Global.h>
 #include <platform/Darwin/PosixConfig.h>
 
 namespace chip {
@@ -33,5 +34,14 @@ CHIP_ERROR DeviceInstanceInfoProviderImpl::GetProductId(uint16_t & productId)
     return Internal::PosixConfig::ReadConfigValue(Internal::PosixConfig::kConfigKey_ProductId, productId);
 }
 
+namespace {
+AtomicGlobal<DeviceInstanceInfoProviderImpl> gInstance;
+} // namespace
+
+DeviceInstanceInfoProviderImpl & DeviceInstanceInfoProviderMgrImpl()
+{
+    return gInstance.get();
+}
+
 } // namespace DeviceLayer
 } // namespace chip
diff --git a/src/platform/Darwin/DeviceInstanceInfoProviderImpl.h b/src/platform/Darwin/DeviceInstanceInfoProviderImpl.h
@@ -30,15 +30,13 @@ class DeviceInstanceInfoProviderImpl : public Internal::GenericDeviceInstanceInf
     CHIP_ERROR GetVendorId(uint16_t & vendorId) override;
     CHIP_ERROR GetProductId(uint16_t & productId) override;
 
+    DeviceInstanceInfoProviderImpl() : DeviceInstanceInfoProviderImpl(ConfigurationManagerImpl::GetDefaultInstance()) {}
     DeviceInstanceInfoProviderImpl(ConfigurationManagerImpl & configManager) :
         Internal::GenericDeviceInstanceInfoProvider<Internal::PosixConfig>(configManager)
     {}
 };
 
-inline DeviceInstanceInfoProviderImpl & DeviceInstanceInfoProviderMgrImpl()
-{
-    static DeviceInstanceInfoProviderImpl sInstance(ConfigurationManagerImpl::GetDefaultInstance());
-    return sInstance;
-}
+DeviceInstanceInfoProviderImpl & DeviceInstanceInfoProviderMgrImpl();
+
 } // namespace DeviceLayer
 } // namespace chip