Assert that the Matter stack lock is held while reading/writing attributes (#15152)

* Assert that the Matter stack lock is held while reading/writing attributes.

Several changes here:

1) Add an assert that when attributes are being read/written the
   Matter stack lock is held, so the read/write does not race with
   interaction model messages touching the attribute store.
2) Implement the Matter stack lock assertion bits on FreeRTOS.
3) Add FreeRTOS to the set of configurations that actually enable
   Matter stack lock assertions.
4) For the configurations that enable the assertions, change the
   default behavior from "log" to "fatal", so we actually notice the

* Remove stray debugging log.

* Make p6 and qpg CI compile.

* Also assert in MatterReportingAttributeChangeCallback.

Author: bzbarsky-apple

config/qpg/chip-gn/args.gni

@@ -32,6 +32,10 @@ qpg_cc = "arm-none-eabi-gcc"
 qpg_cxx = "arm-none-eabi-g++"
 chip_mdns = "platform"
 
+# Disable lock tracking, since our FreeRTOS configuration does not set
+# INCLUDE_xSemaphoreGetMutexHolder
+chip_stack_lock_tracking = "none"
+
 chip_project_config_include_dirs =
     [ "//examples/platform/qpg/project_include/" ]
 chip_project_config_include = "<CHIPProjectConfig.h>"

examples/all-clusters-app/p6/args.gni

@@ -18,3 +18,9 @@ import("${chip_root}/src/platform/P6/args.gni")
 
 p6_target_project =
     get_label_info(":all_clusters_app_sdk_sources", "label_no_toolchain")
+
+declare_args() {
+  # Disable lock tracking, since our FreeRTOS configuration does not set
+  # INCLUDE_xSemaphoreGetMutexHolder
+  chip_stack_lock_tracking = "none"
+}

examples/lighting-app/p6/args.gni

@@ -18,3 +18,9 @@ import("${chip_root}/src/platform/P6/args.gni")
 
 p6_target_project =
     get_label_info(":lighting_app_sdk_sources", "label_no_toolchain")
+
+declare_args() {
+  # Disable lock tracking, since our FreeRTOS configuration does not set
+  # INCLUDE_xSemaphoreGetMutexHolder
+  chip_stack_lock_tracking = "none"
+}

examples/lighting-app/qpg/args.gni

@@ -20,6 +20,10 @@ qpg_sdk_target = get_label_info(":sdk", "label_no_toolchain")
 
 declare_args() {
   chip_enable_ota_requestor = true
+
+  # Disable lock tracking, since our FreeRTOS configuration does not set
+  # INCLUDE_xSemaphoreGetMutexHolder
+  chip_stack_lock_tracking = "none"
 }
 
 pw_log_BACKEND = "${chip_root}/src/lib/support/pw_log_chip"

examples/lock-app/p6/args.gni

@@ -18,3 +18,9 @@ import("${chip_root}/src/platform/P6/args.gni")
 
 p6_target_project =
     get_label_info(":lock_app_sdk_sources", "label_no_toolchain")
+
+declare_args() {
+  # Disable lock tracking, since our FreeRTOS configuration does not set
+  # INCLUDE_xSemaphoreGetMutexHolder
+  chip_stack_lock_tracking = "none"
+}

examples/lock-app/qpg/args.gni

@@ -20,6 +20,10 @@ qpg_sdk_target = get_label_info(":sdk", "label_no_toolchain")
 
 declare_args() {
   chip_enable_ota_requestor = true
+
+  # Disable lock tracking, since our FreeRTOS configuration does not set
+  # INCLUDE_xSemaphoreGetMutexHolder
+  chip_stack_lock_tracking = "none"
 }
 
 pw_log_BACKEND = "${chip_root}/src/lib/support/pw_log_chip"

examples/persistent-storage/qpg/args.gni

@@ -17,3 +17,9 @@ import("//build_overrides/chip.gni")
 import("${chip_root}/examples/platform/qpg/args.gni")
 qpg_sdk_target = get_label_info(":sdk", "label_no_toolchain")
 chip_enable_openthread = false
+
+declare_args() {
+  # Disable lock tracking, since our FreeRTOS configuration does not set
+  # INCLUDE_xSemaphoreGetMutexHolder
+  chip_stack_lock_tracking = "none"
+}

src/app/util/attribute-storage.cpp

@@ -47,6 +47,7 @@
 #include <app/util/attribute-storage.h>
 #include <lib/support/CodeUtils.h>
 #include <lib/support/logging/CHIPLogging.h>
+#include <platform/LockTracker.h>
 
 #include <app-common/zap-generated/attribute-type.h>
 #include <app-common/zap-generated/callback.h>
