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

Add bochs magic breakpoint, read instruction pointer, inline instructions #79

Merged
merged 3 commits into from
Jun 26, 2019

Conversation

64
Copy link
Contributor

@64 64 commented Jun 24, 2019

  • Added functions for a bochs magic breakpoint and reading the instruction pointer
  • Marked small single-instruction functions as #[inline]
  • Improved the docs for the segmentation functions

Copy link
Member

@phil-opp phil-opp left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the pull request! It looks good to me overall, I only have a few suggestions for the read_rip function.

src/instructions/mod.rs Show resolved Hide resolved
/// Gets the current instruction pointer. Note that this is only approximate as it requires a few
/// instructions to execute.
#[inline]
pub fn read_rip() -> u64 {
Copy link
Member

Choose a reason for hiding this comment

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

I think this function fits better in the registers module, given that rip is a CPU register.


/// Gets the current instruction pointer. Note that this is only approximate as it requires a few
/// instructions to execute.
#[inline]
Copy link
Member

Choose a reason for hiding this comment

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

This function should probably be inline(always). Without always, this is just a hint to the compiler that this might be useful for inlining. But this function does not make much sense without inlining, as it would always return the same value independent of the calling location.

unsafe {
asm!(
"call .next
.next:
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if inline assembly does support local labels (prefixed with a dot). With global_asm, which uses the same syntax, such labels are treated as global labels so that a linker error occurs if another .next label is defined elsewhere in the program. Could you try whether this is the case for inline assembly too?

In case it is, it might make sense to manually mangle that label name to something less common. Alternatively, it seems that lea rax, [rip] also works to get the rip value, without needing any labels.

@64
Copy link
Contributor Author

64 commented Jun 26, 2019

done

@phil-opp phil-opp merged commit 38e57ac into rust-osdev:master Jun 26, 2019
@phil-opp
Copy link
Member

Thanks!

phil-opp added a commit that referenced this pull request Jun 26, 2019
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.

2 participants