Add support for QCatchSyscalls #57
Conversation
There was a problem hiding this comment.
Thanks for yet another great PR!
As always, the core implementation looks great, and my comments are primarily nits.
I think the iterator approach is a fine way to pass syscall numbers to the Target. The alternative would be to break the API into multiple methods such as as catch_syscall(&mut self, syscall_no: Usize) and clear_filter(&mut self, syscall_no: Usize), where you iterate over the list within gdbstub and invoke catch_syscall for each syscall. I'll leave it up to you to decide how you'd like the API to work, since you've got hands on experience actually implementing the underlying Target functionality.
And thank you for taking the initiative to include the protocol output from your emulator! I'll update the PR template to include something along the lines of "if the feature isn't feasible to demo in armv4t, please include output from your own gdbstub integration".
- Refactor syscall number parsing code. - Add support returning error codes from catch syscall handlers
|
Updated with changes and rebased on top of the latest master. |
There was a problem hiding this comment.
I've got one final nit, but aside from that, LGTM!
Just a heads up, but I'll be out from the 28th (tomorrow) to the 31st (monday). As such, while we could probably merge this into master tonight, I don't think I'll have time to cut a 0.5.1 release until early next week.
|
Great, thanks. It's not an issue for me not being released -- I have many patched github dependencies already. |
|
Done. I've also updated the trace output in the PR description. |
|
Awesome. Thanks again! 🎉 |
Description
Implements
QCatchSyscallsextension, see:QCatchSyscalls.I needed this for finding a bug in my emulator.
Notes:
There is a minor issue where if GDB sends syscall numbers larger than
Arch::Usize, it will result in an integer overflow and the current implementation will silently ignore them and they will not reach the target. This is because I reused the argument handling code fromvRunto avoid allocations. This could probably be improved by writing a new handler, but I avoided doing for now, since I wasn't sure about using the iterator approach to pass the syscall numbers to the Target impl any way.API Stability
Checklist
cargo buildcompiles withouterrorsorwarningscargo clippyruns withouterrorsorwarningscargo fmtwas runexamples/armv4texamples/armv4twithRUST_LOG=trace+ any relevant GDB output under the "Validation" section below./scripts/test_dead_code_elim.shand/or./example_no_std/check_size.sh)Validation
GDB output
armv4t output
Since the example implementation never actually performs a syscall, I've also attached a trace of it running in my emulator (binary just prints
Hello Worldusing a singlewritevsyscall).GDB output
Protocol output