Break dependency cycle between CHIPMem and logging. (#29685)
CHIPMem used things from CodeUtils.h which used logging, but logging might need
to allocate memory via Memory::Alloc to actually work.
This breaks the cycle (both conceptual and real: we could try to Memory::Alloc
to log the fact that Memory::Init had not been called!), by making CHIPMem not
use any macros that involve logging, at the cost of not having nice error
messages when CHIPMem invariants are violated.
diff --git a/src/lib/support/BUILD.gn b/src/lib/support/BUILD.gn
index ae745b5..e516b6e 100644
--- a/src/lib/support/BUILD.gn
+++ b/src/lib/support/BUILD.gn
@@ -73,6 +73,40 @@
]
}
+source_set("verifymacros_no_logging") {
+ sources = [ "VerificationMacrosNoLogging.h" ]
+
+ public_deps = [ "${nlassert_root}:nlassert" ]
+}
+
+source_set("safeint") {
+ sources = [ "SafeInt.h" ]
+}
+
+source_set("memory") {
+ sources = [
+ "CHIPMem.cpp",
+ "CHIPMem.h",
+ "CHIPPlatformMemory.cpp",
+ "CHIPPlatformMemory.h",
+ ]
+
+ if (chip_config_memory_management == "simple") {
+ sources += [ "CHIPMem-Simple.cpp" ]
+ }
+ if (chip_config_memory_management == "malloc") {
+ sources += [ "CHIPMem-Malloc.cpp" ]
+ }
+
+ public_deps = [ "${chip_root}/src/lib/core:error" ]
+
+ deps = [
+ ":safeint",
+ ":verifymacros_no_logging",
+ "${chip_root}/src/lib/core:chip_config_header",
+ ]
+}
+
source_set("chip_version_header") {
sources = get_target_outputs(":gen_chip_version")
@@ -97,11 +131,7 @@
"BytesToHex.h",
"CHIPArgParser.cpp",
"CHIPCounter.h",
- "CHIPMem.cpp",
- "CHIPMem.h",
"CHIPMemString.h",
- "CHIPPlatformMemory.cpp",
- "CHIPPlatformMemory.h",
"CodeUtils.h",
"DLLUtil.h",
"DefaultStorageKeyAllocator.h",
@@ -124,7 +154,6 @@
"PrivateHeap.cpp",
"PrivateHeap.h",
"ReferenceCountedHandle.h",
- "SafeInt.h",
"SerializableIntegerSet.cpp",
"SerializableIntegerSet.h",
"SetupDiscriminator.h",
@@ -172,6 +201,9 @@
":attributes",
":chip_version_header",
":logging_constants",
+ ":memory",
+ ":safeint",
+ ":verifymacros_no_logging",
"${chip_root}/src/lib/core:chip_config_header",
"${chip_root}/src/lib/core:error",
"${chip_root}/src/platform:platform_buildconfig",
@@ -203,12 +235,6 @@
":storage_audit_config",
]
- if (chip_config_memory_management == "simple") {
- sources += [ "CHIPMem-Simple.cpp" ]
- }
- if (chip_config_memory_management == "malloc") {
- sources += [ "CHIPMem-Malloc.cpp" ]
- }
if (chip_with_nlfaultinjection) {
sources += [
"CHIPFaultInjection.cpp",
diff --git a/src/lib/support/CHIPMem-Malloc.cpp b/src/lib/support/CHIPMem-Malloc.cpp
index ec247a5..7ef0425 100644
--- a/src/lib/support/CHIPMem-Malloc.cpp
+++ b/src/lib/support/CHIPMem-Malloc.cpp
@@ -25,7 +25,7 @@
#include <lib/core/CHIPConfig.h>
#include <lib/support/CHIPMem.h>
-#include <lib/support/CodeUtils.h>
+#include <lib/support/VerificationMacrosNoLogging.h>
#include <stdlib.h>
@@ -57,28 +57,33 @@
static void VerifyInitialized(const char * func)
{
- VerifyOrDieWithMsg(memoryInitialized, Support, "ABORT: chip::Platform::%s() called before chip::Platform::MemoryInit()\n",
- func);
+ // Logging can use Memory::Alloc, so we can't use logging with our
+ // VerifyOrDie bits here.
+ VerifyOrDieWithoutLogging(memoryInitialized);
}
-#define VERIFY_POINTER(p) \
- VerifyOrDieWithMsg((p) == nullptr || MemoryDebugCheckPointer((p)), Support, \
- "ABORT: chip::Platform::%s() found corruption on %p\n", __func__, (p))
+// Logging can use Memory::Alloc, so we can't use logging with our
+// VerifyOrDie bits here.
+#define VERIFY_POINTER(p) VerifyOrDieWithoutLogging((p) == nullptr || MemoryDebugCheckPointer((p)))
#endif
CHIP_ERROR MemoryAllocatorInit(void * buf, size_t bufSize)
{
+ // Logging can use Memory::Alloc, so we can't use logging with our
+ // VerifyOrDie bits here.
#ifndef NDEBUG
- VerifyOrDieWithMsg(memoryInitialized++ == 0, Support, "ABORT: chip::Platform::MemoryInit() called twice.\n");
+ VerifyOrDieWithoutLogging(memoryInitialized++ == 0);
#endif
return CHIP_NO_ERROR;
}
void MemoryAllocatorShutdown()
{
+ // Logging can use Memory::Alloc, so we can't use logging with our
+ // VerifyOrDie bits here.
#ifndef NDEBUG
- VerifyOrDieWithMsg(--memoryInitialized == 0, Support, "ABORT: chip::Platform::MemoryShutdown() called twice.\n");
+ VerifyOrDieWithoutLogging(--memoryInitialized == 0);
#endif
#if CHIP_CONFIG_MEMORY_DEBUG_DMALLOC
dmalloc_shutdown();
diff --git a/src/lib/support/CHIPMem-Simple.cpp b/src/lib/support/CHIPMem-Simple.cpp
index 3a1e0aa..8816afe 100644
--- a/src/lib/support/CHIPMem-Simple.cpp
+++ b/src/lib/support/CHIPMem-Simple.cpp
@@ -20,7 +20,7 @@
#include <string.h>
-#include <lib/support/CodeUtils.h>
+#include <lib/support/VerificationMacrosNoLogging.h>
#include <system/SystemMutex.h>
namespace chip {
diff --git a/src/lib/support/CodeUtils.h b/src/lib/support/CodeUtils.h
index 60780a6..bbb1854 100644
--- a/src/lib/support/CodeUtils.h
+++ b/src/lib/support/CodeUtils.h
@@ -29,6 +29,7 @@
#include <lib/core/CHIPConfig.h>
#include <lib/core/CHIPError.h>
#include <lib/core/ErrorStr.h>
+#include <lib/support/VerificationMacrosNoLogging.h>
#include <lib/support/logging/TextOnlyLogging.h>
/**
@@ -547,7 +548,7 @@
#define VerifyOrDie(aCondition) \
nlABORT_ACTION(aCondition, ChipLogDetail(Support, "VerifyOrDie failure at %s:%d: %s", __FILE__, __LINE__, #aCondition))
#else // CHIP_CONFIG_VERBOSE_VERIFY_OR_DIE
-#define VerifyOrDie(aCondition) nlABORT(aCondition)
+#define VerifyOrDie(aCondition) VerifyOrDieWithoutLogging(aCondition)
#endif // CHIP_CONFIG_VERBOSE_VERIFY_OR_DIE
/**
diff --git a/src/lib/support/VerificationMacrosNoLogging.h b/src/lib/support/VerificationMacrosNoLogging.h
new file mode 100644
index 0000000..8846390
--- /dev/null
+++ b/src/lib/support/VerificationMacrosNoLogging.h
@@ -0,0 +1,27 @@
+/*
+ * Copyright (c) 2023 Project CHIP Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * @file
+ * This file defines and implements macros for verifying values that do not
+ * involve logging.
+ */
+
+#pragma once
+
+#include <nlassert.h>
+
+#define VerifyOrDieWithoutLogging(aCondition) nlABORT(aCondition)
