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

Contents from global_asm! leak into other assemblies #81838

Open
nagisa opened this issue Feb 6, 2021 · 4 comments
Open

Contents from global_asm! leak into other assemblies #81838

nagisa opened this issue Feb 6, 2021 · 4 comments
Labels
A-inline-assembly Area: Inline assembly (`asm!(…)`) C-bug Category: This is a bug. F-asm `#![feature(asm)]` (not `llvm_asm`) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nagisa
Copy link
Member

nagisa commented Feb 6, 2021

In code like this as noted here

#![feature(global_asm)]
#![feature(asm)]

global_asm!(r"
.macro foo, arg, size
  add \arg, \arg, \size
.endm");

const BAR: u64 = 10;

pub fn bar(mut v: u64) -> u64 {
    unsafe { 
        asm!("foo {v}, {}", const(BAR), v = inout(reg) v);
    }
    v
}

the macro will leak into the inline asm!, but this is likely extremely brittle. For instance, Rust is free to split this module into 2 distinct CGUs, making the code like this mysteriously work or break.

This could probably be mitigated by isolating all the global_asm! into its own CGU – that way none of the inline asm invocations would be able to observe the global_asm! stuff. Alternatively, if we find we want to support this behaviour we would be forced to collect all uses of global_asm! and items containing inline asm in a crate into a single CGU. Supporting this would also have implications to alternative backend support.

cc #35119

@nagisa nagisa added A-inline-assembly Area: Inline assembly (`asm!(…)`) F-asm `#![feature(asm)]` (not `llvm_asm`) labels Feb 6, 2021
@jgouly
Copy link

jgouly commented Feb 6, 2021

The example code above is mine, and I like it "leaking" as it allows me to use const and sym.

If global_asm was extended to support const and sym too, that would also be a useful alternative, to not leak.

#![feature(global_asm)]

const BAR: u64 = 10;

global_asm!("
.global bar
bar:
  add x0, x0, {bar}
  ret
", bar = const(BAR));

@jonas-schievink jonas-schievink added C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 6, 2021
@Amanieu
Copy link
Member

Amanieu commented Feb 6, 2021

We do not want to support this behavior. This only exists in LLVM to support building the Linux kernel which relies on a lot of GCC internal details.

However I'm less sure whether it's worth the trouble of messing with the CGU partitioning to actually enforce this.

@nagisa
Copy link
Member Author

nagisa commented Feb 6, 2021

If it is not enforced, people will rely on this and various other non-obvious forms of this unintended feature. It will become stable by convention, even if we say this should not work and people should not rely on it.

The only alternative I see is to keep global_asm! unstable until things we don't want to support are actually enforced, either through the stability system or via compiler's behaviour.

@Amanieu
Copy link
Member

Amanieu commented Feb 6, 2021

The same issue happens with just asm!. This isn't specific to global_asm!.

#![feature(asm)]

const BAR: u64 = 10;

pub fn bar(mut v: u64) -> u64 {
    unsafe { 
        asm!(r"
        .macro foo, arg, size
          add \arg, \size
        .endm");
        asm!("foo {v}, {}", const(BAR), v = inout(reg) v);
    }
    v
}

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. F-asm `#![feature(asm)]` (not `llvm_asm`) 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