GDB MI Adapter for OpenOCD Embedded Debugging in Binary Ninja #875
Conversation
GDB MI now implements most basic functionality Notify UI that we're connected and ready to serve data small fixes to handle quit cleanup feat(gdb): Refactor event handling and command execution - An event loop (`EventLoop`) running in a dedicated thread (`m_eventThread`) now handles all asynchronous output from GDB. This replaces the previous callback-based approach (`AsyncRecordHandler`). - A mutex (`m_gdbCommandMutex`) is now used exclusively within the `SendCommand` method, centralizing locking and preventing race conditions. Higher-level functions no longer manage the lock themselves. - The target's running state is now managed by a `std::atomic<bool>` (`m_targetRunningAtomic`) for safe access across threads. - Asynchronous commands (`-exec-run`, `-exec-step`, etc.) are now sent via a new `SendCommandAsync` method, which does not wait for a result, allowing the UI to remain responsive while the target is running. - Logging has been adjusted, changing many `LogInfo` calls to `LogDebug` to reduce default output verbosity. - The GDB-MI value parser (`MiValue::parse`) has been rewritten to be more robust and correctly handle complex GDB output formats. Add watch widget implementation for basic types MIParser fixes and cleanup Added unit test for MI parser Implemented StepOut function
|
|
- fix missing default init values
Clear cache on stop/quit/restart
|
Thanks for the contribution! I had a quick glance and it looks good, I will review it when I get time. Also I had copilot look at it as well -- I have not gone through them yet, if they are correct, please address them; if they are wrong, please point it out! |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a comprehensive GDB MI (Machine Interface) adapter for Binary Ninja, specifically optimized for embedded ARM debugging via OpenOCD. The adapter implements the full DebugAdapter interface with thread-safe operations, intelligent architecture detection, and MI protocol parsing.
Key changes:
- GDB MI protocol parser and connector with async event handling
- Complete debug adapter implementation supporting breakpoints, stepping, memory access, and register inspection
- Comprehensive unit tests for MI protocol parsing edge cases
- Architecture auto-detection for ARM Cortex-M targets
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| core/adapters/gdbmiconnector.h | Header defining MI protocol structures and connector interface |
| core/adapters/gdbmiconnector.cpp | MI protocol parser and GDB process management implementation |
| core/adapters/gdbmiadapter.h | Debug adapter interface declarations with caching system |
| core/adapters/gdbmiadapter.cpp | Full debug adapter implementation with OpenOCD-specific functionality |
| core/tests/miparser_test.cpp | Unit tests validating MI protocol parsing for complex nested structures |
| core/tests/CMakeLists.txt | Test build configuration with GoogleTest integration |
| core/debugger.cpp | Registration of new adapter type |
| core/CMakeLists.txt | Build system integration for new adapter files |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
core/adapters/gdbmiconnector.cpp
Outdated
| for (;;) | ||
| { | ||
| ssize_t n = read(m_gdb_stdout_read, buffer, sizeof(buffer)); | ||
| buffer[n] = 0; |
There was a problem hiding this comment.
Potential buffer overflow: the buffer is declared with size 4096, but writing at index n without bounds checking. If n equals 4096, this writes beyond the buffer. Should check n < sizeof(buffer) before this write.
| buffer[n] = 0; | |
| if (n >= 0 && n < (ssize_t)sizeof(buffer)) | |
| buffer[n] = 0; | |
| // If n == sizeof(buffer), buffer is not null-terminated |
Copilot uses AI. Check for mistakes.
core/adapters/gdbmiadapter.cpp
Outdated
| if (address > 0x60000000) return {}; | ||
| if (address < 0x08000000) return {}; |
There was a problem hiding this comment.
Magic numbers for memory address ranges should be defined as named constants. Consider defining ARM_CORTEX_MEMORY_MIN and ARM_CORTEX_MEMORY_MAX to make the code more maintainable and document the reasoning.
Copilot uses AI. Check for mistakes.
|
Remove all breakpoints at address Use BackendMessageEventType as output for gdb messages
3 participants
Key Features:
Technical Implementation Details
Architecture:
miparser_testKey Technical Components:
Setup and Configuration
Required Tools:
gdb-multiarchorarm-none-eabi-gdbTypical Workflow:
openocd -f interface/stlink.cfg -f target/stm32f4x.cfgAdapter Settings:
gdb.path: Path to GDB executable (default:gdb-multiarch)connect.ipAddress: OpenOCD server IP (default:127.0.0.1)connect.port: OpenOCD GDB port (default:3333)common.inputFile: Firmware file for base address resolutiongdb.symbolFile: ELF or DWARF debug information so gdb would know funcs boundaries and names for backtraceTesting and Validation
Verified Platforms:
Validation Checklist:
Benefits for Binary Ninja Community
This implementation significantly enhances Binary Ninja's capabilities for: