Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Semihosting #1432

Merged
merged 1 commit into from
Apr 4, 2023
Merged

Fix: Semihosting #1432

merged 1 commit into from
Apr 4, 2023

Conversation

dragonmux
Copy link
Member

Detailed description

In the wake of #1284, Koen correctly identified that a private (UB) declaration of gdb_main_loop() had the semihosting Host IO code broken with nobody having noticed. This PR addresses this by removing that declaration, putting one in the gdb_main.h header so mismatches with gdb_main.c result in compilation failure, and updating how the Host IO code calls the main loop.

Your checklist for this pull request

  • I've read the Code of Conduct
  • I've read the guidelines for contributing to this repository
  • It builds for hardware native (make PROBE_HOST=native)
  • It builds as BMDA (make PROBE_HOST=hosted)
  • I've tested it to the best of my ability
  • My commit messages provide a useful short description of what the commits do

Closing issues

Fixes #1385

…o a hidden UB declaration of gdb_main_loop()
@dragonmux dragonmux added Bug Confirmed bug GDB Issue/PR related to GDB Regression Bug caused by a regression labels Apr 3, 2023
@dragonmux dragonmux added this to the v1.10 milestone Apr 3, 2023
@dragonmux dragonmux requested a review from esden April 3, 2023 13:48
@dragonmux
Copy link
Member Author

@koendv Please give this a test when you find a minute. It should have solved the bug you reported back in February. We are unfortunately ill-equipped with suitable firmware, etc, to actually test this ourself so please let us know how you get on and if this PR needs to go further.

Copy link
Member

@esden esden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@esden esden added this pull request to the merge queue Apr 4, 2023
Merged via the queue into main with commit 906f887 Apr 4, 2023
@dragonmux dragonmux deleted the fix/semihosting branch April 4, 2023 07:51
@koendv
Copy link
Contributor

koendv commented Apr 5, 2023

ok.
As test programs I use the examples of this Arduino library: https://github.com/koendv/STM32duino-Semihosting
Running the test program "SemihostingTest.ino.elf" in a stm32f103 with openocd, a commercial probe, blackmagic hosted and blackmagic firmware ought to give mostly similar results. (I say similar because not all debuggers implement all semihosting calls. E.g. blackmagic firmware implements semi-hosting using the gdb fileio calls, while blackmagic hosted implements semi-hosting using posix system calls.)
Sample output of "SemihostingTest.ino.elf" is the attachment "semihosting_testbench.txt" in #665
Output of current firmware is semihosting-output.txt
I add a zip with the compiled binary of "SemihostingTest". SemihostingTest.zip
Tests were run with arm-none-eabi-gdb 12.1.90.

My impression is it doesn't quite work.
When using bmp firmware, sometimes speed is an order of magnitude slower than I expect; as if something in bmp is looping. Return values are odd; e.g. the flen() semihosting call for determing file length returns -1, independent of file length.
Edit: Using latest git, semihosting works on bmp hosted.
This feels as if there might be more than one bug.

I'd like a second opinion on this one.

@koendv
Copy link
Contributor

koendv commented Apr 6, 2023 via email

@dragonmux
Copy link
Member Author

dragonmux commented Apr 6, 2023

Using a newer BMDA with older firmware should work (there are fallbacks in BMDA for this) - if it doesn't, please let us know as that's a bug.

Do note that v1.8 firmware with BMDA from main will fall back right now to the low-level JTAG/SWD remote functions as the protocol rewrite done in #1389 did not extend to providing a complete set of backwards compatible remote protocol functions for firmware with remote protocol versions 1 and 2, so that may be part of where the slowness you were experiencing is coming from

@ALTracer ALTracer mentioned this pull request Nov 19, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Confirmed bug GDB Issue/PR related to GDB Regression Bug caused by a regression
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gdb_main_loop
3 participants