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)
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)]
    |        ^^^^^^^^^^^^^^^^