blackmagic-debug / blackmagic

In application debugger for ARM Cortex microcontrollers.
GNU General Public License v3.0
3.22k stars 767 forks source link

Semihost: regression from v1.9.3 to aa67538 #1845

Closed maksimdrachov closed 3 months ago

maksimdrachov commented 3 months ago

One of my semihost on-target tests is failing when updating the BlackMagicProbe firmware from v1.9.3 to the latest aa6753833764b41697900536395bb343f611377d in main.

The issue arises when I try to write a negative value through semihost:

auto val = semihost_persistence::add("./logs/test_add", -3);
zassert_equal(-3, val.value());

(semihost_persistence::add will rely on both read as well as write calls)

The debugger will hang in the following manner:

image

Positive values work in both versions.

This is using SAMC21.

pavel-kirienko commented 3 months ago

The problem is also found in the v1.10.2 release.

ALTracer commented 3 months ago

Weird. These are semihosting file read & write calls? Please exit TUI, enable gdb logging for remote (gdb) set debug remote on and record to file (gdb) set logging enabled on, set logging file gdb1.txt something more detailed than a word-wrapped screenshot. I'm not sure whether your probes support ENABLE_DEBUG=1 or whether BMDA would reproduce it. Relevant merged PRs are 1686, but also 1726, 1730, if it's pressing you can try to manually test or bisect across them. Take note that 1686 was backported to v1.10 before v1.10.1 (and v1.10.2 includes it) so may exhibit brokenness I did not address -- test on v1.10.0 to confirm it's from v1.9->v1.10 development cycle.

dragonmux commented 3 months ago

The problem is also found in the v1.10.2 release.

Semihosting is known slightly broken in that release, Pavel, and is nothing to do with this. See #1732 for the complete low-down on that. As this user is testing main, this is irrelevant to them as they're testing post-1732.

dragonmux commented 3 months ago

The issue arises when I try to write a negative value through semihost:

auto val = semihost_persistence::add("./logs/test_add", -3);
zassert_equal(-3, val.value());

(semihost_persistence::add will rely on both read as well as write calls)

It would be helpful if you can share what your semihost_persistence class instance is doing in the add method please and which semihosting call it's trying to make when it appears to go wrong. Likewise, please drop out the TUI and provide: bt + info regs in your semihost_exec(). There's just too little information in your screenshot to go on and without also seeing the output of semihosting that's wrong, there's nothing here to debug right now.

Likewise as ALTracer suggests, use BMDA as blackmagic -v 5 to get a log of the semihosting activity and possibly even the hang. We can't even tell which syscall is being made to know where to start looking.

maksimdrachov commented 3 months ago

After re-testing (note for all first three below, I've applied a patch that added samc21 support). I have concluded the following:

(All compiled using gcc-arm-none-eabi-9-2020-q2-update, v1.9.x doesn't compile when using more recent versions)


Here's an attempt at debugging, using the last main version:

gdb_debug.txt

Likewise, please drop out the TUI and provide: bt + info regs in your semihost_exec().

I'm assuming you mean when the bug occurs? The issue is that when it does, the gdb session hangs and I'm not able to put in any command anymore:

image

Todo after lunch:

maksimdrachov commented 3 months ago

I hope this is helpful for debugging:

I think the cause of the issue is to be found here:

image
ALTracer commented 3 months ago

I've found the problem on https://github.com/blackmagic-debug/blackmagic/blob/aa6753833764b41697900536395bb343f611377d/src/target/semihosting.c#L180-L190 In PR1686 I fixed [whatever existed previously] incompletely, and the 'F' check also matches against m-replies containing first (LE) byte of 0xF0..0xFF emitted by BMP, because a) BMP reuses the gdb_packet_buffer due to SRAM constraints; b) no extra info is used otherwise. I reproduced this on Koen's sketch SemihostingTest.ino modified, and blackmagic-test-fw-archive does not contain tests for file read/writes.