Be stricter about rejecting LLVM reserved registers in asm! #84658

Amanieu · 2021-04-28T15:31:55Z

LLVM will silently produce incorrect code if these registers are used as operands.

cc @rust-lang/wg-inline-asm

rust-highfive · 2021-04-28T15:31:58Z

r? @petrochenkov

(rust-highfive has picked a reviewer for you, use r? to override)

petrochenkov · 2021-04-28T20:59:04Z

@bors r+

bors · 2021-04-28T20:59:05Z

📌 Commit e6a731e has been approved by petrochenkov

Be stricter about rejecting LLVM reserved registers in asm! LLVM will silently produce incorrect code if these registers are used as operands. cc `@rust-lang/wg-inline-asm`

Be stricter about rejecting LLVM reserved registers in asm! LLVM will silently produce incorrect code if these registers are used as operands. cc ``@rust-lang/wg-inline-asm``

Be stricter about rejecting LLVM reserved registers in asm! LLVM will silently produce incorrect code if these registers are used as operands. cc ```@rust-lang/wg-inline-asm```

Be stricter about rejecting LLVM reserved registers in asm! LLVM will silently produce incorrect code if these registers are used as operands. cc ````@rust-lang/wg-inline-asm````

jackh726 · 2021-04-30T11:04:37Z

Possibly failed in #84739 (comment)

@bors rollup=iffy

Amanieu · 2021-04-30T17:46:46Z

@bors r=petrochenkov

bors · 2021-04-30T17:46:48Z

📌 Commit 2fb751c1edd59b7199fe33207767c01f1cd98487 has been approved by petrochenkov

haraldh · 2021-05-03T09:08:52Z

The enclu operation takes rbx as an input.
Do you suggest to change:

        asm!(
            "enclu",

            in("rax") EREPORT,
            in("rbx") ti.as_ptr(),
            in("rcx") data.0.as_ptr(),
            in("rdx") report.as_mut_ptr(),
        );

to

        asm!(
            "xchg rsi, rbx",
            "enclu",
            "xchg rsi, rbx",

            in("rax") EREPORT,
            in("rsi") ti.as_ptr(),
            in("rcx") data.0.as_ptr(),
            in("rdx") report.as_mut_ptr(),
        );

Amanieu · 2021-05-03T09:14:19Z

Yes. See the changes here.

haraldh · 2021-05-03T09:23:57Z

@Amanieu thanks!

error: invalid register `rbx`: rbx is used internally by LLVM and cannot be used as an operand for inline asm See: rust-lang/rust#84658 (comment) and: https://github.com/rust-lang/rust/pull/84658/files#diff-d7283132d97a993fad4e2d491ac883dbce4e17fe248cdf37fa3f9334e2a5a115

error: invalid register rbx: rbx is used internally by LLVM and cannot be used as an operand for inline asm See: rust-lang/rust#84658 (comment) and: https://github.com/rust-lang/rust/pull/84658/files#diff-d7283132d97a993fad4e2d491ac883dbce4e17fe248cdf37fa3f9334e2a5a115 Signed-off-by: Harald Hoyer <harald@redhat.com>

error: invalid register `rbx`: rbx is used internally by LLVM and cannot be used as an operand for inline asm See: rust-lang/rust#84658 (comment) and: https://github.com/rust-lang/rust/pull/84658/files#diff-d7283132d97a993fad4e2d491ac883dbce4e17fe248cdf37fa3f9334e2a5a115

jethrogb · 2021-05-07T21:18:08Z

compiler/rustc_target/src/asm/x86.rs

