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

Inline assembly with tied operands of different sizes #65452

Closed
ahomescu opened this issue Oct 15, 2019 · 2 comments
Closed

Inline assembly with tied operands of different sizes #65452

ahomescu opened this issue Oct 15, 2019 · 2 comments
Labels
A-inline-assembly Area: Inline assembly (`asm!(…)`) P-low Low priority requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ahomescu
Copy link
Contributor

When transpiling (or manually translating) the following C code from /usr/include/bits/select.h:

# define __FD_ZERO(fdsp) \
  do {									      \
    int __d0, __d1;							      \
    __asm__ __volatile__ ("cld; rep; " __FD_ZERO_STOS			      \
			  : "=c" (__d0), "=D" (__d1)			      \
			  : "a" (0), "0" (sizeof (fd_set)		      \
					  / sizeof (__fd_mask)),	      \
			    "1" (&__FDS_BITS (fdsp)[0])			      \
			  : "memory");					      \
  } while (0)

into Rust, we get approximately the following code (manually rewritten to make it cleaner):

    let mut x: u32 = 0;
    let mut y: u32;
    let mut z: u32;
    unsafe {
        asm!("cld; rep stosb"
             : "={di}"(y), "={cx}"(z)
             : "{al}"(42), "0"(&mut x), "1"(4)
             : "memory"
             : "volatile"
             );
    }

the resulting Rust inline assembly works fine in release mode, but crashes in debug mode.
Link to Playground

The issue here is that the "0"(&mut x) input operand is tied to the same register as the "={di}"(y) output operand, but they have different types and sizes (&mut u32 vs u32). LLVM assigns both operands to the edi register in debug mode (which is wrong) but to rdi in release mode (which is the expected assignment).

gcc and clang compile the original C code correctly because they extend both operands to the larger size in the front end (link to clang code). The equivalent correct Rust code would be something like

    unsafe {
        let y64: u64;
        asm!("cld; rep stosb"
             : "={di}"(y64), "={cx}"(z)
             : "{al}"(42), "0"(&mut x), "1"(4)
             : "memory"
             : "volatile"
             );
        y = y64 as u32;
    }

Should rustc expect to never see this input (I think it's LLVM UB), or replicate clang's behavior? If it's the latter, I could implement that myself and submit a PR, but I'm trying to get some feedback before writing any code.

@Centril Centril added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-inline-assembly Area: Inline assembly (`asm!(…)`) P-low Low priority labels Oct 16, 2019
@nagisa nagisa added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. and removed A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. labels Oct 16, 2019
@nagisa
Copy link
Member

nagisa commented Oct 16, 2019

Should rustc expect to never see this input (I think it's LLVM UB), or replicate clang's behavior?

The current implementation of Rust’s asm! is pretty much "pass it through to LLVM" and any larger changes to it, such as replicating some of the clang’s behaviour, are likely going to be a part of the larger inline assembly design work that will be necessary for inline assembly to stabilise.

Nevertheless this issue is a good corner case to consider when exploring our options and, eventually, implementing a new inline asm. Thanks for the report!

@Centril Centril added the requires-nightly This issue requires a nightly compiler in some way. label Oct 25, 2019
@Amanieu
Copy link
Member

Amanieu commented May 23, 2020

This issue does not apply to the new asm! (RFC 2850) which properly rejects tied operands of different sizes.

The legacy llvm_asm! is deprecated and is no longer maintained.

@Amanieu Amanieu closed this as completed May 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-inline-assembly Area: Inline assembly (`asm!(…)`) P-low Low priority requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants