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

probe: stlink: Fix 1-byte transfers #1476

Merged
merged 1 commit into from
Nov 29, 2022

Conversation

fkjagodzinski
Copy link
Contributor

@fkjagodzinski fkjagodzinski commented Nov 23, 2022

Internally the 1-byte transfers are handled in 3 phases:

  1. read/write 8-bit chunks from every unaligned addresses until the first aligned address is reached,
  2. read/write 32-bit chunks from all aligned addresses,
  3. read/write 8-bit chunks from the remaining unaligned addresses.

Size of the first unaligned read/write is set to the result of address (4-byte) alignment check and can be either 1, 2, or 3 bytes (the value of unaligned_count calculated as addr & 0x3). This is incorrect and every transfer with the requested size smaller than unaligned_count is terminated with the following error:

Unhandled exception in handle_message (b'm'): result size (3) != requested size (1) [gdbserver]


Skip the first unaligned transfer if the requested size is smaller than the result of the address alignment check. Handle the whole request in the second, unaligned read/write (phase 3. mentioned above).

@fkjagodzinski
Copy link
Contributor Author

The issue was discovered in the latest release of pyOCD (0.34.2) when debugging stm32u585aiix target, and can be exposed e.g.

(gdb) x/128xb 0x20000000
0x20000000 <foo>:	0x00	0x00	Cannot access memory at address 0x20000002
(gdb) 

Matching pyocd log and traceback:

0155554 E Unhandled exception in handle_message (b'm'): result size (2) != requested size (1) [gdbserver]
Traceback (most recent call last):
  File "/usr/local/lib/python3.10/site-packages/pyocd/gdbserver/gdbserver.py", line 419, in handle_message
    reply = handler(msg[msgStart:])
  File "/usr/local/lib/python3.10/site-packages/pyocd/gdbserver/gdbserver.py", line 840, in get_memory
    mem = self.target_context.read_memory_block8(addr, length)
  File "/usr/local/lib/python3.10/site-packages/pyocd/debug/cache.py", line 42, in read_memory_block8
    return self._memcache.read_memory_block8(addr, size)
  File "/usr/local/lib/python3.10/site-packages/pyocd/cache/memory.py", line 258, in read_memory_block8
    assert len(result) == size, "result size ({}) != requested size ({})".format(len(result), size)
AssertionError: result size (2) != requested size (1)

When inspected with pdb, this leads to read_memory_block8:

(Pdb) bt
  /usr/local/Cellar/python@3.10/3.10.8/Frameworks/Python.framework/Versions/3.10/lib/python3.10/threading.py(973)_bootstrap()
-> self._bootstrap_inner()
  /usr/local/Cellar/python@3.10/3.10.8/Frameworks/Python.framework/Versions/3.10/lib/python3.10/threading.py(1016)_bootstrap_inner()
-> self.run()
  <local-pyocd-source>/pyocd/gdbserver/gdbserver.py(333)run()
-> self._run_connection()
  <local-pyocd-source>/pyocd/gdbserver/gdbserver.py(385)_run_connection()
-> resp = self.handle_message(packet)
  <local-pyocd-source>/pyocd/gdbserver/gdbserver.py(422)handle_message()
-> reply = handler(msg[msgStart:])
  <local-pyocd-source>/pyocd/gdbserver/gdbserver.py(840)get_memory()
-> mem = self.target_context.read_memory_block8(addr, length)
  <local-pyocd-source>/pyocd/debug/cache.py(42)read_memory_block8()
-> return self._memcache.read_memory_block8(addr, size)
  <local-pyocd-source>/pyocd/cache/memory.py(255)read_memory_block8()
-> combined = self._read(addr, size)
  <local-pyocd-source>/pyocd/cache/memory.py(137)_read()
-> uncachedData = self._read_uncached(uncached)
  <local-pyocd-source>/pyocd/cache/memory.py(97)_read_uncached()
-> data = self._context.read_memory_block8(uncachedIv.begin, uncachedIv.end - uncachedIv.begin)
  <local-pyocd-source>/pyocd/coresight/cortex_m.py(493)read_memory_block8()
-> data = self.ap.read_memory_block8(addr, size)
  <local-pyocd-source>/pyocd/utility/concurrency.py(29)_locking()
-> return func(self, *args, **kwargs)
  <local-pyocd-source>/pyocd/coresight/ap.py(1236)_accelerated_read_memory_block8()