@@ -549,6 +550,8 @@ bool emAfMatchAttribute(const EmberAfCluster * cluster, const EmberAfAttributeMe
 EmberAfStatus emAfReadOrWriteAttribute(EmberAfAttributeSearchRecord * attRecord, const EmberAfAttributeMetadata ** metadata,
                                        uint8_t * buffer, uint16_t readLength, bool write)
 {
+    assertChipStackLockedByCurrentThread();
+
     uint16_t attributeOffsetIndex = 0;
 
     for (uint8_t ep = 0; ep < emberAfEndpointCount(); ep++)

src/app/util/ember-compatibility-functions.cpp

@@ -42,6 +42,7 @@
 #include <lib/support/CodeUtils.h>
 #include <lib/support/SafeInt.h>
 #include <lib/support/TypeTraits.h>
+#include <platform/LockTracker.h>
 #include <protocols/interaction_model/Constants.h>
 
 #include <app-common/zap-generated/att-storage.h>
@@ -1029,6 +1030,10 @@ void MatterReportingAttributeChangeCallback(EndpointId endpoint, ClusterId clust
 
 void MatterReportingAttributeChangeCallback(EndpointId endpoint, ClusterId clusterId, AttributeId attributeId)
 {
+    // Attribute writes have asserted this already, but this assert should catch
+    // applications notifying about changes from their end.
+    assertChipStackLockedByCurrentThread();
+
     ClusterInfo info;
     info.mClusterId   = clusterId;
     info.mAttributeId = attributeId;

src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.cpp

@@ -104,6 +104,27 @@ void GenericPlatformManagerImpl_FreeRTOS<ImplClass>::_UnlockChipStack(void)
     xSemaphoreGive(mChipStackLock);
 }
 
+#if CHIP_STACK_LOCK_TRACKING_ENABLED
+template <class ImplClass>
+bool GenericPlatformManagerImpl_FreeRTOS<ImplClass>::_IsChipStackLockedByCurrentThread() const
+{
+    // We can't check for INCLUDE_xTaskGetCurrentTaskHandle because it's often
+    // _not_ set, but xTaskGetCurrentTaskHandle works anyway because
+    // configUSE_MUTEXES is set.  So in practice, xTaskGetCurrentTaskHandle can
+    // be assumed to be available here.
+#if INCLUDE_xSemaphoreGetMutexHolder != 1
+#error Must either set INCLUDE_xSemaphoreGetMutexHolder = 1 in FreeRTOSConfig.h or set chip_stack_lock_tracking = "none" in Matter gn configuration.
+#endif
+    // If we have not started our event loop yet, return true because in that
+    // case we can't be racing against the (not yet started) event loop.
+    //
+    // Similarly, if mChipStackLock has not been created yet, might as well
+    // return true.
+    return (mEventLoopTask == nullptr) || (mChipStackLock == nullptr) ||
+        (xSemaphoreGetMutexHolder(mChipStackLock) == xTaskGetCurrentTaskHandle());
+}
+#endif // CHIP_STACK_LOCK_TRACKING_ENABLED
+
 template <class ImplClass>
 CHIP_ERROR GenericPlatformManagerImpl_FreeRTOS<ImplClass>::_PostEvent(const ChipDeviceEvent * event)
 {

src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.h

@@ -78,6 +78,10 @@ class GenericPlatformManagerImpl_FreeRTOS : public GenericPlatformManagerImpl<Im
     CHIP_ERROR _StartChipTimer(System::Clock::Timeout duration);
     CHIP_ERROR _Shutdown(void);
 
+#if CHIP_STACK_LOCK_TRACKING_ENABLED
+    bool _IsChipStackLockedByCurrentThread() const;
+#endif // CHIP_STACK_LOCK_TRACKING_ENABLED
+
     // ===== Methods available to the implementation subclass.
 
     void PostEventFromISR(const ChipDeviceEvent * event, BaseType_t & yieldRequired);

src/platform/BUILD.gn

@@ -69,9 +69,9 @@ if (chip_device_platform != "none") {
 
   if (chip_stack_lock_tracking == "auto") {
     if (chip_device_platform == "linux" || chip_device_platform == "tizen" ||
-        chip_device_platform == "android") {
+        chip_device_platform == "android" || current_os == "freertos") {
       # TODO: should be fatal for development. Change once bugs are fixed
-      chip_stack_lock_tracking = "log"
+      chip_stack_lock_tracking = "fatal"
     } else {
       # TODO: may want to enable at least logging for embedded to find bugs
       # this needs tuning depending on how many resources various platforms have