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

Fix bad conversion from llvm_asm! to asm! #218

Merged
merged 1 commit into from
Dec 28, 2020

Conversation

tomaka
Copy link
Contributor

@tomaka tomaka commented Dec 28, 2020

When updating x86_64 from 0.11.x to 0.12.x, I've gotten the following compilation error:

error: invalid instruction mnemonic 'xchgw'
  --> /home/pierre/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/x86_64-0.12.3/src/instructions/mod.rs:51:15
   |
51 |         asm!("xchgw bx, bx", options(nomem, nostack));
   |               ^
   |
note: instantiated into assembly here
  --> <inline asm>:2:2
   |
2  |     xchgw bx, bx
   |     ^^^^^

According to this Bochs-related page, the instruction is indeed just xchg, while xchgw is the weird AT&T syntax equivalent.

My code compiles with this change, but I haven't tested if the Bochs breakpoint actually works.

@tomaka
Copy link
Contributor Author

tomaka commented Dec 28, 2020

Note that just running cargo build on the source code of x86_64 doesn't seem to actually verify the assembly. You can try this by writing asm!("xchgfoobarbaz bx, bx") and it will still compile.
I don't know exactly how it works, but it's likely that it's only the linker that checks the correctness of the assembly.

@phil-opp
Copy link
Member

Thanks a lot for reporting this! I wasn't aware that asm! is only checked when it's used, but it seems like you're right: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=74fb87fe9c686b6e4beffb1b1e1fd564

I opened a Rust issue about this at rust-lang/rust#80440 to see if this is a bug or the intended behavior.

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