about summary refs log tree commit diff
diff options
context:
space:
mode:
authorGary Guo <gary@garyguo.net>2024-10-11 00:23:32 +0100
committerGary Guo <gary@garyguo.net>2024-11-24 14:18:10 +0000
commitb8df869ebb9219b98ca798aacb16c9be3f84496b (patch)
tree387e3bf6c0ba767d6e9ec9e463925a5f91b9de98
parentf5d18576856ef45d1e47de79889ae7db9d1afa29 (diff)
downloadrust-b8df869ebb9219b98ca798aacb16c9be3f84496b.tar.gz
rust-b8df869ebb9219b98ca798aacb16c9be3f84496b.zip
Fix asm goto with outputs
When outputs are used together with labels, they are considered
to be written for all destinations, not only when falling through.
-rw-r--r--compiler/rustc_codegen_llvm/src/asm.rs35
-rw-r--r--tests/codegen/asm/goto.rs24
-rw-r--r--tests/ui/asm/x86_64/goto.rs42
-rw-r--r--tests/ui/asm/x86_64/goto.stderr4
4 files changed, 68 insertions, 37 deletions
diff --git a/compiler/rustc_codegen_llvm/src/asm.rs b/compiler/rustc_codegen_llvm/src/asm.rs
index bb74dfe1487..2455d13df8d 100644
--- a/compiler/rustc_codegen_llvm/src/asm.rs
+++ b/compiler/rustc_codegen_llvm/src/asm.rs
@@ -342,24 +342,25 @@ impl<'ll, 'tcx> AsmBuilderMethods<'tcx> for Builder<'_, 'll, 'tcx> {
         }
         attributes::apply_to_callsite(result, llvm::AttributePlace::Function, &{ attrs });
 
