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 asm: The "m" constraint is broken #16383

Closed
main-- opened this issue Aug 9, 2014 · 8 comments
Closed

Inline asm: The "m" constraint is broken #16383

main-- opened this issue Aug 9, 2014 · 8 comments
Labels
A-inline-assembly Area: Inline assembly (`asm!(…)`) C-bug Category: This is a bug. 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

@main--
Copy link
Contributor

main-- commented Aug 9, 2014

#![feature(asm)]

fn main() {
    let foo = 42u;
    unsafe { asm!("lea rax, $0" :: "m"(foo) : "rax" : "intel"); }
}

Expected result: rax contains the address of foo

However:

unknown operand type!
UNREACHABLE executed at /build/buildd/rust-nightly-201408090406~39bafb0~trusty/src/llvm/lib/Target/X86/X86AsmPrinter.cpp:208!
Aborted (core dumped)

Using a struct instead of an uint:

#![feature(asm)]

struct Foo(u64, u64);

fn main() {
    let foo = Foo(42, 1337);
    unsafe { asm!("lea rax, $0" :: "m"(foo) : "rax" : "intel"); }
}

Now the compiler doesn't crash. But the emitted lea instruction references the memory location of the address of foo instead of foo itself:

; create foo
mov    QWORD PTR [rbp-0x10],0x2a
mov    QWORD PTR [rbp-0x8],0x539

; the (broken) implementation of the "m" constraint
lea    rax,[rbp-0x10] ; this instruction should not be emitted
mov    QWORD PTR [rbp-0x18],rax ; this instruction should not be emitted

 ; the actual inline asm
lea    rax,[rbp-0x18] ; this should be: lea rax, [rbp-0x10]

So basically "m"(foo) generates the code for "m"(&foo) if foo is a struct.

@kmcallister
Copy link
Contributor

Possibly related: #13366

@sfackler sfackler added A-inline-assembly Area: Inline assembly (`asm!(…)`) I-wrong labels Sep 26, 2014
@kmcallister
Copy link
Contributor

I tested the first example with Rust master (34dfa45) on x86-64 Linux. It only crashes when -O is enabled, and it doesn't crash with this equivalent (?) AT&T syntax assembly:

asm!("lea $0, %rax" :: "m"(foo) : "rax");

So I'd guess it's something with LLVM not knowing the operand size — I tried adding QWORD PTR but to no avail.

It seems like that's not related to the second issue, which also manifests with AT&T syntax.

@nodakai
Copy link
Contributor

nodakai commented Oct 20, 2014

The "g" constraint doesn't work, either:

#![feature(asm)]

fn main() {
    let mut foo = 42u;
//  unsafe { asm!("leaq $0, %rax" : : "m"(foo) : "rax"); }
    unsafe { asm!("pushq $0; popq $0" : "+g"(foo) ); }
}
$ rustc b16383.rs
b16383.rs:4:9: 4:16 warning: variable `foo` is assigned to, but never used, #[warn(unused_variables)] on by default
b16383.rs:4     let mut foo = 42u;
                    ^~~~~~~
b16383.rs:6:46: 6:49 warning: value assigned to `foo` is never read, #[warn(unused_assignments)] on by default
b16383.rs:6     unsafe { asm!("pushq $0; popq $0" : "+g"(foo) ); }
                                                         ^~~
