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

RIP size on x86-64 #80

Closed
gz opened this issue Sep 25, 2021 · 7 comments · Fixed by #85 or #87
Closed

RIP size on x86-64 #80

gz opened this issue Sep 25, 2021 · 7 comments · Fixed by #85 or #87
Labels
API-non-breaking Non-breaking API change bug Something isn't working gdbstub_arch Related to code in `gdbstub_arch`

Comments

@gz
Copy link
Contributor

gz commented Sep 25, 2021

rip size is currently 4 bytes, but maybe it should be 8?

https://github.com/bminor/binutils-gdb/blob/master/gdb/features/i386/64bit-core.xml lists it as:

  <reg name="rip" bitsize="64" type="code_ptr"/>
@bet4it
Copy link
Contributor

bet4it commented Sep 25, 2021

I will fix it in a following PR ( with another way ) , wait for me😅

@gz
Copy link
Contributor Author

gz commented Sep 25, 2021

random thought: it might be nice to statically type-encode the size of the registers when when they're passed to the read/write_registers. One way could be to move the val: &[u8] and dst: &mut [u8] in some enum with the RegId and encode the length explicitly (specify as &[u8; 8] etc.)?

I'm thinking that might save some slice length checks when implementing these functions. But not sure if even possible just a thought :)

@daniel5151
Copy link
Owner

@gz I assume you meant read/write_register (as opposed to read/write_registers)?

If so, then you might find this discussion from the initial implementation PR interesting: #22 (comment)

Though now that I'm re-reading this idea a year later, I realize that there might be some easier approaches to pull off something similar... I certainly wouldn't be opposed to having stricter type-safety in these APIs, so I'm open to any ideas!

@daniel5151
Copy link
Owner

daniel5151 commented Sep 25, 2021

And as for the original issue: yes, rip having a size of 4 seems like an oversight in the initial #34 implementation.

Pinging @keiichiw, since this might translate to a subtle correctness issue in your implementation.

@daniel5151 daniel5151 added API-non-breaking Non-breaking API change bug Something isn't working labels Sep 25, 2021
@gz
Copy link
Contributor Author

gz commented Sep 25, 2021

Yes I meant the single register writes with the dynamic val and dst slices.

@daniel5151 daniel5151 added the gdbstub_arch Related to code in `gdbstub_arch` label Sep 25, 2021
@daniel5151
Copy link
Owner

@gz if this is something you'd like fixed sooner, feel free to send a quick fixup PR. Otherwise, a fix for this will land alongside some other changes to read_register in #85

@gz
Copy link
Contributor Author

gz commented Sep 27, 2021

Sent pull request, yes would be nice to have this fixed, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API-non-breaking Non-breaking API change bug Something isn't working gdbstub_arch Related to code in `gdbstub_arch`
Projects
None yet
3 participants