From 7f1daf38e98be0e58cfa3dbf302a44630b041d9f Mon Sep 17 00:00:00 2001 From: Justin Wood Date: Wed, 10 Jul 2024 08:14:45 -1000 Subject: [PATCH] Revert "Darwin: Prohibit static initializers in Matter.framework (#34168)" This reverts commit f199ba44c0265ebd6de112bc261551a0d8b38dcd. --- src/app/AttributeAccessInterfaceCache.h | 8 ++-- src/app/EventLoggingTypes.h | 2 +- src/app/EventManagement.cpp | 2 +- src/app/EventManagement.h | 9 ++-- src/app/clusters/descriptor/descriptor.cpp | 7 +-- .../Matter.xcodeproj/project.pbxproj | 2 - .../GenericConfigurationManagerImpl.ipp | 12 ++--- src/lib/support/IntrusiveList.h | 8 ++-- .../ReliableMessageProtocolConfig.cpp | 44 ++++++++----------- src/messaging/ReliableMessageProtocolConfig.h | 5 +-- .../Darwin/ConfigurationManagerImpl.cpp | 8 +--- .../Darwin/ConfigurationManagerImpl.h | 2 +- .../Darwin/DeviceInstanceInfoProviderImpl.cpp | 10 ----- .../Darwin/DeviceInstanceInfoProviderImpl.h | 8 ++-- 14 files changed, 47 insertions(+), 80 deletions(-) diff --git a/src/app/AttributeAccessInterfaceCache.h b/src/app/AttributeAccessInterfaceCache.h index 21e29410f9a367..9268a7096ea9d5 100644 --- a/src/app/AttributeAccessInterfaceCache.h +++ b/src/app/AttributeAccessInterfaceCache.h @@ -48,7 +48,7 @@ class AttributeAccessInterfaceCache kDefinitelyUsed }; - constexpr AttributeAccessInterfaceCache() = default; + AttributeAccessInterfaceCache() { Invalidate(); } /** * @brief Invalidate the whole cache. Must be called every time list of AAI registrations changes. @@ -106,8 +106,6 @@ class AttributeAccessInterfaceCache private: struct AttributeAccessCacheEntry { - constexpr AttributeAccessCacheEntry() = default; - EndpointId endpointId = kInvalidEndpointId; ClusterId clusterId = kInvalidClusterId; AttributeAccessInterface * accessor = nullptr; @@ -139,8 +137,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 index 666cb3886331fe..04a843ebcfbd60 100644 --- a/src/app/EventLoggingTypes.h +++ b/src/app/EventLoggingTypes.h @@ -100,7 +100,7 @@ struct Timestamp kSystem = 0, kEpoch }; - constexpr Timestamp() = default; + Timestamp() {} 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 index 7b210064898b6e..8e6d53c24c9636 100644 --- a/src/app/EventManagement.cpp +++ b/src/app/EventManagement.cpp @@ -32,7 +32,7 @@ using namespace chip::TLV; namespace chip { namespace app { -EventManagement EventManagement::sInstance; +static EventManagement sInstance; /** * @brief diff --git a/src/app/EventManagement.h b/src/app/EventManagement.h index 16e2271d3448b5..ce5a34039ea0fe 100644 --- 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; -inline constexpr uint16_t kRequiredEventField = +constexpr uint16_t kRequiredEventField = (1 << to_underlying(EventDataIB::Tag::kPriority)) | (1 << to_underlying(EventDataIB::Tag::kPath)); /** @@ -388,9 +388,6 @@ class EventManagement void SetScheduledEventInfo(EventNumber & aEventNumber, uint32_t & aInitialWrittenEventBytes) const; private: - constexpr EventManagement() = default; - static EventManagement sInstance; - /** * @brief * Internal structure for traversing events. @@ -558,9 +555,9 @@ class EventManagement MonotonicallyIncreasingCounter * 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 index 4eb68621c5767e..514051226fdcf5 100644 --- a/src/app/clusters/descriptor/descriptor.cpp +++ b/src/app/clusters/descriptor/descriptor.cpp @@ -27,7 +27,6 @@ #include #include #include -#include #include #include @@ -206,9 +205,7 @@ CHIP_ERROR DescriptorAttrAccess::ReadClusterRevision(EndpointId endpoint, Attrib return aEncoder.Encode(kClusterRevision); } -namespace { -Global gAttrAccess; -} +DescriptorAttrAccess gAttrAccess; CHIP_ERROR DescriptorAttrAccess::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) { @@ -247,5 +244,5 @@ CHIP_ERROR DescriptorAttrAccess::Read(const ConcreteReadAttributePath & aPath, A void MatterDescriptorPluginServerInitCallback() { - registerAttributeAccessOverride(&gAttrAccess.get()); + registerAttributeAccessOverride(&gAttrAccess); } diff --git a/src/darwin/Framework/Matter.xcodeproj/project.pbxproj b/src/darwin/Framework/Matter.xcodeproj/project.pbxproj index 05037ef3e16822..df5949868d0012 100644 --- a/src/darwin/Framework/Matter.xcodeproj/project.pbxproj +++ b/src/darwin/Framework/Matter.xcodeproj/project.pbxproj @@ -2497,7 +2497,6 @@ "-Wl,-unexported_symbol,\"__Unwind_*\"", "-Wl,-unexported_symbol,\"_unw_*\"", "-Wl,-hidden-lCHIP", - "-Wl,-no_inits", ); "OTHER_LDFLAGS[sdk=macosx*]" = ( "-framework", @@ -2516,7 +2515,6 @@ "-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 index bb9e56a8bb2a82..9b36f341042aa9 100644 --- a/src/include/platform/internal/GenericConfigurationManagerImpl.ipp +++ b/src/include/platform/internal/GenericConfigurationManagerImpl.ipp @@ -49,8 +49,6 @@ #include #endif -#include - // TODO : may be we can make it configurable #define BLE_ADVERTISEMENT_VERSION 0 @@ -58,9 +56,7 @@ namespace chip { namespace DeviceLayer { namespace Internal { -namespace { -std::optional gFirmwareBuildChipEpochTime; -} +static Optional sFirmwareBuildChipEpochTime; #if CHIP_USE_TRANSITIONAL_COMMISSIONABLE_DATA_PROVIDER @@ -292,9 +288,9 @@ template CHIP_ERROR GenericConfigurationManagerImpl::GetFirmwareBuildChipEpochTime(System::Clock::Seconds32 & chipEpochTime) { // If the setter was called and we have a value in memory, return this. - if (gFirmwareBuildChipEpochTime.has_value()) + if (sFirmwareBuildChipEpochTime.HasValue()) { - chipEpochTime = gFirmwareBuildChipEpochTime.value(); + chipEpochTime = sFirmwareBuildChipEpochTime.Value(); return CHIP_NO_ERROR; } #ifdef CHIP_DEVICE_CONFIG_FIRMWARE_BUILD_TIME_MATTER_EPOCH_S @@ -327,7 +323,7 @@ CHIP_ERROR GenericConfigurationManagerImpl::SetFirmwareBuildChipEpo // // Implementations that can't use the hard-coded time for whatever reason // should set this at each init. - gFirmwareBuildChipEpochTime = chipEpochTime; + sFirmwareBuildChipEpochTime.SetValue(chipEpochTime); return CHIP_NO_ERROR; } diff --git a/src/lib/support/IntrusiveList.h b/src/lib/support/IntrusiveList.h index be9c139babc3dc..961113e8d1ee6b 100644 --- a/src/lib/support/IntrusiveList.h +++ b/src/lib/support/IntrusiveList.h @@ -84,7 +84,7 @@ enum class IntrusiveMode class IntrusiveListNodePrivateBase { public: - constexpr IntrusiveListNodePrivateBase() : mPrev(nullptr), mNext(nullptr) {} + 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; - constexpr IntrusiveListNodePrivateBase(IntrusiveListNodePrivateBase * prev, IntrusiveListNodePrivateBase * next) : + IntrusiveListNodePrivateBase(IntrusiveListNodePrivateBase * prev, IntrusiveListNodePrivateBase * next) : mPrev(prev), mNext(next) {} @@ -284,7 +284,7 @@ class IntrusiveListBase // ^ | // \------------------------------------------/ // - constexpr IntrusiveListBase() : mNode(&mNode, &mNode) {} + IntrusiveListBase() : mNode(&mNode, &mNode) {} ~IntrusiveListBase() { VerifyOrDie(Empty()); @@ -399,7 +399,7 @@ template // nogncheck #endif -#include - namespace chip { using namespace System::Clock::Literals; #if CONFIG_BUILD_FOR_HOST_UNIT_TEST -namespace { -// Use std::optional for globals to avoid static initializers -std::optional gIdleRetransTimeoutOverride; -std::optional gActiveRetransTimeoutOverride; -std::optional gActiveThresholdTimeOverride; -} // namespace +static Optional idleRetransTimeoutOverride = NullOptional; +static Optional activeRetransTimeoutOverride = NullOptional; +static Optional activeThresholdTimeOverride = NullOptional; void OverrideLocalMRPConfig(System::Clock::Timeout idleRetransTimeout, System::Clock::Timeout activeRetransTimeout, System::Clock::Timeout activeThresholdTime) { - gIdleRetransTimeoutOverride = idleRetransTimeout; - gActiveRetransTimeoutOverride = activeRetransTimeout; - gActiveThresholdTimeOverride = activeThresholdTime; + idleRetransTimeoutOverride.SetValue(idleRetransTimeout); + activeRetransTimeoutOverride.SetValue(activeRetransTimeout); + activeThresholdTimeOverride.SetValue(activeThresholdTime); } void ClearLocalMRPConfigOverride() { - gActiveRetransTimeoutOverride = std::nullopt; - gIdleRetransTimeoutOverride = std::nullopt; - gActiveThresholdTimeOverride = std::nullopt; + activeRetransTimeoutOverride.ClearValue(); + idleRetransTimeoutOverride.ClearValue(); + activeThresholdTimeOverride.ClearValue(); } #endif @@ -67,15 +62,14 @@ namespace { // This is not a static member of ReliableMessageProtocolConfig because the free // function GetLocalMRPConfig() needs access to it. -// Use std::optional to avoid a static initializer -std::optional sDynamicLocalMPRConfig; +Optional sDynamicLocalMPRConfig; } // anonymous namespace bool ReliableMessageProtocolConfig::SetLocalMRPConfig(const Optional & localMRPConfig) { auto oldConfig = GetLocalMRPConfig(); - sDynamicLocalMPRConfig = localMRPConfig.std_optional(); + sDynamicLocalMPRConfig = localMRPConfig; return oldConfig != GetLocalMRPConfig(); } #endif // CHIP_DEVICE_CONFIG_ENABLE_DYNAMIC_MRP_CONFIG @@ -95,9 +89,9 @@ Optional 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.has_value()) + if (sDynamicLocalMPRConfig.HasValue()) { - config = sDynamicLocalMPRConfig.value(); + config = sDynamicLocalMPRConfig.Value(); } #endif // CHIP_DEVICE_CONFIG_ENABLE_DYNAMIC_MRP_CONFIG @@ -111,19 +105,19 @@ Optional GetLocalMRPConfig() #endif #if CONFIG_BUILD_FOR_HOST_UNIT_TEST - if (gIdleRetransTimeoutOverride.has_value()) + if (idleRetransTimeoutOverride.HasValue()) { - config.mIdleRetransTimeout = gIdleRetransTimeoutOverride.value(); + config.mIdleRetransTimeout = idleRetransTimeoutOverride.Value(); } - if (gActiveRetransTimeoutOverride.has_value()) + if (activeRetransTimeoutOverride.HasValue()) { - config.mActiveRetransTimeout = gActiveRetransTimeoutOverride.value(); + config.mActiveRetransTimeout = activeRetransTimeoutOverride.Value(); } - if (gActiveThresholdTimeOverride.has_value()) + if (activeThresholdTimeOverride.HasValue()) { - config.mActiveThresholdTime = gActiveThresholdTimeOverride.value(); + config.mActiveThresholdTime = activeRetransTimeoutOverride.Value(); } #endif diff --git a/src/messaging/ReliableMessageProtocolConfig.h b/src/messaging/ReliableMessageProtocolConfig.h index f8d8cfa9bbf2fe..beceee6c52de46 100644 --- a/src/messaging/ReliableMessageProtocolConfig.h +++ b/src/messaging/ReliableMessageProtocolConfig.h @@ -191,9 +191,8 @@ inline constexpr System::Clock::Milliseconds32 kDefaultActiveTime = System::Cloc */ struct ReliableMessageProtocolConfig { - constexpr ReliableMessageProtocolConfig(System::Clock::Milliseconds32 idleInterval, - System::Clock::Milliseconds32 activeInterval, - System::Clock::Milliseconds16 activeThreshold = kDefaultActiveTime) : + 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 index ae00faf58d81a1..10a2ad0a8eb600 100644 --- a/src/platform/Darwin/ConfigurationManagerImpl.cpp +++ b/src/platform/Darwin/ConfigurationManagerImpl.cpp @@ -26,7 +26,6 @@ #include #include -#include #include #include #include @@ -139,13 +138,10 @@ CHIP_ERROR GetMACAddressFromInterfaces(io_iterator_t primaryInterfaceIterator, u } #endif // TARGET_OS_OSX -namespace { -AtomicGlobal gInstance; -} - ConfigurationManagerImpl & ConfigurationManagerImpl::GetDefaultInstance() { - return gInstance.get(); + static ConfigurationManagerImpl sInstance; + return sInstance; } CHIP_ERROR ConfigurationManagerImpl::Init() diff --git a/src/platform/Darwin/ConfigurationManagerImpl.h b/src/platform/Darwin/ConfigurationManagerImpl.h index 5f99243b95adc4..74a13606be9c53 100644 --- a/src/platform/Darwin/ConfigurationManagerImpl.h +++ b/src/platform/Darwin/ConfigurationManagerImpl.h @@ -34,7 +34,7 @@ namespace chip { namespace DeviceLayer { -inline constexpr int kCountryCodeLength = 2; +static 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 index caf4e25191575e..6bb1d1bfd79c65 100644 --- a/src/platform/Darwin/DeviceInstanceInfoProviderImpl.cpp +++ b/src/platform/Darwin/DeviceInstanceInfoProviderImpl.cpp @@ -18,7 +18,6 @@ #include "DeviceInstanceInfoProviderImpl.h" -#include #include namespace chip { @@ -34,14 +33,5 @@ CHIP_ERROR DeviceInstanceInfoProviderImpl::GetProductId(uint16_t & productId) return Internal::PosixConfig::ReadConfigValue(Internal::PosixConfig::kConfigKey_ProductId, productId); } -namespace { -AtomicGlobal 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 index 48603b583d9f86..630fdbe24c8a12 100644 --- a/src/platform/Darwin/DeviceInstanceInfoProviderImpl.h +++ b/src/platform/Darwin/DeviceInstanceInfoProviderImpl.h @@ -30,13 +30,15 @@ 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(configManager) {} }; -DeviceInstanceInfoProviderImpl & DeviceInstanceInfoProviderMgrImpl(); - +inline DeviceInstanceInfoProviderImpl & DeviceInstanceInfoProviderMgrImpl() +{ + static DeviceInstanceInfoProviderImpl sInstance(ConfigurationManagerImpl::GetDefaultInstance()); + return sInstance; +} } // namespace DeviceLayer } // namespace chip