-        // Switch to the 'normal' basic block if we did an `invoke` instead of a `call`
-        if let Some(dest) = dest {
-            self.switch_to_block(dest);
-        }
+        // Write results to outputs. We need to do this for all possible control flow.
+        for block in Some(dest).into_iter().chain(labels.iter().copied().map(Some)) {
+            if let Some(block) = block {
+                self.switch_to_block(block);
+            }
 
-        // Write results to outputs
-        for (idx, op) in operands.iter().enumerate() {
-            if let InlineAsmOperandRef::Out { reg, place: Some(place), .. }
-            | InlineAsmOperandRef::InOut { reg, out_place: Some(place), .. } = *op
-            {
-                let value = if output_types.len() == 1 {
-                    result
-                } else {
-                    self.extract_value(result, op_idx[&idx] as u64)
-                };
-                let value =
-                    llvm_fixup_output(self, value, reg.reg_class(), &place.layout, instance);
-                OperandValue::Immediate(value).store(self, place);
+            for (idx, op) in operands.iter().enumerate() {
+                if let InlineAsmOperandRef::Out { reg, place: Some(place), .. }
+                | InlineAsmOperandRef::InOut { reg, out_place: Some(place), .. } = *op
+                {
+                    let value = if output_types.len() == 1 {
+                        result
+                    } else {
+                        self.extract_value(result, op_idx[&idx] as u64)
+                    };
+                    let value =
+                        llvm_fixup_output(self, value, reg.reg_class(), &place.layout, instance);
+                    OperandValue::Immediate(value).store(self, place);
+                }
             }
         }
     }
diff --git a/tests/codegen/asm/goto.rs b/tests/codegen/asm/goto.rs
index e522d0da5b4..0e69c4d5840 100644
--- a/tests/codegen/asm/goto.rs
+++ b/tests/codegen/asm/goto.rs
@@ -6,17 +6,6 @@
 
 use std::arch::asm;
 
-#[no_mangle]
-pub extern "C" fn panicky() {}
-
-struct Foo;
-
-impl Drop for Foo {
-    fn drop(&mut self) {
-        println!();
-    }
-}
-
 // CHECK-LABEL: @asm_goto
 #[no_mangle]
 pub unsafe fn asm_goto() {
@@ -38,6 +27,19 @@ pub unsafe fn asm_goto_with_outputs() -> u64 {
     out
 }
 
+// CHECK-LABEL: @asm_goto_with_outputs_use_in_label
+#[no_mangle]
+pub unsafe fn asm_goto_with_outputs_use_in_label() -> u64 {
+    let out: u64;
+    // CHECK: [[RES:%[0-9]+]] = callbr i64 asm sideeffect alignstack inteldialect "
+    // CHECK-NEXT: to label %[[FALLTHROUGHBB:[a-b0-9]+]] [label %[[JUMPBB:[a-b0-9]+]]]
+    asm!("{} /* {} */", out(reg) out, label { return out; });
+    // CHECK: [[JUMPBB]]:
+    // CHECK-NEXT: [[RET:%.+]] = phi i64 [ 1, %[[FALLTHROUGHBB]] ], [ [[RES]], %start ]
+    // CHECK-NEXT: ret i64 [[RET]]
+    1
+}
+
 // CHECK-LABEL: @asm_goto_noreturn
 #[no_mangle]
 pub unsafe fn asm_goto_noreturn() -> u64 {
diff --git a/tests/ui/asm/x86_64/goto.rs b/tests/ui/asm/x86_64/goto.rs
index 6c14bb57ac6..ab22317c458 100644
--- a/tests/ui/asm/x86_64/goto.rs
+++ b/tests/ui/asm/x86_64/goto.rs
@@ -31,10 +31,6 @@ fn goto_jump() {
     }
 }
 
-// asm goto with outputs cause miscompilation in LLVM. UB can be triggered
-// when outputs are used inside the label block when optimisation is enabled.
-// See: https://github.com/llvm/llvm-project/issues/74483
-/*
 fn goto_out_fallthrough() {
     unsafe {
         let mut out: usize;
@@ -68,7 +64,38 @@ fn goto_out_jump() {
         assert!(value);
     }
 }
-*/
+
+// asm goto with outputs cause miscompilation in LLVM when multiple outputs are present.
+// The code sample below is adapted from https://github.com/llvm/llvm-project/issues/74483
+// and does not work with `-C opt-level=0`
+#[expect(unused)]
+fn goto_multi_out() {
+    #[inline(never)]
+    #[allow(unused)]
+    fn goto_multi_out(a: usize, b: usize) -> usize {
+        let mut x: usize;
+        let mut y: usize;
+        let mut z: usize;
+        unsafe {
+            core::arch::asm!(
+                "mov {x}, {a}",
+                "test {a}, {a}",
+                "jnz {label1}",
+                "/* {y} {z} {b} {label2} */",
+                x = out(reg) x,
+                y = out(reg) y,
+                z = out(reg) z,
+                a = in(reg) a,
+                b = in(reg) b,
+                label1 = label { return x },
+                label2 = label { return 1 },
+            );
+            0
+        }
+    }
+
+    assert_eq!(goto_multi_out(11, 22), 11);
+}
 
 fn goto_noreturn() {
     unsafe {
@@ -102,8 +129,9 @@ fn goto_noreturn_diverge() {
 fn main() {
     goto_fallthough();
     goto_jump();
-    // goto_out_fallthrough();
-    // goto_out_jump();
+    goto_out_fallthrough();
+    goto_out_jump();
+    // goto_multi_out();
     goto_noreturn();
     goto_noreturn_diverge();
 }
diff --git a/tests/ui/asm/x86_64/goto.stderr b/tests/ui/asm/x86_64/goto.stderr
index 27e227d71a5..216b4d252ec 100644
--- a/tests/ui/asm/x86_64/goto.stderr
+++ b/tests/ui/asm/x86_64/goto.stderr
@@ -1,5 +1,5 @@
 warning: unreachable statement
-  --> $DIR/goto.rs:97:9
+  --> $DIR/goto.rs:124:9
    |
 LL | /         asm!(
 LL | |             "jmp {}",
@@ -13,7 +13,7 @@ LL |           unreachable!();
    |           ^^^^^^^^^^^^^^ unreachable statement
    |
 note: the lint level is defined here
-  --> $DIR/goto.rs:87:8
+  --> $DIR/goto.rs:114:8
    |
 LL | #[warn(unreachable_code)]
    |        ^^^^^^^^^^^^^^^^