Skip to content

Commit 23a912b

Browse files
ksperling-applebzbarsky-apple
authored and
austina-csa
committed
Darwin: Prohibit static initializers in Matter.framework (project-chip#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 <bzbarsky@apple.com> * 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 <bzbarsky@apple.com>
1 parent 8130655 commit 23a912b

14 files changed

+80
-47
lines changed

‎src/app/AttributeAccessInterfaceCache.h

+5-3
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ class AttributeAccessInterfaceCache
4848
kDefinitelyUsed
4949
};
5050

51-
AttributeAccessInterfaceCache() { Invalidate(); }
51+
constexpr AttributeAccessInterfaceCache() = default;
5252

5353
/**
5454
* @brief Invalidate the whole cache. Must be called every time list of AAI registrations changes.
@@ -106,6 +106,8 @@ class AttributeAccessInterfaceCache
106106
private:
107107
struct AttributeAccessCacheEntry
108108
{
109+
constexpr AttributeAccessCacheEntry() = default;
110+
109111
EndpointId endpointId = kInvalidEndpointId;
110112
ClusterId clusterId = kInvalidClusterId;
111113
AttributeAccessInterface * accessor = nullptr;
@@ -137,8 +139,8 @@ class AttributeAccessInterfaceCache
137139
return &mCacheSlots[0];
138140
}
139141

140-
AttributeAccessCacheEntry mCacheSlots[1];
141-
AttributeAccessCacheEntry mLastUnusedEntry;
142+
AttributeAccessCacheEntry mCacheSlots[1] = {};
143+
AttributeAccessCacheEntry mLastUnusedEntry{};
142144
};
143145

144146
} // namespace app

‎src/app/EventLoggingTypes.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ struct Timestamp
100100
kSystem = 0,
101101
kEpoch
102102
};
103-
Timestamp() {}
103+
constexpr Timestamp() = default;
104104
Timestamp(Type aType, uint64_t aValue) : mType(aType), mValue(aValue) {}
105105
Timestamp(System::Clock::Timestamp aValue) : mType(Type::kSystem), mValue(aValue.count()) {}
106106
static Timestamp Epoch(System::Clock::Timestamp aValue)

‎src/app/EventManagement.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ using namespace chip::TLV;
3232

3333
namespace chip {
3434
namespace app {
35-
static EventManagement sInstance;
35+
EventManagement EventManagement::sInstance;
3636

3737
/**
3838
* @brief

‎src/app/EventManagement.h

+6-3
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ namespace app {
7373
inline constexpr const uint32_t kEventManagementProfile = 0x1;
7474
inline constexpr const uint32_t kFabricIndexTag = 0x1;
7575
inline constexpr size_t kMaxEventSizeReserve = 512;
76-
constexpr uint16_t kRequiredEventField =
76+
inline constexpr uint16_t kRequiredEventField =
7777
(1 << to_underlying(EventDataIB::Tag::kPriority)) | (1 << to_underlying(EventDataIB::Tag::kPath));
7878

7979
/**
@@ -388,6 +388,9 @@ class EventManagement
388388
void SetScheduledEventInfo(EventNumber & aEventNumber, uint32_t & aInitialWrittenEventBytes) const;
389389

390390
private:
391+
constexpr EventManagement() = default;
392+
static EventManagement sInstance;
393+
391394
/**
392395
* @brief
393396
* Internal structure for traversing events.
@@ -555,9 +558,9 @@ class EventManagement
555558
MonotonicallyIncreasingCounter<EventNumber> * mpEventNumberCounter = nullptr;
556559

557560
EventNumber mLastEventNumber = 0; ///< Last event Number vended
558-
Timestamp mLastEventTimestamp; ///< The timestamp of the last event in this buffer
561+
Timestamp mLastEventTimestamp{}; ///< The timestamp of the last event in this buffer
559562

560-
System::Clock::Milliseconds64 mMonotonicStartupTime;
563+
System::Clock::Milliseconds64 mMonotonicStartupTime{};
561564
};
562565
} // namespace app
563566
} // namespace chip

‎src/app/clusters/descriptor/descriptor.cpp

+5-2
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include <app/AttributeAccessInterfaceRegistry.h>
2828
#include <app/util/attribute-storage.h>
2929
#include <app/util/endpoint-config-api.h>
30+
#include <lib/core/Global.h>
3031
#include <lib/support/CodeUtils.h>
3132
#include <lib/support/logging/CHIPLogging.h>
3233

@@ -205,7 +206,9 @@ CHIP_ERROR DescriptorAttrAccess::ReadClusterRevision(EndpointId endpoint, Attrib
205206
return aEncoder.Encode(kClusterRevision);
206207
}
207208

208-
DescriptorAttrAccess gAttrAccess;
209+
namespace {
210+
Global<DescriptorAttrAccess> gAttrAccess;
211+
}
209212

210213
CHIP_ERROR DescriptorAttrAccess::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder)
211214
{
@@ -244,5 +247,5 @@ CHIP_ERROR DescriptorAttrAccess::Read(const ConcreteReadAttributePath & aPath, A
244247

245248
void MatterDescriptorPluginServerInitCallback()
246249
{
247-
registerAttributeAccessOverride(&gAttrAccess);
250+
registerAttributeAccessOverride(&gAttrAccess.get());
248251
}

‎src/darwin/Framework/Matter.xcodeproj/project.pbxproj

+2
Original file line numberDiff line numberDiff line change
@@ -2497,6 +2497,7 @@
24972497
"-Wl,-unexported_symbol,\"__Unwind_*\"",
24982498
"-Wl,-unexported_symbol,\"_unw_*\"",
24992499
"-Wl,-hidden-lCHIP",
2500+
"-Wl,-no_inits",
25002501
);
25012502
"OTHER_LDFLAGS[sdk=macosx*]" = (
25022503
"-framework",
@@ -2515,6 +2516,7 @@
25152516
"-Wl,-unexported_symbol,\"__Unwind_*\"",
25162517
"-Wl,-unexported_symbol,\"_unw_*\"",
25172518
"-Wl,-hidden-lCHIP",
2519+
"-Wl,-no_inits",
25182520
);
25192521
PRODUCT_BUNDLE_IDENTIFIER = com.csa.matter;
25202522
PRODUCT_NAME = "$(TARGET_NAME:c99extidentifier)";

‎src/include/platform/internal/GenericConfigurationManagerImpl.ipp

+8-4
Original file line numberDiff line numberDiff line change
@@ -49,14 +49,18 @@
4949
#include <platform/ThreadStackManager.h>
5050
#endif
5151

52+
#include <optional>
53+
5254
// TODO : may be we can make it configurable
5355
#define BLE_ADVERTISEMENT_VERSION 0
5456

5557
namespace chip {
5658
namespace DeviceLayer {
5759
namespace Internal {
5860

59-
static Optional<System::Clock::Seconds32> sFirmwareBuildChipEpochTime;
61+
namespace {
62+
std::optional<System::Clock::Seconds32> gFirmwareBuildChipEpochTime;
63+
}
6064

6165
#if CHIP_USE_TRANSITIONAL_COMMISSIONABLE_DATA_PROVIDER
6266

@@ -288,9 +292,9 @@ template <class ConfigClass>
288292
CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::GetFirmwareBuildChipEpochTime(System::Clock::Seconds32 & chipEpochTime)
289293
{
290294
// If the setter was called and we have a value in memory, return this.
291-
if (sFirmwareBuildChipEpochTime.HasValue())
295+
if (gFirmwareBuildChipEpochTime.has_value())
292296
{
293-
chipEpochTime = sFirmwareBuildChipEpochTime.Value();
297+
chipEpochTime = gFirmwareBuildChipEpochTime.value();
294298
return CHIP_NO_ERROR;
295299
}
296300
#ifdef CHIP_DEVICE_CONFIG_FIRMWARE_BUILD_TIME_MATTER_EPOCH_S
@@ -323,7 +327,7 @@ CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::SetFirmwareBuildChipEpo
323327
//
324328
// Implementations that can't use the hard-coded time for whatever reason
325329
// should set this at each init.
326-
sFirmwareBuildChipEpochTime.SetValue(chipEpochTime);
330+
gFirmwareBuildChipEpochTime = chipEpochTime;
327331
return CHIP_NO_ERROR;
328332
}
329333

‎src/lib/support/IntrusiveList.h

+4-4
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ enum class IntrusiveMode
8484
class IntrusiveListNodePrivateBase
8585
{
8686
public:
87-
IntrusiveListNodePrivateBase() : mPrev(nullptr), mNext(nullptr) {}
87+
constexpr IntrusiveListNodePrivateBase() : mPrev(nullptr), mNext(nullptr) {}
8888
~IntrusiveListNodePrivateBase() { VerifyOrDie(!IsInList()); }
8989

9090
// Note: The copy construct/assignment is not provided because the list node state is not copyable.
@@ -98,7 +98,7 @@ class IntrusiveListNodePrivateBase
9898

9999
private:
100100
friend class IntrusiveListBase;
101-
IntrusiveListNodePrivateBase(IntrusiveListNodePrivateBase * prev, IntrusiveListNodePrivateBase * next) :
101+
constexpr IntrusiveListNodePrivateBase(IntrusiveListNodePrivateBase * prev, IntrusiveListNodePrivateBase * next) :
102102
mPrev(prev), mNext(next)
103103
{}
104104

@@ -284,7 +284,7 @@ class IntrusiveListBase
284284
// ^ |
285285
// \------------------------------------------/
286286
//
287-
IntrusiveListBase() : mNode(&mNode, &mNode) {}
287+
constexpr IntrusiveListBase() : mNode(&mNode, &mNode) {}
288288
~IntrusiveListBase()
289289
{
290290
VerifyOrDie(Empty());
@@ -399,7 +399,7 @@ template <typename T, IntrusiveMode Mode = IntrusiveMode::Strict, typename Hook
399399
class IntrusiveList : public IntrusiveListBase
400400
{
401401
public:
402-
IntrusiveList() : IntrusiveListBase() {}
402+
constexpr IntrusiveList() : IntrusiveListBase() {}
403403

404404
IntrusiveList(IntrusiveList &&) = default;
405405
IntrusiveList & operator=(IntrusiveList &&) = default;

‎src/messaging/ReliableMessageProtocolConfig.cpp

+25-19
Original file line numberDiff line numberDiff line change
@@ -32,28 +32,33 @@
3232
#include <app/icd/server/ICDConfigurationData.h> // nogncheck
3333
#endif
3434

35+
#include <optional>
36+
3537
namespace chip {
3638

3739
using namespace System::Clock::Literals;
3840

3941
#if CONFIG_BUILD_FOR_HOST_UNIT_TEST
40-
static Optional<System::Clock::Timeout> idleRetransTimeoutOverride = NullOptional;
41-
static Optional<System::Clock::Timeout> activeRetransTimeoutOverride = NullOptional;
42-
static Optional<System::Clock::Timeout> activeThresholdTimeOverride = NullOptional;
42+
namespace {
43+
// Use std::optional for globals to avoid static initializers
44+
std::optional<System::Clock::Timeout> gIdleRetransTimeoutOverride;
45+
std::optional<System::Clock::Timeout> gActiveRetransTimeoutOverride;
46+
std::optional<System::Clock::Timeout> gActiveThresholdTimeOverride;
47+
} // namespace
4348

4449
void OverrideLocalMRPConfig(System::Clock::Timeout idleRetransTimeout, System::Clock::Timeout activeRetransTimeout,
4550
System::Clock::Timeout activeThresholdTime)
4651
{
47-
idleRetransTimeoutOverride.SetValue(idleRetransTimeout);
48-
activeRetransTimeoutOverride.SetValue(activeRetransTimeout);
49-
activeThresholdTimeOverride.SetValue(activeThresholdTime);
52+
gIdleRetransTimeoutOverride = idleRetransTimeout;
53+
gActiveRetransTimeoutOverride = activeRetransTimeout;
54+
gActiveThresholdTimeOverride = activeThresholdTime;
5055
}
5156

5257
void ClearLocalMRPConfigOverride()
5358
{
54-
activeRetransTimeoutOverride.ClearValue();
55-
idleRetransTimeoutOverride.ClearValue();
56-
activeThresholdTimeOverride.ClearValue();
59+
gActiveRetransTimeoutOverride = std::nullopt;
60+
gIdleRetransTimeoutOverride = std::nullopt;
61+
gActiveThresholdTimeOverride = std::nullopt;
5762
}
5863
#endif
5964

@@ -62,14 +67,15 @@ namespace {
6267

6368
// This is not a static member of ReliableMessageProtocolConfig because the free
6469
// function GetLocalMRPConfig() needs access to it.
65-
Optional<ReliableMessageProtocolConfig> sDynamicLocalMPRConfig;
70+
// Use std::optional to avoid a static initializer
71+
std::optional<ReliableMessageProtocolConfig> sDynamicLocalMPRConfig;
6672

6773
} // anonymous namespace
6874

6975
bool ReliableMessageProtocolConfig::SetLocalMRPConfig(const Optional<ReliableMessageProtocolConfig> & localMRPConfig)
7076
{
7177
auto oldConfig = GetLocalMRPConfig();
72-
sDynamicLocalMPRConfig = localMRPConfig;
78+
sDynamicLocalMPRConfig = localMRPConfig.std_optional();
7379
return oldConfig != GetLocalMRPConfig();
7480
}
7581
#endif // CHIP_DEVICE_CONFIG_ENABLE_DYNAMIC_MRP_CONFIG
@@ -89,9 +95,9 @@ Optional<ReliableMessageProtocolConfig> GetLocalMRPConfig()
8995
ReliableMessageProtocolConfig config(CHIP_CONFIG_MRP_LOCAL_IDLE_RETRY_INTERVAL, CHIP_CONFIG_MRP_LOCAL_ACTIVE_RETRY_INTERVAL);
9096

9197
#if CHIP_DEVICE_CONFIG_ENABLE_DYNAMIC_MRP_CONFIG
92-
if (sDynamicLocalMPRConfig.HasValue())
98+
if (sDynamicLocalMPRConfig.has_value())
9399
{
94-
config = sDynamicLocalMPRConfig.Value();
100+
config = sDynamicLocalMPRConfig.value();
95101
}
96102
#endif // CHIP_DEVICE_CONFIG_ENABLE_DYNAMIC_MRP_CONFIG
97103

@@ -105,19 +111,19 @@ Optional<ReliableMessageProtocolConfig> GetLocalMRPConfig()
105111
#endif
106112

107113
#if CONFIG_BUILD_FOR_HOST_UNIT_TEST
108-
if (idleRetransTimeoutOverride.HasValue())
114+
if (gIdleRetransTimeoutOverride.has_value())
109115
{
110-
config.mIdleRetransTimeout = idleRetransTimeoutOverride.Value();
116+
config.mIdleRetransTimeout = gIdleRetransTimeoutOverride.value();
111117
}
112118

113-
if (activeRetransTimeoutOverride.HasValue())
119+
if (gActiveRetransTimeoutOverride.has_value())
114120
{
115-
config.mActiveRetransTimeout = activeRetransTimeoutOverride.Value();
121+
config.mActiveRetransTimeout = gActiveRetransTimeoutOverride.value();
116122
}
117123

118-
if (activeThresholdTimeOverride.HasValue())
124+
if (gActiveThresholdTimeOverride.has_value())
119125
{
120-
config.mActiveThresholdTime = activeRetransTimeoutOverride.Value();
126+
config.mActiveThresholdTime = gActiveThresholdTimeOverride.value();
121127
}
122128
#endif
123129

‎src/messaging/ReliableMessageProtocolConfig.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -191,8 +191,9 @@ inline constexpr System::Clock::Milliseconds32 kDefaultActiveTime = System::Cloc
191191
*/
192192
struct ReliableMessageProtocolConfig
193193
{
194-
ReliableMessageProtocolConfig(System::Clock::Milliseconds32 idleInterval, System::Clock::Milliseconds32 activeInterval,
195-
System::Clock::Milliseconds16 activeThreshold = kDefaultActiveTime) :
194+
constexpr ReliableMessageProtocolConfig(System::Clock::Milliseconds32 idleInterval,
195+
System::Clock::Milliseconds32 activeInterval,
196+
System::Clock::Milliseconds16 activeThreshold = kDefaultActiveTime) :
196197
mIdleRetransTimeout(idleInterval),
197198
mActiveRetransTimeout(activeInterval), mActiveThresholdTime(activeThreshold)
198199
{}

‎src/platform/Darwin/ConfigurationManagerImpl.cpp

+6-2
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include <platform/Darwin/ConfigurationManagerImpl.h>
2727

2828
#include <lib/core/CHIPVendorIdentifiers.hpp>
29+
#include <lib/core/Global.h>
2930
#include <platform/CHIPDeviceConfig.h>
3031
#include <platform/ConfigurationManager.h>
3132
#include <platform/Darwin/DiagnosticDataProviderImpl.h>
@@ -138,10 +139,13 @@ CHIP_ERROR GetMACAddressFromInterfaces(io_iterator_t primaryInterfaceIterator, u
138139
}
139140
#endif // TARGET_OS_OSX
140141

142+
namespace {
143+
AtomicGlobal<ConfigurationManagerImpl> gInstance;
144+
}
145+
141146
ConfigurationManagerImpl & ConfigurationManagerImpl::GetDefaultInstance()
142147
{
143-
static ConfigurationManagerImpl sInstance;
144-
return sInstance;
148+
return gInstance.get();
145149
}
146150

147151
CHIP_ERROR ConfigurationManagerImpl::Init()

‎src/platform/Darwin/ConfigurationManagerImpl.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
namespace chip {
3535
namespace DeviceLayer {
3636

37-
static constexpr int kCountryCodeLength = 2;
37+
inline constexpr int kCountryCodeLength = 2;
3838

3939
/**
4040
* Concrete implementation of the ConfigurationManager singleton object for the Darwin platform.

‎src/platform/Darwin/DeviceInstanceInfoProviderImpl.cpp

+10
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
#include "DeviceInstanceInfoProviderImpl.h"
2020

21+
#include <lib/core/Global.h>
2122
#include <platform/Darwin/PosixConfig.h>
2223

2324
namespace chip {
@@ -33,5 +34,14 @@ CHIP_ERROR DeviceInstanceInfoProviderImpl::GetProductId(uint16_t & productId)
3334
return Internal::PosixConfig::ReadConfigValue(Internal::PosixConfig::kConfigKey_ProductId, productId);
3435
}
3536

37+
namespace {
38+
AtomicGlobal<DeviceInstanceInfoProviderImpl> gInstance;
39+
} // namespace
40+
41+
DeviceInstanceInfoProviderImpl & DeviceInstanceInfoProviderMgrImpl()
42+
{
43+
return gInstance.get();
44+
}
45+
3646
} // namespace DeviceLayer
3747
} // namespace chip

‎src/platform/Darwin/DeviceInstanceInfoProviderImpl.h

+3-5
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,13 @@ class DeviceInstanceInfoProviderImpl : public Internal::GenericDeviceInstanceInf
3030
CHIP_ERROR GetVendorId(uint16_t & vendorId) override;
3131
CHIP_ERROR GetProductId(uint16_t & productId) override;
3232

33+
DeviceInstanceInfoProviderImpl() : DeviceInstanceInfoProviderImpl(ConfigurationManagerImpl::GetDefaultInstance()) {}
3334
DeviceInstanceInfoProviderImpl(ConfigurationManagerImpl & configManager) :
3435
Internal::GenericDeviceInstanceInfoProvider<Internal::PosixConfig>(configManager)
3536
{}
3637
};
3738

38-
inline DeviceInstanceInfoProviderImpl & DeviceInstanceInfoProviderMgrImpl()
39-
{
40-
static DeviceInstanceInfoProviderImpl sInstance(ConfigurationManagerImpl::GetDefaultInstance());
41-
return sInstance;
42-
}
39+
DeviceInstanceInfoProviderImpl & DeviceInstanceInfoProviderMgrImpl();
40+
4341
} // namespace DeviceLayer
4442
} // namespace chip

0 commit comments

Comments
 (0)