+    match arch {
+        InlineAsmArch::X86 => Ok(()),
+        InlineAsmArch::X86_64 => {
+            Err("rbx is used internally by LLVM and cannot be used as an operand for inline asm")


This seems like a problematic restriction. Many x86 instructions that you might want to run with asm! take rbx as an operand. Same applies to esi below, really.

From the RFC:

The intent is that support for asm! should be independent of the rustc back-end used

Given that, I think backend limitations such as these should be handled by rustc prior to passing the assembly to the backend. It shouldn't be visible to the user. Otherwise, asm! would only be able to support the intersection of all inline assembly implementations across all codegen backends.

Handling this in rustc is actually non-trivial (see this long Zulip discussion), which is also why LLVM doesn't just do it for us.

error: invalid register rbx: rbx is used internally by LLVM and cannot be used as an operand for inline asm So, we pull in a newer `sgx` crate version, which has this fixed. See: rust-lang/rust#84658 (comment) and: https://github.com/rust-lang/rust/pull/84658/files#diff-d7283132d97a993fad4e2d491ac883dbce4e17fe248cdf37fa3f9334e2a5a115 Signed-off-by: Harald Hoyer <harald@redhat.com>

error: invalid register rbx: rbx is used internally by LLVM and cannot be used as an operand for inline asm See: rust-lang/rust#84658 (comment) and: https://github.com/rust-lang/rust/pull/84658/files#diff-d7283132d97a993fad4e2d491ac883dbce4e17fe248cdf37fa3f9334e2a5a115 Signed-off-by: Harald Hoyer <harald@redhat.com>

error: invalid register rbx: rbx is used internally by LLVM and cannot be used as an operand for inline asm So, we pull in a newer `sgx` crate version, which has this fixed. See: rust-lang/rust#84658 (comment) and: https://github.com/rust-lang/rust/pull/84658/files#diff-d7283132d97a993fad4e2d491ac883dbce4e17fe248cdf37fa3f9334e2a5a115 Signed-off-by: Harald Hoyer <harald@redhat.com>

Fix: - clippy::needless_borrow Silence: - clippy::manual_flatten Not addressed: - deprecated `llvm_asm!()` is deprecated, but must be used. The new `asm!()` does not allow using `ebx` [1], which holds the argument for the SMMSTORE command. [1]: rust-lang/rust#84658 Signed-off-by: Tim Crawford <tcrawford@system76.com>

Fix: - clippy::needless_borrow - clippy::bool_assert_comparison Silence: - clippy::manual_flatten Not addressed: - deprecated `llvm_asm!()` is deprecated, but must be used. The new `asm!()` does not allow using `ebx` [1], which holds the argument for the SMMSTORE command. [1]: rust-lang/rust#84658 Signed-off-by: Tim Crawford <tcrawford@system76.com>

rust-highfive assigned petrochenkov Apr 28, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 28, 2021

This comment has been minimized.

Sign in to view

Amanieu force-pushed the reserved_regs branch from a15f862 to a3485b6 Compare April 28, 2021 15:53

This comment has been minimized.

Sign in to view

Be stricter about rejecting LLVM reserved registers in asm!

e6a731e

Amanieu force-pushed the reserved_regs branch from a3485b6 to e6a731e Compare April 28, 2021 17:30

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 28, 2021

Dylan-DPC-zz mentioned this pull request Apr 29, 2021

Rollup of 8 pull requests #84689

Closed

Dylan-DPC-zz mentioned this pull request Apr 29, 2021

Rollup of 7 pull requests #84691

Closed

Dylan-DPC-zz mentioned this pull request Apr 29, 2021

Rollup of 8 pull requests #84693

Closed

Dylan-DPC-zz mentioned this pull request Apr 30, 2021

Rollup of 4 pull requests #84739

Closed

Avoid using rbx in SGX inline assembly since it is reserved

09cfb24

Reserve x18 on AArch64 and un-reserve x16

ea310d9

Amanieu force-pushed the reserved_regs branch from 2fb751c to ea310d9 Compare May 1, 2021 12:26

bors merged commit 603a42e into rust-lang:master May 1, 2021

rustbot added this to the 1.54.0 milestone May 1, 2021

bors mentioned this pull request May 1, 2021

Move all sys::ext modules to os #84200

Merged

haraldh mentioned this pull request May 3, 2021

fix: do not use rbx register in asm!() enarx/sgx#33

Merged

haraldh mentioned this pull request May 3, 2021

Make enarx-keepldr compile again enarx-archive/enarx-keepldr#388

Merged

phil-opp mentioned this pull request May 5, 2021

Change register used in setting SS in stage_4 rust-osdev/bootloader#156

Merged

jethrogb reviewed May 7, 2021

View reviewed changes

jethrogb mentioned this pull request May 7, 2021

LLVM-reserved registers can't be used in inline assembly #85056

Open

Goirad mentioned this pull request May 11, 2021

Register rbx is now reserved in asm! macros fortanix/rust-sgx#335

Closed

nodakai mentioned this pull request Jul 6, 2021

Avoiding use of LLVM reserved register rbx fortanix/rust-sgx#336

Merged

andjo403 mentioned this pull request Oct 3, 2021

Avoid using ebx as an asm! operand rust-lang/measureme#185

Merged

crawfxrd mentioned this pull request Oct 19, 2021

Update toolchain to 2021-06-15 system76/firmware-smmstore#17

Merged

johnmave126 mentioned this pull request Jan 12, 2023

Inline assembly fails to compile after allowing inlining on the enclosing function for i686-pc-windows-msvc #106781

Open

mratsim mentioned this pull request Jun 12, 2023

Field arithmetic benchmark + reworked assembly (+40% perf) privacy-scaling-explorations/halo2curves#49

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Be stricter about rejecting LLVM reserved registers in asm! #84658

Be stricter about rejecting LLVM reserved registers in asm! #84658

Amanieu commented Apr 28, 2021

rust-highfive commented Apr 28, 2021

This comment has been minimized.

This comment has been minimized.

petrochenkov commented Apr 28, 2021

bors commented Apr 28, 2021

jackh726 commented Apr 30, 2021

Amanieu commented Apr 30, 2021

bors commented Apr 30, 2021

haraldh commented May 3, 2021

Amanieu commented May 3, 2021

haraldh commented May 3, 2021

jethrogb May 7, 2021

jethrogb May 7, 2021

Amanieu May 7, 2021

Be stricter about rejecting LLVM reserved registers in asm! #84658

Be stricter about rejecting LLVM reserved registers in asm! #84658

Conversation

Amanieu commented Apr 28, 2021

rust-highfive commented Apr 28, 2021

This comment has been minimized.

This comment has been minimized.

petrochenkov commented Apr 28, 2021

bors commented Apr 28, 2021

jackh726 commented Apr 30, 2021

Amanieu commented Apr 30, 2021

bors commented Apr 30, 2021

haraldh commented May 3, 2021

Amanieu commented May 3, 2021

haraldh commented May 3, 2021

jethrogb May 7, 2021

Choose a reason for hiding this comment

jethrogb May 7, 2021

Choose a reason for hiding this comment

Amanieu May 7, 2021

Choose a reason for hiding this comment