Implement custom backtrace based on linunwind and libbacktrace [ci patch inc]

[HyperWinX]

No description provided.

[mentatbot]

I'm getting to work. See my steps and track my cost here.

• Wake on any new activity.
  _{I always see new activity while actively working. When paused, I'll only respond when tagged unless this box is checked.}

[mentatbot]

PR Review: Implement custom backtrace based on libunwind and libbacktrace

This PR adds a custom backtrace provider that provides more detailed crash information when segmentation faults or floating-point exceptions occur. The implementation uses libunwind and libbacktrace libraries to capture stack frames with file names, line numbers, and register values.

Architecture

The implementation adds:

• Two new Git submodules for dependencies: libbacktrace and libunwind
• A BacktraceProvider component with controller class and signal handlers
• Conditional compilation based on libunwind availability
• Integration in both main executables (assembler and emulator)

Overall, this is a valuable addition to improve debugging capabilities, especially for crash reports from users.

Observations and Suggestions

1. Signal Handler Safety:

   • Signal handlers have strict limitations on what functions they can safely call
   • The current implementation does a lot of complex operations in a signal handler context
   • Consider making the signal handler only set a flag and defer complex processing to a safer context when possible

2. Thread Safety:

   • The global controller and signal type variables could cause issues in multi-threaded contexts
   • Consider adding mutex protection or documenting thread safety constraints

3. Minor Issues:

   • Typo: "catched_signal_type" should be "caught_signal_type"
   • Consider handling additional signals like SIGABRT, SIGILL, and SIGBUS
   • Error handling in some callbacks calls std::exit(1) directly, which may bypass normal cleanup

4. Build System:

   • The CMake integration for libbacktrace looks good with ExternalProject
   • Consider adding a fallback or graceful degradation when libraries aren't available

Despite these points, the PR appears well-implemented and is a valuable addition to the project. The formatting is consistent with the rest of the codebase, and the integration looks clean.

[mentatbot]

Detailed Feedback

Thank you for implementing this backtrace functionality! It's a valuable addition that will help with debugging crashes. Here are some specific suggestions for improvement:

1. Signal Handler Safety

Signal handlers operate in a very restricted context where many standard library functions aren't safe to call. The current implementation in SignalHandler() may run into issues because:

• Complex operations like string handling and formatted output aren't guaranteed to be signal-safe
• Memory allocation/deallocation in a signal handler can cause deadlocks or corruption

Suggestion: Consider using a two-phase approach where the signal handler only sets flags and records minimal information, then exits, allowing normal program flow to handle the more complex backtrace generation.

2. Terminology and Naming

• catched_signal_type should be caught_signal_type (grammar correction)

3. Error Handling Consistency

• Some error paths use std::exit(1) while others use std::abort()
• Consider being consistent or documenting why different approaches are used in different contexts

4. Additional Signal Coverage

Consider adding handlers for additional critical signals:

• SIGILL (illegal instruction)
• SIGBUS (bus error)
• SIGABRT (abort)

5. Thread Safety

The global state (global_bt_controller and signal type) could cause issues in multithreaded contexts. Consider:

• Documenting thread safety limitations
• Adding appropriate synchronization if needed
• Making the controller thread-local where appropriate

6. Style Consistency

For consistency with other error messages, consider using the color formatting (with B_RED and RESET) in all error message functions.

Overall, this is a great feature that will improve the debugging experience significantly!