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

Change rip size to 8 in X86_64CoreRegId. #87

Merged
merged 2 commits into from
Sep 28, 2021
Merged

Conversation

gz
Copy link
Contributor

@gz gz commented Sep 27, 2021

API Stability

  • This PR does not require a breaking API change

Fixes #80.

Checklist

I ran this in the past but this change was with the github editor :)

@daniel5151
Copy link
Owner

Seems like Eflags is also incorrect? Shouldn't it be 4?

@daniel5151
Copy link
Owner

Ah, and it seems you'll need to update the test as well.

@gz
Copy link
Contributor Author

gz commented Sep 27, 2021

Ha, totally broke the tests with a change that I figured would never do that :) I'm gonna fix it..

Regarding eflags, on x86-64 eflags was technically renamed to rflags and extended to be 64bit (even though the upper 32 bits are not really used):
https://docs.rs/x86/0.42.0/src/x86/bits64/rflags.rs.html#14

I will check the xml of gdb as well to see if it's really 8 bytes there.

@gz
Copy link
Contributor Author

gz commented Sep 27, 2021

Hm seems to be still 32bit in gdb: https://github.com/bminor/binutils-gdb/blob/master/gdb/features/i386/64bit-core.c#L49

So I can change this too.

@daniel5151
Copy link
Owner

daniel5151 commented Sep 27, 2021

Haha, all good.

Regardless what the x86 spec says, we'll need to match GDB's target XML. If they're only using 32 bits, then we should match that behavior.

Of course, if you want to use the full 64-bit value, you can always define a custom Arch + target.xml that uses the full 64 bits. For this gdbstub_arch implementation though, we rely on the target.xml files present on the GDB client, so we'll need to match those.

This also fixes the tests because it corresponds again
to what is in `X86_64CoreRegs`.
@gz
Copy link
Contributor Author

gz commented Sep 28, 2021

Fixed tests, let me know if there are still other things missing.

@daniel5151 daniel5151 merged commit d616b2b into daniel5151:dev/0.6 Sep 28, 2021
@daniel5151
Copy link
Owner

Nice, thanks for the fix 🎉

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.

RIP size on x86-64
2 participants