feat/cm_backtrace: integrate cm_backtrace for MCU fault.#292
feat/cm_backtrace: integrate cm_backtrace for MCU fault.#292sunzhen222 merged 14 commits intomainfrom
Conversation
…ware version to cm_backtrace.
WalkthroughThis update integrates the CmBacktrace library for Cortex-M fault diagnosis and backtrace support. It adds new source files, headers, and configuration for the library, including language support in English and Chinese. The build system is updated to include these sources. Fault handler logic is refactored: detailed handlers are replaced with simple display functions, and new assembly handlers are introduced for GCC. The linker script now marks code boundaries. The error shutdown flow now includes a short delay after clearing the display. Changes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
core/embed/firmware/delay.c (1)
120-131: Add comment explaining the delay approximationThe function calculates delay based on clock speed assumptions.
void software_delay_ms(uint32_t delay) { + // This is an approximate delay based on CPU cycles uint32_t sysClockHz = HAL_RCC_GetSysClockFreq(); // 4 cycles per loop, 1000 loops per ms uint32_t loops_per_ms = sysClockHz / (4 * 1000);core/embed/cm_backtrace/Languages/README.md (1)
1-2: Title language inconsistencyTitle in Chinese, content in English.
-# CmBacktrace: ARM Cortex-M 系列 MCU 错误追踪库 +# CmBacktrace: ARM Cortex-M Series MCU Error Tracking Librarycore/embed/trezorhal/common.c (1)
180-180: Purpose of delay unclearAdd comment explaining why 3ms delay before display.
display_clear(); + // Short delay to ensure display is fully cleared before drawing software_delay_ms(3); display_image(9, 50, 46, 40, toi_icon_warning + 12,core/embed/cm_backtrace/cm_backtrace.c (2)
137-137: Typo in variable name.
600-601: Typo in function name.core/embed/cm_backtrace/cmb_def.h (1)
271-271: Spelling mistake.- // Indicates a stack overflow error has occured + // Indicates a stack overflow error has occurred
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
core/SConscript.firmware(3 hunks)core/embed/cm_backtrace/Languages/README.md(1 hunks)core/embed/cm_backtrace/Languages/en-US/cmb_en_US.h(1 hunks)core/embed/cm_backtrace/Languages/zh-CN/cmb_zh_CN.h(1 hunks)core/embed/cm_backtrace/Languages/zh-CN/cmb_zh_CN_UTF8.h(1 hunks)core/embed/cm_backtrace/cm_backtrace.c(1 hunks)core/embed/cm_backtrace/cm_backtrace.h(1 hunks)core/embed/cm_backtrace/cmb_cfg.h(1 hunks)core/embed/cm_backtrace/cmb_def.h(1 hunks)core/embed/cm_backtrace/cmb_user_cfg.c(1 hunks)core/embed/cm_backtrace/cmb_user_cfg.h(1 hunks)core/embed/cm_backtrace/fault_handler/gcc/cmb_fault.S(1 hunks)core/embed/firmware/delay.c(1 hunks)core/embed/firmware/main.c(3 hunks)core/embed/firmware/memory_H.ld(2 hunks)core/embed/trezorhal/common.c(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
core/embed/trezorhal/common.c (1)
core/embed/firmware/delay.c (1)
software_delay_ms(120-131)
core/embed/cm_backtrace/cmb_user_cfg.c (1)
core/embed/cm_backtrace/cmb_user_cfg.h (2)
delete_err_info_file(7-7)cmb_user_println(8-8)
core/embed/firmware/main.c (5)
core/embed/cm_backtrace/cm_backtrace.h (1)
cm_backtrace_init(38-38)core/embed/cm_backtrace/cm_backtrace.c (1)
cm_backtrace_init(145-175)core/embed/trezorhal/hardware_version.c (2)
hw_ver_to_str(13-41)get_hw_ver(65-78)core/embed/trezorhal/hardware_version.h (2)
hw_ver_to_str(30-30)get_hw_ver(33-33)core/embed/trezorhal/common.h (1)
error_shutdown(85-86)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Style check
🔇 Additional comments (15)
core/embed/firmware/memory_H.ld (1)
62-62: Added symbols for CmBacktrace libraryThese symbols define the boundaries of executable code in flash memory, essential for accurate backtrace and fault analysis by CmBacktrace.
Also applies to: 75-75
core/embed/cm_backtrace/cmb_user_cfg.h (1)
1-10: Function declarations look good.The header file correctly defines interfaces for error file management and formatted printing, with proper include guards.
core/SConscript.firmware (3)
457-461: CM_BACKTRACE source files integration looks good.The source files for the CmBacktrace library are properly added to the build system.
706-706: Include path correctly added.The CmBacktrace include path is properly integrated into the build configuration.
1094-1095: CM_BACKTRACE object files properly included.The CmBacktrace object files are correctly added to the program object list.
core/embed/cm_backtrace/cmb_cfg.h (4)
1-27: License header is well-formatted.The MIT license header is properly formatted and includes all necessary information.
29-37: Configuration looks appropriate.The configuration correctly sets up user config inclusion and defines the print function macro.
38-48: Platform configuration looks good.The bare metal platform is enabled and CPU is correctly configured for Cortex-M7.
49-63: Fallback configuration is well-defined.The fallback configuration provides reasonable defaults if user config is not enabled.
core/embed/cm_backtrace/Languages/en-US/cmb_en_US.h (3)
1-29: License header is appropriate.The MIT license header is correctly formatted with proper attribution and usage notes.
31-41: Error and information messages are clear.The general error and information messages are well-written and provide useful diagnostic information.
42-72: Fault descriptors are comprehensive.The fault descriptors provide detailed explanations for various fault types and causes, helping with debugging.
core/embed/cm_backtrace/Languages/zh-CN/cmb_zh_CN.h (1)
1-72: LGTM! Chinese (GB18030) language strings look correct.The strings provide good error descriptions for fault conditions. The encoding is properly documented.
core/embed/cm_backtrace/Languages/zh-CN/cmb_zh_CN_UTF8.h (1)
1-72: LGTM! UTF-8 encoded Chinese strings match GB18030 version.Good to have UTF-8 encoding option alongside GB18030 for better compatibility.
core/embed/cm_backtrace/cm_backtrace.h (1)
1-49: LGTM! Well-structured header with clear API.The public interface provides necessary functions for backtrace and fault handling.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
core/embed/cm_backtrace/cmb_def.h (2)
138-138: Typo. "statck" → "stack"
296-296: Spelling. "occured" → "occurred"core/embed/cm_backtrace/Languages/en-US/cmb_en_US.h (2)
86-86: Grammar error. Redundant phrase "is caused by indicates"
93-93: Grammar error. Redundant phrase "is caused by Indicates"core/embed/cm_backtrace/cm_backtrace.c (3)
620-620: Typo. "statck_del_fpu_regs"
621-621: Typo. "statck_has_fpu_regs"
623-623: Typo. "statck_has_fpu_regs"core/embed/firmware/main.c (1)
240-259: New simplified fault handlers.Each handler just shows error code. No context about why they're called from assembly.
Add brief comment stating these are called from assembly handlers after context capture.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (10)
core/embed/cm_backtrace/Languages/en-US/cmb_en_US.h(1 hunks)core/embed/cm_backtrace/Languages/zh-CN/cmb_zh_CN.h(1 hunks)core/embed/cm_backtrace/Languages/zh-CN/cmb_zh_CN_UTF8.h(1 hunks)core/embed/cm_backtrace/cm_backtrace.c(1 hunks)core/embed/cm_backtrace/cm_backtrace.h(1 hunks)core/embed/cm_backtrace/cmb_cfg.h(1 hunks)core/embed/cm_backtrace/cmb_def.h(1 hunks)core/embed/cm_backtrace/cmb_user_cfg.c(1 hunks)core/embed/cm_backtrace/cmb_user_cfg.h(1 hunks)core/embed/firmware/main.c(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- core/embed/cm_backtrace/cmb_user_cfg.c
🚧 Files skipped from review as they are similar to previous changes (5)
- core/embed/cm_backtrace/Languages/zh-CN/cmb_zh_CN_UTF8.h
- core/embed/cm_backtrace/cmb_cfg.h
- core/embed/cm_backtrace/cmb_user_cfg.h
- core/embed/cm_backtrace/Languages/zh-CN/cmb_zh_CN.h
- core/embed/cm_backtrace/cm_backtrace.h
🧰 Additional context used
🧬 Code Graph Analysis (1)
core/embed/cm_backtrace/cm_backtrace.c (4)
core/embed/cm_backtrace/cm_backtrace.h (5)
cm_backtrace_init(38-39)cm_backtrace_firmware_info(40-40)cm_backtrace_call_stack(41-41)cm_backtrace_assert(42-42)cm_backtrace_fault(43-43)core/embed/cm_backtrace/cmb_def.h (18)
cmb_get_sp(400-400)cmb_get_sp(400-400)cmb_get_sp(412-412)cmb_get_sp(429-432)cmb_get_sp(429-429)cmb_get_sp(445-445)cmb_get_psp(399-399)cmb_get_psp(399-399)cmb_get_psp(407-407)cmb_get_psp(425-428)cmb_get_psp(425-425)cmb_get_psp(440-440)cmb_get_msp(398-398)cmb_get_msp(398-398)cmb_get_msp(402-402)cmb_get_msp(421-424)cmb_get_msp(421-421)cmb_get_msp(435-435)core/embed/cm_backtrace/cmb_user_cfg.h (1)
delete_err_info_file(7-7)core/embed/cm_backtrace/cmb_user_cfg.c (1)
delete_err_info_file(9-9)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Style check
- GitHub Check: Defs check
- GitHub Check: Gen check
🔇 Additional comments (2)
core/embed/firmware/main.c (2)
76-77: Added includes for cm_backtrace.
142-142: Initialized cm_backtrace after memory setup.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
core/embed/cm_backtrace/cmb_user_cfg.c (1)
32-33:⚠️ Potential issueWrite result not verified.
No error checking after file write.
🧹 Nitpick comments (1)
core/embed/cm_backtrace/cmb_user_cfg.c (1)
13-14: Buffer size hard-coded.Fixed 512-byte buffer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
core/embed/cm_backtrace/cmb_user_cfg.c(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
core/embed/cm_backtrace/cmb_user_cfg.c (1)
core/embed/cm_backtrace/cmb_user_cfg.h (2)
delete_err_info_file(7-7)cmb_user_println(8-8)
🪛 Cppcheck (2.10-2)
core/embed/cm_backtrace/cmb_user_cfg.c
[error] 30-30: va_list 'args' was opened but not closed by va_end().
(va_end_missing)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Style check
- GitHub Check: Gen check
- GitHub Check: Defs check
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
core/embed/cm_backtrace/cm_backtrace.c(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
core/embed/cm_backtrace/cm_backtrace.c (4)
core/embed/cm_backtrace/cm_backtrace.h (5)
cm_backtrace_init(38-39)cm_backtrace_firmware_info(40-40)cm_backtrace_call_stack(41-41)cm_backtrace_assert(42-42)cm_backtrace_fault(43-43)core/embed/cm_backtrace/cmb_def.h (18)
cmb_get_sp(400-400)cmb_get_sp(400-400)cmb_get_sp(412-412)cmb_get_sp(429-432)cmb_get_sp(429-429)cmb_get_sp(445-445)cmb_get_psp(399-399)cmb_get_psp(399-399)cmb_get_psp(407-407)cmb_get_psp(425-428)cmb_get_psp(425-425)cmb_get_psp(440-440)cmb_get_msp(398-398)cmb_get_msp(398-398)cmb_get_msp(402-402)cmb_get_msp(421-424)cmb_get_msp(421-421)cmb_get_msp(435-435)core/embed/cm_backtrace/cmb_user_cfg.c (1)
delete_err_info_file(9-9)core/embed/cm_backtrace/cmb_user_cfg.h (1)
delete_err_info_file(7-7)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Defs check
- GitHub Check: Style check
- GitHub Check: Gen check
3 participants
Summary by CodeRabbit