Report malformed remote protocol data instead of defaulting to zero
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 331
- Forks
- 33
- PR merge metrics
- No merged PRs in 30d
Description
Follow-up to #1164 / #1166.
#1166 stops the crash by parsing remote stub data with a non-throwing helper that returns 0 on malformed input. That means a corrupt packet silently becomes a valid-looking answer — a bad thread id turns into thread 0, and we then show the user the wrong thread's registers with no indication anything is wrong. Accepted deliberately to keep the fix local during feature freeze.
The session should instead end deliberately, reporting the offending field and data. An attempt at that was dropped from #1166 (commit f7a1961); review of it showed what a real implementation needs:
- Wider catch boundary than
ExecuteAdapterAndWait.NotifyStoppedruns after it returns and refreshes caches;HandleSpontaneousAdapterStophas no boundary;WorkerThreadMainruns tasks bare, so an escape isstd::terminate;InvokeBackendCommandreaches monitor-command decoding directly from the API/FFI. - Real transport teardown. A synthetic
TargetExitedonly updates BN-side state. The teardown lives in the adapter (m_socket->Kill(),delete m_rspConnector,InvalidateCache). Alsom_adapteris never reset on any session end today, soCreateDebugAdapterreuses it. - RAII on the stop channel. An exception between setting and clearing
m_inAdapterWaitleaves a stale pending stop that swallows later adapter events. - Ignore unrecognized stop-reply fields.
PacketToUnorderedMapparses everykey:valueas hex, but the spec requires unknown pairs be ignored and defines non-hex values we advertise support for (replaylog:begin,exec:<hex pathname>,fork/vforkpPID.TID). Parsing only the keys we consume is both spec-correct and smaller.
Outstanding regardless of the exception work:
esrevenadapter.cppstill uses rawstd::stoull(:1797,:2249,:2251) andstd::stoi(:2309) — the call sites #1164 describes.PacketToUnorderedMap:T05thread:1:garbage;splits into three tokens and falls through topacket_map[key] = 0; values are truncated to 16 chars before validation, so trailing garbage disappears.GdbMiAdapter::ReadMemorychecksfrom_chars().ecbut not full consumption, so"aZ"parses as0x0a; odd-length replies truncate; hard errors return an all-zero buffer indistinguishable from valid memory.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reading the dropped implementation in commit f7a1961 and the exception boundaries around ExecuteAdapterAndWait, NotifyStopped, HandleSpontaneousAdapterStop, WorkerThreadMain, and InvokeBackendCommand. Inspect esrevenadapter.cpp, PacketToUnorderedMap, and GdbMiAdapter::ReadMemory for the listed parsing problems. Done means malformed data is reported with its field and value, sessions tear down fully, stop-channel state is exception-safe, and unrecognized stop-reply fields are ignored.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- devtools, reverse-engineering
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100