-> return self._accelerated_memory_interface.read_memory_block8(addr, size,
> <local-pyocd-source>/pyocd/probe/stlink_probe.py(304)read_memory_block8()
-> addr &= 0xffffffff
(Pdb) 

@fkjagodzinski
Copy link
Contributor Author

Related (or pretty much the same 😉 ) issue -- #1444.

@flit
Copy link
Member

flit commented Nov 23, 2022

Thanks! And helpful detail in the description.

I'm about to push a change to remove Python 3.6 from the CI workflow, so it will run successfully.

@fkjagodzinski
Copy link
Contributor Author

The way we calculate unaligned_count was incorrect, so I extended the patch to fix it.

@DariusBabrauskas
Copy link

I think this commit not correct too. See fix at #1428 in comments.

pyocd/probe/stlink_probe.py Show resolved Hide resolved
pyocd/probe/stlink_probe.py Outdated Show resolved Hide resolved
Copy link
Member

@flit flit left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for the added comments.

Just noticed: you should probably add your copyright to the file header comment (meaning I'd prefer it 😄). I'll give you a chance before merging.

@flit
Copy link
Member

flit commented Nov 28, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Internally the 1-byte transfers are handled in 3 phases:
1. read/write 8-bit chunks until the first aligned address is reached,
2. read/write 32-bit chunks from all aligned addresses,
3. read/write 8-bit chunks from the remaining unaligned addresses.

Size of the first unaligned read/write is set to the result of address
alignment check (4-byte) and can be either 1, 2, or 3 bytes (the value
of `unaligned_count` calculated as `addr & 0x3`). This is incorrect and
every transfer with the requested size smaller than `unaligned_count` is
terminated with the following error:

  Unhandled exception in handle_message (b'm'): result size (3) != requested size (1) [gdbserver]

Skip the first unaligned transfer if the requested size is so small that
phase-1 would not even reach aligned address. Handle the whole request
in the second unaligned read/write (phase-3).
@fkjagodzinski
Copy link
Contributor Author

Just noticed: you should probably add your copyright to the file header comment (meaning I'd prefer it 😄). I'll give you a chance before merging.

OK, updated the copyright. Too bad this cancelled the whole CI pipeline. 😅

@flit
Copy link
Member

flit commented Nov 28, 2022

Thanks!

@flit
Copy link
Member

flit commented Nov 28, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@flit
Copy link
Member

flit commented Nov 28, 2022

Btw, I plan to cherry pick this fix to main and release a bug fix update quickly. Thanks again.

@flit flit merged commit 4e3eaaf into pyocd:develop Nov 29, 2022
flit pushed a commit to flit/pyOCD that referenced this pull request Nov 29, 2022
Internally the 1-byte transfers are handled in 3 phases:
1. read/write 8-bit chunks until the first aligned address is reached,
2. read/write 32-bit chunks from all aligned addresses,
3. read/write 8-bit chunks from the remaining unaligned addresses.

Size of the first unaligned read/write is set to the result of address
alignment check (4-byte) and can be either 1, 2, or 3 bytes (the value
of `unaligned_count` calculated as `addr & 0x3`). This is incorrect and
every transfer with the requested size smaller than `unaligned_count` is
terminated with the following error:

  Unhandled exception in handle_message (b'm'): result size (3) != requested size (1) [gdbserver]

Skip the first unaligned transfer if the requested size is so small that
phase-1 would not even reach aligned address. Handle the whole request
in the second unaligned read/write (phase-3).
@fkjagodzinski fkjagodzinski deleted the fix/stlink-1-byte-read branch November 29, 2022 17:27
flit pushed a commit to flit/pyOCD that referenced this pull request Dec 10, 2022
Internally the 1-byte transfers are handled in 3 phases:
1. read/write 8-bit chunks until the first aligned address is reached,
2. read/write 32-bit chunks from all aligned addresses,
3. read/write 8-bit chunks from the remaining unaligned addresses.

Size of the first unaligned read/write is set to the result of address
alignment check (4-byte) and can be either 1, 2, or 3 bytes (the value
of `unaligned_count` calculated as `addr & 0x3`). This is incorrect and
every transfer with the requested size smaller than `unaligned_count` is
terminated with the following error:

  Unhandled exception in handle_message (b'm'): result size (3) != requested size (1) [gdbserver]

Skip the first unaligned transfer if the requested size is so small that
phase-1 would not even reach aligned address. Handle the whole request
in the second unaligned read/write (phase-3).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants