Memfault Firmware SDK 0.41.2 (Build 1696)

Memfault Inc · Memfault Inc · commit a04d9302ae49 · 2023-03-13T14:34:31.000Z
diff --git a/CHANGES.md b/CHANGES.md
@@ -1,8 +1,18 @@
+### Changes between Memfault SDK 0.41.2 and SDK 0.41.1 - Mar 10, 2023
+
+#### :chart_with_upwards_trend: Improvements
+
+- Zephyr / nRF-Connect SDK:
+  - Improve compatibility with Zephyr pre-3.0 deferred logging, when using the
+    Memfault backend
+  - Add an option to the [examples/nrf-connect/nrf5](examples/nrf-connect/nrf5)
+    to enable capturing all of RAM in a coredump.
+
 ### Changes between Memfault SDK 0.41.1 and SDK 0.41.0 - Mar 1, 2023
 
 #### :chart_with_upwards_trend: Improvements
 
-- Zephyr / nRF-Connect SDk:
+- Zephyr / nRF-Connect SDK:
 
   - Fix a bug 🐛 in the Memfault Logging backend, that causes coredump saving to
     fail when building with Zephyr versions before 3.0 (nRF-Connect SDK < 2.0).
diff --git a/VERSION b/VERSION
@@ -1,2 +1,2 @@
-BUILD ID: 1633
-GIT COMMIT: 12ef0db7f
+BUILD ID: 1696
+GIT COMMIT: e44b190c9
diff --git a/components/include/memfault/version.h b/components/include/memfault/version.h
@@ -19,7 +19,7 @@ typedef struct {
   uint8_t patch;
 } sMfltSdkVersion;
 
-#define MEMFAULT_SDK_VERSION   { .major = 0, .minor = 41, .patch = 1 }
+#define MEMFAULT_SDK_VERSION   { .major = 0, .minor = 41, .patch = 2 }
 
 #ifdef __cplusplus
 }
diff --git a/examples/nrf-connect-sdk/nrf5/memfault_demo_app/1mbaud_uart.overlay b/examples/nrf-connect-sdk/nrf5/memfault_demo_app/1mbaud_uart.overlay
@@ -0,0 +1,4 @@
+/* gotta go fast 🐎 */
+&uart0 {
+	current-speed = <1000000>;
+};
diff --git a/examples/nrf-connect-sdk/nrf5/memfault_demo_app/Kconfig b/examples/nrf-connect-sdk/nrf5/memfault_demo_app/Kconfig
@@ -0,0 +1,31 @@
+
+menu "MEMFAULT_APP"
+
+config MEMFAULT_APP_CAPTURE_ALL_RAM
+    bool "Capture all RAM"
+    help
+      Capture all RAM regions. This is useful for debugging purposes, but
+      will increase the amount of data captured by the Memfault SDK.
+
+# If MEMFAULT_APP_CAPTURE_ALL_RAM is enabled, increase the configured space
+# for the Memfault storage partition to accommodate the larger coredump.
+#
+# This overrides the default setting, which can be found here:
+# https://github.com/nrfconnect/sdk-nrf/blob/96f97684565fbf711f1ceb7da061fed0bfe7b60b/modules/memfault-firmware-sdk/Kconfig#L170-L172
+#
+# Expanded into the template here:
+# https://github.com/nrfconnect/sdk-nrf/blob/96f97684565fbf711f1ceb7da061fed0bfe7b60b/subsys/partition_manager/Kconfig.template.partition_size
+#
+# Keep 'source "Kconfig.zephyr"' at the end of this file so this default takes
+# precendence.
+config PM_PARTITION_SIZE_MEMFAULT_STORAGE
+    hex "Size of the Memfault storage partition"
+    # The default size is set to one page above the total RAM size on the
+    # NRF52840 SOC, which provides room for the coredump header and NVIC and CPU
+    # registers. This size needs to be flash page-aligned ("minimum eraseable
+    # unit").
+    default 0x41000 if MEMFAULT_APP_CAPTURE_ALL_RAM
+
+endmenu
+
+source "Kconfig.zephyr"
diff --git a/examples/nrf-connect-sdk/nrf5/memfault_demo_app/prj.conf b/examples/nrf-connect-sdk/nrf5/memfault_demo_app/prj.conf
@@ -12,7 +12,6 @@ CONFIG_MEMFAULT=y
 
 # Use internal flash to store the coredump
 CONFIG_MEMFAULT_NCS_INTERNAL_FLASH_BACKED_COREDUMP=y