note: in expansion of asm!
b16383.rs:6:14: 6:53 note: expansion site
rustc: /home/nodakai/src/rust-HEAD/src/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:6555: void llvm::SelectionDAGBuilder::visitInlineAsm(llvm::ImmutableCallSite): Assertion `OpInfo.isIndirect && "Memory output must be indirect operand"' failed.
zsh: abort (core dumped)  rustc b16383.rs
$ rustc -v verbose
rustc 0.13.0-dev (5cba29d33 2014-10-19 21:42:05 +0000)
binary: rustc
commit-hash: 5cba29d3343ee17b28d39c8d675aa0980d0c5b9c
commit-date: 2014-10-19 21:42:05 +0000
host: x86_64-unknown-linux-gnu
release: 0.13.0-dev

But I could confirm Clang/LLVM 3.0 corectly understood it:

#include <inttypes.h>

int main() {
    uint64_t a = 42;
    __asm__ __volatile__ ( "push %0; pop %0" : "+g"(a) );
    printf("%016" PRIu64 "x\n", a);

    return 0;
}
$ clang pushPop.c -S -o -O
...
.Ltmp5:
        .cfi_def_cfa_register %rbp
        subq    $16, %rsp
        movq    $42, -8(%rbp)
        movl    $42, %esi
        #APP
        push %rsi; pop %rsi
        #NO_APP
        movq    %rsi, -8(%rbp)
        movl    $.L.str, %edi
        xorb    %al, %al
        callq   printf
        xorl    %eax, %eax
        addq    $16, %rsp
        popq    %rbp
        ret
...
$ clang --version
Ubuntu clang version 3.0-6ubuntu3 (tags/RELEASE_30/final) (based on LLVM 3.0)
Target: x86_64-pc-linux-gnu
Thread model: posix

@steveklabnik
Copy link
Member

The original examples seem to work now, but @nodakai 's still don't.

@main--
Copy link
Contributor Author

main-- commented Sep 27, 2015

@steveklabnik Not on playpen though (as of right now). There, my first example still crashes (as @kmcallister mentioned, it will only crash with optimizations enabled) while the second one still miscompiles. The only difference is that after one year of optimization improvements, LLVM is now able to load the two 64-bit qwords as one 128-bit xmmword.

bors added a commit that referenced this issue Nov 4, 2015
The "m" memory constraint in inline assembly is broken (generates incorrect code or triggers LLVM asserts) and should not be used. Instead, indirect memory operands should be used with "\*m", "=\*m" and "+\*m".

Clang does this transparently by transforming "m" constraints into "\*m" indirect constraints, but for now just being able to use "\*m" directly is enough since asm! isn't stable.

While "\*m" works fine as an input operand, "=\*m" and "+\*m" need to be specified as input operands because they take a pointer value as an input. This PR relaxes the constraint checker to allow constraints starting with "=" or "+" if the constraint string contains a "\*", which indicates an indirect operand.

This (indirectly) fixes these issues: #29382, #16383 and #13366. The code will need to be changed to use "\*m" instead of "m".
@JustAPerson
Copy link
Contributor

Try loading a type larger than usize into a "m" constrained input. Causes an ICE. I suspect this is mostly asm!() not rejecting an invalid input, but the error occurs in MIR.

#![feature(asm)]

fn main() {
    struct Descriptor{
        size: usize,
        ptr:  usize,
    }
    let descriptor = Descriptor {
        size: 0,
        ptr:  0,
    };
    unsafe {
        asm!("lidt $0" :: "m"(descriptor) :: "intel");
    }
}
error: internal compiler error: ../src/librustc_trans/mir/operand.rs:82: impossible case reached
thread 'rustc' panicked at 'Box<Any>', ../src/librustc_errors/lib.rs:634
stack backtrace:
   1:     0x7fa594550cdd - std::sys::backtrace::tracing::imp::write::h29f5fdb9fc0a7395
   2:     0x7fa594561a71 - std::panicking::default_hook::_{{closure}}::h17d8437f66223ab1
   3:     0x7fa59455fed8 - std::panicking::default_hook::hbbe7fa36a995aca0
   4:     0x7fa5945606b8 - std::panicking::rust_panic_with_hook::h105c3d42fcd2fb5e
   5:     0x7fa58c2f6aa7 - std::panicking::begin_panic::hccc513334ab977d2
   6:     0x7fa58c30764a - rustc_errors::Handler::bug::h9fe35eb8aa73a45f
   7:     0x7fa590dd2334 - rustc::session::opt_span_bug_fmt::_{{closure}}::h62b0957667555cfe
   8:     0x7fa590d0e345 - rustc::session::opt_span_bug_fmt::hb71219f119a31511
   9:     0x7fa590d0e182 - rustc::session::bug_fmt::h8de2935acd9a57e4
  10:     0x7fa593444acf - _<collections..vec..Vec<T> as core..iter..traits..FromIterator<T>>::from_iter::
h2cbb281f2d52df9f
  11:     0x7fa593541109 - rustc_trans::mir::rvalue::_<impl rustc_trans..mir..MirContext<'bcx, 'tcx>>::tra
ns_rvalue::hdb93ac455c9e5a8c
  12:     0x7fa59353088c - rustc_trans::mir::block::_<impl rustc_trans..mir..MirContext<'bcx, 'tcx>>::tran
s_block::hd8dbf4a4fd69198b
  13:     0x7fa59352e618 - rustc_trans::mir::trans_mir::h2a26a93b40b40e45
  14:     0x7fa593489894 - rustc_trans::base::trans_closure::he79a96cff38dfe5c
  15:     0x7fa593550603 - rustc_trans::trans_item::TransItem::define::h1e5a74077917a073
  16:     0x7fa59348f566 - rustc_trans::base::trans_crate::h9baf3a0389546061
  17:     0x7fa594aecaad - rustc_driver::driver::phase_4_translate_to_llvm::h54e99578fb1bb696
  18:     0x7fa594b2bf96 - rustc_driver::driver::compile_input::_{{closure}}::h7236bd0d96fe8ce9
  19:     0x7fa594b13b4e - rustc_driver::driver::phase_3_run_analysis_passes::_{{closure}}::h7f2cec505064b
4bf
  20:     0x7fa594a71e9e - rustc::ty::context::TyCtxt::create_and_enter::he5dca3f2a7a0810a
  21:     0x7fa594ad64f8 - rustc_driver::driver::compile_input::hb4cc34cf85dc1edf
  22:     0x7fa594b00602 - rustc_driver::run_compiler::h50f95674bd902ab5
  23:     0x7fa594a47ddd - std::panicking::try::call::h31fc30b58c55d6c3
  24:     0x7fa59456ff16 - __rust_maybe_catch_panic
  25:     0x7fa594a60ac0 - _<F as alloc..boxed..FnBox<A>>::call_box::h24f3eb0b42327962
  26:     0x7fa59455e202 - std::sys::thread::Thread::new::thread_start::h8f3bd45211e9f5ea
  27:     0x7fa58b9eb6f9 - start_thread
  28:     0x7fa5941a0b5c - clone
  29:                0x0 - <unknown>

On rustc 1.12.0-nightly (1deb02ea6 2016-08-12).

@Mark-Simulacrum Mark-Simulacrum added C-bug Category: This is a bug. and removed C-enhancement Category: An issue proposing an enhancement or a PR with one. I-wrong labels Jul 21, 2017
@comex
Copy link
Contributor

comex commented May 14, 2018

The recent blog post "How a Rust upgrade more than tripled the speed of my code" describes an issue that I think is caused by this bug. The author wrote "m"(self_t[0]), where self_t: &[u64; 4]. This didn't crash, but it did unnecessarily copy self_t[0] to a new stack variable and produce a reference to that, rather than producing a pointer directly to self_t. Since their assembly only read a single u64 from that pointer, rather than writing or trying to read multiple u64s, this appeared to behave correctly, but was slow. However, in general this is a correctness issue, because it ought to be legal to pass a mutable lvalue and expect it to be modified; the alternative would be surprising for users experienced with the original GCC C extension, where it is legal.

See also #29543:

Clang does this transparently by transforming "m" constraints into "*m" indirect constraints, but for now just being able to use "*m" directly is enough since asm! isn't stable.

@Centril Centril added the requires-nightly This issue requires a nightly compiler in some way. label Oct 25, 2019
@jonas-schievink jonas-schievink added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 12, 2020
@Amanieu
Copy link
Member

Amanieu commented May 22, 2020

This issue does not apply to the new asm! (RFC 2850) which does not support memory operands.

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

@Amanieu Amanieu closed this as completed May 22, 2020
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 21, 2024
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!(…)`) C-bug Category: This is a bug. 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