7
31
Fork
You've already forked blackmagic
23

Fix NULL jtagtap_cycle() which crashed BMDA #2179

Merged
ALTracer merged 3 commits from fix/jtagtap_cycle into main 2026年01月29日 14:42:38 +01:00
ALTracer commented 2026年01月28日 18:15:18 +01:00 (Migrated from github.com)
Copy link

Detailed description

  • No real new features.
  • The existing problem is SIGSEGV in BMDA when trying to invoke jtag_proc.jtagtap_cycle() in initial JTAG scan.
  • This PR solves it by providing simplified wrappers for that (around jtagtap_next()).

Affected backends are FTDI MPSSE, J-Link, and CMSIS-DAP.
Unaffected backends are BMP HL remote, STLINK. BMF was not affected either.
Tested with FT2232HL against a RV32I target, the other 3 callsites are not fixed.

Your checklist for this pull request

Closing issues

## Detailed description * No real new features. * The existing problem is SIGSEGV in BMDA when trying to invoke `jtag_proc.jtagtap_cycle()` in initial JTAG scan. * This PR solves it by providing simplified wrappers for that (around `jtagtap_next()`). Affected backends are FTDI MPSSE, J-Link, and CMSIS-DAP. Unaffected backends are BMP HL remote, STLINK. BMF was not affected either. Tested with FT2232HL against a RV32I target, the other 3 callsites are not fixed. ## Your checklist for this pull request * [x] I've read the [Code of Conduct](https://github.com/blackmagic-debug/blackmagic/blob/main/CODE_OF_CONDUCT.md) * [x] I've read the [guidelines for contributing](https://github.com/blackmagic-debug/blackmagic/blob/main/CONTRIBUTING.md) to this repository * [x] It builds for hardware native (see [Building the firmware](https://github.com/blackmagic-debug/blackmagic?tab=readme-ov-file#building-black-magic-debug-firmware)) * [x] It builds as BMDA (see [Building the BMDA](https://github.com/blackmagic-debug/blackmagic?tab=readme-ov-file#building-black-magic-debug-app)) * [x] I've tested it to the best of my ability * [x] My commit messages provide a useful short description of what the commits do ## Closing issues

Rather than assigning this to NULL explicitly, we would far prefer a dap_jtag_cycle be implemented to polyfill for now till a proper implementation for CMSIS-DAP can be created (and same for J-Link). We want this to never be NULL rather than having to check and manually polyfill in the call sites.

Rather than assigning this to `NULL` explicitly, we would far prefer a `dap_jtag_cycle` be implemented to polyfill for now till a proper implementation for CMSIS-DAP can be created (and same for J-Link). We want this to never be `NULL` rather than having to check and manually polyfill in the call sites.
ALTracer (Migrated from github.com) reviewed 2026年01月28日 20:30:28 +01:00
ALTracer (Migrated from github.com) commented 2026年01月28日 20:30:28 +01:00
Copy link

Added some untested implementations to both.

Added some untested implementations to both.
ALTracer commented 2026年01月28日 20:33:21 +01:00 (Migrated from github.com)
Copy link

Now that all three affected backends have got some implementations of that, commits 1-2 can be discarded (explicit NULL and a nullcheck at callsite).

Now that all three affected backends have got some implementations of that, commits 1-2 can be discarded (explicit NULL and a nullcheck at callsite).
dragonmux left a comment
Copy link

This is looking better - we've got one review note that applies in a couple of places which is just about layout, otherwise LGTM 👍🏼

This is looking better - we've got one review note that applies in a couple of places which is just about layout, otherwise LGTM 👍🏼
@ -60,11 +61,11 @@ bool dap_jtag_init(void)
jtag_proc.jtagtap_tms_seq = dap_jtag_tms_seq;

Would suggest not adding an extra newline here for consistency with the bitbanged impl (the SWD-to-JTAG sequence is very much part and parcel of the line 66 comment aboout ensuring we're in JTAG mode)

Would suggest not adding an extra newline here for consistency with the bitbanged impl (the SWD-to-JTAG sequence is very much part and parcel of the line 66 comment aboout ensuring we're in JTAG mode)
ALTracer (Migrated from github.com) reviewed 2026年01月29日 05:14:55 +01:00
@ -60,11 +61,11 @@ bool dap_jtag_init(void)
jtag_proc.jtagtap_tms_seq = dap_jtag_tms_seq;
ALTracer (Migrated from github.com) commented 2026年01月29日 05:14:55 +01:00
Copy link

Removed extra newline, which I inserted because pre-commit clang-format wants to align the inline comments.

Removed extra newline, which I inserted because pre-commit clang-format wants to align the inline comments.
dragonmux left a comment
Copy link

LGTM, merging. Thank you for the contribution! We'll backport this just like we did the JTAG scan fix as this fixes a v2.0 bug.

LGTM, merging. Thank you for the contribution! We'll backport this just like we did the JTAG scan fix as this fixes a v2.0 bug.
Sign in to join this conversation.
No reviewers
Labels
Clear labels
BMD App
Black Magic Debug App (aka. PC hosted) (not firmware)
BMP Firmware
Black Magic Probe Firmware (not PC hosted software)
Bug
Confirmed bug
Build system
Build system
Can't reproduce
Maintainers can't reproduce this problem
CI
Continuous Integration System
Contribution wanted
User contributions welcome
Documentation
Project documentation
Draft
Work in progress draft
Duplicate
This issue or pull request already exists
Enhancement
General project improvement
Feedback wanted
Requires additional submitter feedback
Foreign Host Board
Non Native hardware to runing Black Magic firmware on
GDB
Issue/PR related to GDB
Good first issue
Good for newcommers
HwIssue Mitigation
Solving or mitigating a Hardware issue in Software
Information Needed
Maintainers need more information
NativeHardware
Official Black Magic Debug Hardware
New Host Board
New hardware to run Black Magic firmware on
New Target
New debug target
Off Topic
Something that does not involve the project in any way
Potential Bug
A potential, unconfirmed or very special circumstance bug
Regression
Bug caused by a regression
User Interest Needed
More user interest required before consideration
User Testing Needed
Looking for user testing reports
Won't fix
Outside of the project scope or works as intended
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
blackmagic-debug/blackmagic!2179
Reference in a new issue
blackmagic-debug/blackmagic
No description provided.
Delete branch "fix/jtagtap_cycle"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?