-CONFIG_PM_PARTITION_SIZE_MEMFAULT_STORAGE=0x20000
 
 # set a dummy project key; this setting is required by the nrf-connect
 # integration, but it is unused in a non-HTTP application
diff --git a/examples/nrf-connect-sdk/nrf5/memfault_demo_app/src/main.c b/examples/nrf-connect-sdk/nrf5/memfault_demo_app/src/main.c
@@ -28,6 +28,7 @@ char *prv_conditional_log_strdup(char *str) {
 
   return str;
 }
+
 static void prv_set_device_id(void) {
   uint8_t dev_id[16] = {0};
   char dev_id_str[sizeof(dev_id) * 2 + 1];
@@ -53,6 +54,18 @@ static void prv_set_device_id(void) {
   memfault_ncs_device_id_set(dev_str, length * 2);
 }
 
+#if CONFIG_MEMFAULT_APP_CAPTURE_ALL_RAM
+// capture *ALL* of ram
+const sMfltCoredumpRegion *memfault_platform_coredump_get_regions(
+  const sCoredumpCrashInfo *crash_info, size_t *num_regions) {
+  static sMfltCoredumpRegion s_regions[] = {
+    MEMFAULT_COREDUMP_MEMORY_REGION_INIT(CONFIG_PM_SRAM_BASE, CONFIG_PM_SRAM_SIZE),
+  };
+  *num_regions = ARRAY_SIZE(s_regions);
+  return s_regions;
+}
+#endif  // CONFIG_MEMFAULT_APP_CAPTURE_ALL_RAM
+
 #define WD_FEED_THREAD_STACK_SIZE 500
 // set priority to lowest application thread; shell_uart, where the 'mflt test
 // hang' command runs from, uses the same priority by default, so this should
diff --git a/ports/freertos/src/memfault_freertos_ram_regions.c b/ports/freertos/src/memfault_freertos_ram_regions.c
@@ -16,7 +16,7 @@
 //!        _sbss = . ;
 //!        __bss_start__ = _sbss;
 //!        __memfault_capture_bss_start = .;
-//!         *tasks.o(.bss COMMON .bss*)
+//!         *tasks*.o(.bss COMMON .bss*)
 //!         *timers*.o(.bss COMMON .bss*)
 //!        __memfault_capture_bss_end = .;
 //!
diff --git a/ports/zephyr/common/memfault_logging.c b/ports/zephyr/common/memfault_logging.c
@@ -80,6 +80,15 @@ typedef struct MfltLogProcessCtx {
 
 static void prv_log_process(const struct log_backend *const backend,
                             union log_msg_generic *msg) {
+  // This can be called in IMMEDIATE mode from an ISR, so in that case,
+  // immediately bail. We currently can't safely serialize to the Memfault
+  // buffer from ISR context
+#if CONFIG_LOG_MODE_IMMEDIATE
+  if (memfault_arch_is_inside_isr()) {
+    return;
+  }
+#endif
+
   // Copied flagging from Zephry's ring buffer (rb) implementation.
   const uint32_t flags = IS_ENABLED(CONFIG_LOG_BACKEND_FORMAT_TIMESTAMP)
                        ? LOG_OUTPUT_FLAG_FORMAT_TIMESTAMP | LOG_OUTPUT_FLAG_LEVEL
diff --git a/scripts/mflt-build-id/README.md b/scripts/mflt-build-id/README.md
@@ -0,0 +1,47 @@
+# Memfault Build ID Tool
+
+This package contains the `mflt_build_id` CLI tool.
+
+The purpose of the tool is simplify reading or writing
+[Build IDs](https://interrupt.memfault.com/blog/gnu-build-id-for-firmware) in a
+firmware image irrespective of the compiler or build system being used in a
+project.
+
+## Example Usage
+
+```
+$ mflt_build_id --help
+usage: mflt_build_id [-h] [--dump [DUMP]] [--crc CRC] [--sha1 SHA1] elf
+
+Inspects provided ELF for a Build ID and when missing adds one if possible.
+
+If a pre-existing Build ID is found (either a GNU Build ID or a Memfault Build ID),
+no action is taken.
+
+If no Build ID is found, this script will generate a unique ID by computing a SHA1 over the
+contents that will be in the final binary. Once computed, the build ID will be "patched" into a
+read-only struct defined in memfault-firmware-sdk/components/core/src/memfault_build_id.c to hold
+the info.
+
+If the --crc <symbol_holding_crc32> argument is used, instead of populating the Memfault Build ID
+structure, the symbol specified will be updated with a CRC32 computed over the contents that will
+be in the final binary.
+
+If the --sha1 <symbol_holding_sha> argument is used, instead of populating the Memfault Build ID
+structure, the symbol specified will be updated directly with Memfault SHA1 using the same strategy
+discussed above. The only expectation in this mode is that a global symbol has been defined as follow:
+
+const uint8_t g_your_symbol_build_id[20] = { 0x1, };
+
+For further reading about Build Ids in general see:
+  https://mflt.io//symbol-file-build-ids
+
+positional arguments:
+  elf
+
+options:
+  -h, --help     show this help message and exit
+  --dump [DUMP]
+  --crc CRC
+  --sha1 SHA1
+```
diff --git a/scripts/mflt-build-id/pyproject.toml b/scripts/mflt-build-id/pyproject.toml
@@ -1,12 +1,13 @@
 [tool.poetry]
 name = "mflt-build-id"
-version = "0.0.5"
+version = "1.0.0"
 description = "Memfault Build Id injector"
 authors = ["Memfault Inc <hello@memfault.com>"]
 repository = "https://github.com/memfault/memfault-firmware-sdk"
 packages = [
   { include = "mflt_build_id", from = "src" }
 ]
+readme = "README.md"
 
 [tool.poetry.scripts]
 mflt_build_id = 'mflt_build_id:main'
diff --git a/scripts/mflt-build-id/src/mflt_build_id/__init__.py b/scripts/mflt-build-id/src/mflt_build_id/__init__.py
@@ -4,18 +4,29 @@
 #
 
 # This file needs to be Python 2.7 compatible (i.e. type annotations must remain in comments).
+# Note: The snippet should be kept in sync with the "Example Usage" section of the README
 """Inspects provided ELF for a Build ID and when missing adds one if possible.
 
 If a pre-existing Build ID is found (either a GNU Build ID or a Memfault Build ID),
 no action is taken.
 
 If no Build ID is found, this script will generate a unique ID by computing a SHA1 over the
-contents that will be in the final binary. Once computed, the build ID will be "patched"
-into a read-only struct defined in memfault/core/memfault_build_id.c to hold the info.
+contents that will be in the final binary. Once computed, the build ID will be "patched" into a
+read-only struct defined in memfault-firmware-sdk/components/core/src/memfault_build_id.c to hold
+the info.
 
-If the --crc <symbol_holding_crc32> is used, instead of populating the Memfault Build ID structure,
-the symbol specified will be updated with a CRC32 computed over the contents that will be in the
-final binary.
+If the --crc <symbol_holding_crc32> argument is used, instead of populating the Memfault Build ID
+structure, the symbol specified will be updated with a CRC32 computed over the contents that will
+be in the final binary.
+
+If the --sha1 <symbol_holding_sha> argument is used, instead of populating the Memfault Build ID
+structure, the symbol specified will be updated directly with Memfault SHA1 using the same strategy
+discussed above. The only expectation in this mode is that a global symbol has been defined as follow:
+
+const uint8_t g_your_symbol_build_id[20] = { 0x1, };
+
+For further reading about Build Ids in general see:
+  https://mflt.io//symbol-file-build-ids
 """
 
 import argparse
@@ -58,6 +69,9 @@ def hexlify(data):
         return data.hex()
 
 
+SHA1_BUILD_ID_SIZE_BYTES = 20
+
+
 class SectionType(Enum):
     TEXT = 1
     DATA = 2
@@ -231,8 +245,8 @@ def __init__(self, elf_file, elf=None):
         self.elf = elf or ELFFile(elf_file)
         self._helper = ELFFileHelper(self.elf)
 
-    def _generate_build_id(self):
-        # type: () -> hashlib._Hash
+    def _generate_build_id(self, sha1_symbol_section=None, sha1_symbol_section_offset=0):
+        # type: (Optional[Section], int) -> hashlib._Hash
         build_id = hashlib.sha1()
         for section in self.elf.iter_sections():
             if not self._helper.section_in_binary(section):
@@ -247,7 +261,17 @@ def _generate_build_id(self):
                 length = section["sh_size"]
                 build_id.update(struct.pack("<I", length))
             else:
-                build_id.update(section.data())
+                data = section.data()
+                if sha1_symbol_section and sha1_symbol_section["sh_addr"] == section["sh_addr"]:
+                    build_id.update(data[:sha1_symbol_section_offset])
+                    # NB: When a Memfault Build ID is generated as part of a memfault-firmware-sdk
+                    # integration we SHA1 over the compile time state of
+                    # "g_memfault_sdk_derived_build_id"... simulate the same behavior when updating
+                    # a SHA1 symbol directly.
+                    build_id.update(bytearray([1] + [0] * 19))
+                    build_id.update(data[sha1_symbol_section_offset + SHA1_BUILD_ID_SIZE_BYTES :])
+                else:
+                    build_id.update(data)
 
         return build_id
 
@@ -266,6 +290,50 @@ def _get_note_sections(elf):
 
         return None
 
+    def check_or_update_sha1_build_id(self, sha1_sym_name, dump_only):
+        symbol, section = self._helper.find_symbol_and_section(sha1_sym_name)
+        if symbol is None or section is None:
+            raise BuildIdError("Could not locate '{}' symbol in provided ELF".format(sha1_sym_name))
+
+        if symbol["st_size"] != SHA1_BUILD_ID_SIZE_BYTES:
+            raise BuildIdError(
+                "A build ID should be {} bytes in size".format(SHA1_BUILD_ID_SIZE_BYTES)
+            )
+
+        current_sha1_bytes = bytearray(self._helper.get_symbol_data(symbol, section))
+
+        symbol_offset = self._helper.get_symbol_offset_in_sector(symbol, section)
+
+        sha1_hash = self._generate_build_id(
+            sha1_symbol_section=section, sha1_symbol_section_offset=symbol_offset
+        )
+        sha1_hash_bytes = sha1_hash.digest()
+
+        if current_sha1_bytes == sha1_hash_bytes:
+            print(
+                "Memfault Generated SHA1 Build ID at '{}': {}".format(
+                    sha1_sym_name, sha1_hash.hexdigest()
+                )
+            )
+            return sha1_hash
+
+        if dump_only:
+            print("Memfault Build ID at '{}' is not written".format(sha1_sym_name))
+            return sha1_hash
+
+        with open(self.elf_file.name, "r+b") as fh:
+            derived_id_patch_offset = section[
+                "sh_offset"
+            ] + self._helper.get_symbol_offset_in_sector(symbol, section)
+
+            fh.seek(derived_id_patch_offset)
+            fh.write(sha1_hash_bytes)
+
+        print("Added Memfault Generated SHA1 Build ID to ELF: {}".format(sha1_hash.hexdigest()))
+
+        # Return the sha1 computed in case someone wants to run this as a library
+        return sha1_hash
+
     def _write_and_return_build_info(self, dump_only):
         # type: (bool) -> Tuple[MemfaultBuildIdTypes, Optional[str], Optional[int]]
         sdk_build_id_sym_name = "g_memfault_build_id"
@@ -330,6 +398,7 @@ def _write_and_return_build_info(self, dump_only):
 
             fh.seek(derived_id_patch_offset)
             fh.write(build_id_hash.digest())
+
         build_id = build_id_hash.hexdigest()
         print("Added Memfault Generated Build ID to ELF: {}".format(build_id))
         return MemfaultBuildIdTypes.MEMFAULT_BUILD_ID_SHA1, build_id, short_len
@@ -452,13 +521,20 @@ def main():
     parser.add_argument("elf", action="store")
     parser.add_argument("--dump", nargs="?", const=7, type=int)
     parser.add_argument("--crc", action="store")
+    parser.add_argument("--sha1", action="store")
 
     args = parser.parse_args()
 
+    if args.sha1 and args.crc:
+        raise Exception("Only one of 'crc' or 'sha1' can be specified")
+
     with open(args.elf, "rb") as elf_file:
         b = BuildIdInspectorAndPatcher(elf_file=elf_file)
-        if args.crc:
-            b.check_or_update_crc_build_id(args.crc, dump_only=args.dump)
+
+        if args.sha1:
+            b.check_or_update_sha1_build_id(args.sha1, dump_only=args.dump)
+        elif args.crc:
+            b.check_or_update_crc_build_id(args.sha1, dump_only=args.dump)
         elif args.dump is None:
             if args.crc is None:
                 b.check_or_update_build_id()
diff --git a/scripts/mflt-build-id/tests_mflt_build_id/test_fw_build_id.py b/scripts/mflt-build-id/tests_mflt_build_id/test_fw_build_id.py

Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,7 @@ typedef struct {`
`19`	`19`	`uint8_t patch;`
`20`	`20`	`} sMfltSdkVersion;`
`21`	`21`
`22`		`-#define MEMFAULT_SDK_VERSION { .major = 0, .minor = 41, .patch = 1 }`
	`22`	`+#define MEMFAULT_SDK_VERSION { .major = 0, .minor = 41, .patch = 2 }`
`23`	`23`
`24`	`24`	`#ifdef __cplusplus`
`25`	`25`	`}`