Deduplication of functions containing inline assembly contradicts Reference's documentation about code-patching.

[zachs18]

(I'm not sure if this should be considered a codegen bug (i.e. the Reference is correct) or a documentation bug (i.e. the codegen is correct). This issue assumes it is a codegen bug. If this is determined to be a documentation bug, then I can instead report this on the rust-lang/reference repo).

I tried this code:

#![no_std]
use core::arch::asm;
pub fn foo() {
    unsafe { asm!("nop"); }
}
pub fn bar() {
    unsafe { asm!("nop"); }
}

View the resulting assembly using either godbolt.org or cargo-show-asm.

I expected to see this happen: foo and bar are not deduplicated, due to the Rust Reference's documentation that

The compiler cannot assume that the instructions in the asm are the ones that will actually end up executed.
This effectively means that the compiler must treat the asm! as a black box and only take the interface specification into account, not the instructions themselves.
Runtime code patching is allowed, via target-specific mechanisms.

# expected output for --target x86_64-unknown-linux-gnu
example::foo:
  push rax
  
  nop
  
  pop rax
  ret

example::bar:
  push rax
  
  nop
  
  pop rax
  ret

Instead, this happened: When compiled with optimizations (-Copt-level=2 or above), foo and bar are deduplicated (godbolt link)

# actual output for --target x86_64-unknown-linux-gnu
example::bar:
  push rax
  
  nop
  
  pop rax
  ret

Notes:

• This only happens when the asm invocations are exactly the same, thus a workaround is to add spaces to the asm template string to make them different.
• Different extern "ABI" and #[no_mangle] do not inherently prevent deduplication.
• Different #[link_section = "..."]s work to prevent deduplication (if the functions are given the same #[link_section = "..."] then they are still deduplicated).

Meta

Happens on nightly (2023-10-16), stable (1.73.0), and 1.59.0 (when inline asm was stabilized) (and probably all versions between).

rustc --version --verbose:

rustc 1.75.0-nightly (49691b1f7 2023-10-16)
binary: rustc
commit-hash: 49691b1f70d71dd7b8349c332b7f277ee527bf08
commit-date: 2023-10-16
host: x86_64-unknown-linux-gnu
release: 1.75.0-nightly
LLVM version: 17.0.2

rustc 1.73.0 (cc66ad468 2023-10-03)
binary: rustc
commit-hash: cc66ad468955717ab92600c770da8c1601a4ff33
commit-date: 2023-10-03
host: x86_64-unknown-linux-gnu
release: 1.73.0
LLVM version: 17.0.2

(no backtrace)

@rustbot label +A-inline-assembly

[workingjubilee]

@zachs18 can you test specifically with -Zmerge-functions?

[the8472]

The compiler cannot assume that the instructions in the asm are the ones that will actually end up executed.
This effectively means that the compiler must treat the asm! as a black box and only take the interface specification into account, not the instructions themselves.
Runtime code patching is allowed, via target-specific mechanisms.

This language could be interpreted as the compiler cannot make assumptions about the assembly because they might get patched, without actually promising that all assembly blocks can be patched in useful ways.

It might some annotation to opt out of unnamed_addr in the IR.

[zachs18]

@workingjubilee Yes, -Zmerge-functions=disabled does make the functions not be merged (godbolt link).

[workingjubilee]

This language could be interpreted as the compiler cannot make assumptions about the assembly because they might get patched, without actually promising that all assembly blocks can be patched in useful ways.

That's certainly already what happens in some cases?
Given

#![no_std]
use core::arch::asm;
pub fn foo<A>() {
    unsafe { asm!("nop"); }
}

fn main() {
    let _ = foo::<u16>();
    let _ = foo::<i32>();
    let _ = foo::<f64>();
}

The compiler does not promise there will be exactly three nops in the program.

[asquared31415]

I'm not sure there's a way to locate instructions in a non-naked function in ways that are guaranteed to work by Rust. There can be arbitrary code around the asm, so an exported fn symbol doesn't suffice. You can't export a symbol from the asm! because it's not guaranteed that it's only emitted once, so there would be conflicting symbols. The best thing I can think of is inserting a well known constant pattern and searching for that, but I'm not sure if it's permitted to read the bytes of a function (maybe only via assembly and that's arch specific?)

However this bug also occurs with #[naked] functions where it would be possible to modify the asm using the exported function symbol, so there's at least a bug there, even if the compiler would be permitted to merge non-naked functions.

[asquared31415]

@rustbot label -needs-triage +T-compiler +A-codegen

[workingjubilee]

I'm not sure there's a way to locate instructions in a non-naked function in ways that are guaranteed to work by Rust. There can be arbitrary code around the asm, so an exported fn symbol doesn't suffice. You can't export a symbol from the asm! because it's not guaranteed that it's only emitted once, so there would be conflicting symbols. The best thing I can think of is inserting a well known constant pattern and searching for that, but I'm not sure if it's permitted to read the bytes of a function (maybe only via assembly and that's arch specific?)

...Technically, a SIGILL is always guaranteed to find the instructions, no?

[saethlin]

cc @rust-lang/opsem is this a soundness hole?

[BattyBoopers]

I just came across a very similar issue here.

However, while it could be argued that two actually identical functions like in the original example might get deduplicated, my issue here is that apparently code is getting deduplicated just based on the text-contents of the asm block, even though repeating the block in two different functions has different meaning.
This basically defines a static variable in assembler and returns a pointer to it, so any repetition of the exact same inline-assembler code is semantically different.

use std::arch::asm;
use std::hint::black_box;

fn foo_() -> *mut u64 {
    let p: *mut u64;
    unsafe {
        asm! {
            "lea {p},[rip + 1f]",
            ".pushsection .bss",
            ".align 8",
            "1:",
            ".quad 0",
            ".popsection",
            p = out(reg) p,
        }
    };
    p
}

fn foo() -> *mut u64 {
    let f: fn() -> _ = black_box(foo_); // prevent inlining
    f()
}

fn bar_() -> *mut u64 {
    let p: *mut u64;
    unsafe {
        asm! {
            "lea {p},[rip + 1f]",
            ".pushsection .bss",
            ".align 8",
            "1:",
            ".quad 0",
            ".popsection",
            p = out(reg) p,
        }
    };
    p
}

fn bar() -> *mut u64 {
    let f: fn() -> _ = black_box(bar_); // prevent inlining
    f()
}

fn main() {
    println!("{:x?} {:x?}", foo(), bar());
}

The two addresses printed should be different, but they are identical due to the incorrect code-deduplication.

I'm not enitirely sure if this should be considered a separate issue instead, since the case for this being incorrect is a lot stronger than the original issue.

Furthermore, I would also expect any inlining of one of the functions to return the same address, which it currently doesn't, likely for the same reason. The only way the asm instructions should currently be allowed to be repeated verbatim and thus generate a different static variable for the same function call is if it is used inside generic code which is monomorphisized in different code-generation units (The reason for that is the the same reason we currently can't have any generic static variables, which is annoying, but a different issue).

[RalfJung]

@Amanieu what is the expected behavior here?

[Amanieu]

The current behavior is correct: the compiler is allowed to deduplicate identical inline assembly. Perhaps this could be clarified in the documentation.

Essentially, if you intend to do any kind of hot patching then you need some way to uniquely identify the assembly that you intend to patch. This is usually done by recording the address of the assembly code in a separate section like this:

0:
    my_patchable_instruction

.section patch_list, "a"
.balign 8
.quad 0b
.previous

This will work correctly no matter how many times the asm is duplicated or deduplicated since you'll just end up with more or fewer entries in the patch_list section. If you need different behavior for different blocks then you should use different section names, which will inhibit deduplication.

[RalfJung]

Should this be mentioned or at least hinted at in the documentation? I can see how the current docs (quoted in the OP) could lead to misunderstandings here.

[Amanieu]

rust-lang/reference#1441