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

Can bypass validation of asm! in a function that is never used via inline(always) #36718

Closed
japaric opened this issue Sep 25, 2016 · 11 comments
Closed
Labels
A-inline-assembly Area: inline asm!(..) C-bug Category: This is a bug. F-asm `#![feature(asm)]` (not `llvm_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

@japaric
Copy link
Member

japaric commented Sep 25, 2016

STR

NOTE target = x86_64-unknown-linux-gnu in all cases

This is rejected:

#![feature(asm)]

unsafe fn bkpt() {
    // ARM specific instruction
    asm!("bkpt");
}
$ rustc asm.rs
error: <inline asm>:1:2: error: invalid instruction mnemonic 'bkpt'
        bkpt
        ^

But this is accepted:

#![feature(asm)]

#[inline(always)]
unsafe fn bkpt() {
    asm!("bkpt");
}
$ rustc asm.rs && echo OK
OK

Don't worry though, because you can't actually use the ARCH specific instruction:

#![feature(asm)]

fn main() {
    unsafe {
        bkpt();
    }
}

#[inline(always)]
unsafe fn bkpt() {
    asm!("bkpt");
}
$ rustc asm.rs
error: <inline asm>:1:2: error: invalid instruction mnemonic 'bkpt'
        bkpt
        ^

Additional comments

This is a pretty useful bug/feature! You can use it cargo test the arch-agnostic parts of a crate
designed for ARM on x86 without adding a bunch of cfg attributes:

#![feature(asm)]

// ARM only
#[inline(always)]
pub unsafe fn control() -> u32 {
    let r: u32;
    asm!("msr CONTROL, $0" : "=r"(r));
    r
}

// arch-agnostic stuff
pub fn foo() -> bool {
    true
}

#[test]
fn bar() {
    assert!(foo());
}
$ cargo test
running 1 test
test bar ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured

Without the inline(always) trick, you'll have to add a cfg(target_arch = "arm") to the bkpt
function to get this to compile.

I don't think we want people to rely on this though. Specially, if the inline(always) trick stops
working in the future.

Meta

$ rustc -V
rustc 1.13.0-nightly (4f9812a59 2016-09-21)

cc @brson I'll let you decide how bad this is.

@hanna-kruppe
Copy link
Contributor

hanna-kruppe commented Sep 25, 2016

This is probably a side effect #36524 (which avoids generating code for #[inline] functions in crates/CGUs that don't call them) combined with this asm validation being performed by LLVM (i.e., after codegen).

@japaric
Copy link
Member Author

japaric commented Sep 25, 2016

Can confirm that the inline(always) variant is rejected by nightly-2016-09-20 which doesn't include #36524. cc @michaelwoerister

@alexcrichton
Copy link
Member

cc #29722, the tracking issue for asm!

@michaelwoerister
Copy link
Member

Good catch, @japaric!
Yes, if asm! statements are only verified in trans, than my recent PR will prevent the compiler from doing so in many cases. Two questions:

  • Will asm be verified later on, once the inline function gets instantiated somewhere?
  • How much is asm verification bound to trans? Could it also be done when just the MIR is available?

@michaelwoerister
Copy link
Member

OK, it seems that

  • @japaric already answered the first question in the initial description. Verification happens once the function is used somewhere, and
  • this verification is done by LLVM, so there is no simple way to do it without generating LLVM IR for the given function.

The question is, how do we want to handle this. I'd very much prefer if translation would always succeed once things pass analysis. On the other hand, before the PR, the compiler also failed as late as translation, so the only difference is that now things can fail in a downstream crate. I bet though that even before the inlining PR, we had exactly the same problem with generic functions, that is, an asm! block would not get verified either until the generic function was instantiated somewhere.

@michaelwoerister michaelwoerister added I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 28, 2016
@michaelwoerister
Copy link
Member

Nominating because I want to discuss if there is anything we can do or if we even want to do something here. The specific question is:

  • Inline assembly only gets fully verified once it ends up in LLVM, which is not the case for unused generic functions and now also inline functions. Are we OK with seemingly successfully compiling code into an rlib that will fail in a downstream crate because it contains invalid inline asssembly?

@pnkfelix
Copy link
Member

marking P-low

as noted above, this is another special case instance of code that can end up in an rlib and then cause a compilation failure further downstream. (We cannot really do anything about it without making our own verification of asm, rather than relying on LLVM's)

@pnkfelix pnkfelix added the P-low Low priority label Sep 29, 2016
@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 26, 2017
@jonas-schievink jonas-schievink added A-inline-assembly Area: inline asm!(..) requires-nightly This issue requires a nightly compiler in some way. labels Apr 5, 2020
@Amanieu
Copy link
Member

Amanieu commented May 24, 2020

I think that this is fundamentally unsolvable in the same way an #[inline(always)] function could call a non-existent extern function since that would only be detected at link time.

@jonas-schievink
Copy link
Contributor

Triage: Also happens with the new inline assembly, but that even accepts this code on x86_64:

#![feature(asm)]

unsafe fn bkpt() {
    asm!("bkpt aa aaa aa");
}

@jonas-schievink jonas-schievink added the F-asm `#![feature(asm)]` (not `llvm_asm`) label Aug 31, 2020
@bstrie bstrie changed the title can bypass validation of asm! in a function declaration using inline(always) Can bypass validation of asm! in a function that is never used via inline(always) Oct 11, 2020
@bstrie
Copy link
Contributor

bstrie commented Feb 3, 2022

Confirmed that using the version of asm that is set to become stable later this month, both versions of the function from the original report (with and without #[inline(always)]) now compile (the version where the function is actually used still fails).

We must conclude that either this is intended behavior, or else this needs to block the stabilization of asm, since becoming stricter later would cause programs on stable Rust to stop compiling. Does anyone think this should block the stabilization of asm? @Amanieu , if you claim this is fundamentally unsolvable then it seems reasonable to declare this to be intended behavior and close this issue.

@Amanieu
Copy link
Member

Amanieu commented Feb 4, 2022

I think this is fundamentally unsolvable in the same way that we can't catch linker errors caused by unused inline functions. So I'm going to close this and just say that this is the intended behavior as you suggested.

@Amanieu Amanieu closed this as completed Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-inline-assembly Area: inline asm!(..) C-bug Category: This is a bug. F-asm `#![feature(asm)]` (not `llvm_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

10 participants