inline asm!: ban OpReturn/OpReturnValue (they're always UB).

Author: eddyb

crates/rustc_codegen_spirv/src/builder/spirv_asm.rs

@@ -163,7 +163,7 @@ impl<'a, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'tcx> {
             (false, AsmBlock::Open) => (),
             (false, AsmBlock::End(terminator)) => {
                 self.err(&format!(
-                    "trailing terminator {terminator:?} requires `options(noreturn)`"
+                    "trailing terminator `Op{terminator:?}` requires `options(noreturn)`"
                 ));
             }
         }
@@ -356,6 +356,19 @@ impl<'cx, 'tcx> Builder<'cx, 'tcx> {
             }
 
             op => {
+                // NOTE(eddyb) allowing the instruction to be added below avoids
+                // spurious "`noreturn` requires a terminator at the end" errors.
+                if let Op::Return | Op::ReturnValue = op {
+                    self.struct_err(&format!(
+                        "using `Op{op:?}` to return from within `asm!` is disallowed"
+                    ))
+                    .note(
+                        "resuming execution, without falling through the end \
+                        of the `asm!` block, is always undefined behavior",
+                    )
+                    .emit();
+                }
+
                 self.emit()
                     .insert_into_block(dr::InsertPoint::End, inst)
                     .unwrap();
@@ -370,7 +383,9 @@ impl<'cx, 'tcx> Builder<'cx, 'tcx> {
                     }
                     AsmBlock::End(terminator) => {
                         if op != Op::Label {
-                            self.err(&format!("expected OpLabel after terminator {terminator:?}"));
+                            self.err(&format!(
+                                "expected `OpLabel` after terminator `Op{terminator:?}`"
+                            ));
                         }
 
                         AsmBlock::Open

tests/ui/lang/asm/block_tracking_fail.stderr

@@ -4,13 +4,13 @@ error: `noreturn` requires a terminator at the end
 11 |         asm!("", options(noreturn));
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: trailing terminator Unreachable requires `options(noreturn)`
+error: trailing terminator `OpUnreachable` requires `options(noreturn)`
   --> $DIR/block_tracking_fail.rs:18:9
    |
 18 |         asm!("OpUnreachable");
    |         ^^^^^^^^^^^^^^^^^^^^^
 
-error: expected OpLabel after terminator Kill
+error: expected `OpLabel` after terminator `OpKill`
   --> $DIR/block_tracking_fail.rs:25:9
    |
 25 | /         asm!(

tests/ui/lang/asm/block_tracking_pass.rs

@@ -8,7 +8,7 @@ use spirv_std::spirv;
 fn asm_label() {
     unsafe {
         asm!(
-            "OpReturn",          // close active block
+            "OpKill",            // close active block
             "%unused = OpLabel", // open new block
         );
     }

tests/ui/lang/asm/issue-1002.rs

@@ -0,0 +1,49 @@
+// Tests that we don't allow returning from `asm!` (which would always be UB).
+// build-fail
+
+use core::arch::asm;
+use spirv_std::spirv;
+
+fn asm_return() {
+    unsafe {
+        asm!("OpReturn", options(noreturn));
+    }
+}
+
+fn asm_return_value(x: u32) -> u32 {
+    unsafe {
+        asm!(
+            "OpReturnValue {x}",
+            x = in(reg) x,
+            options(noreturn),
+        );
+    }
+}
+
+fn asm_return_label() {
+    unsafe {
+        asm!(
+            "OpReturn",          // close active block
+            "%unused = OpLabel", // open new block
+        );
+    }
+}
+
+fn asm_return_value_label(x: u32) -> u32 {
+    unsafe {
+        asm!(
+            "OpReturnValue {x}", // close active block
+            "%unused = OpLabel", // open new block
+            x = in(reg) x
+        );
+    }
+    0
+}
+
+#[spirv(fragment)]
+pub fn main() {
+    asm_return();
+    asm_return_value(123);
+    asm_return_label();
+    asm_return_value_label(123);
+}

tests/ui/lang/asm/issue-1002.stderr

@@ -0,0 +1,45 @@
+error: using `OpReturn` to return from within `asm!` is disallowed
+ --> $DIR/issue-1002.rs:9:9
+  |
+9 |         asm!("OpReturn", options(noreturn));
+  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  |
+  = note: resuming execution, without falling through the end of the `asm!` block, is always undefined behavior
+
+error: using `OpReturnValue` to return from within `asm!` is disallowed
+  --> $DIR/issue-1002.rs:15:9
+   |
+15 | /         asm!(
+16 | |             "OpReturnValue {x}",
+17 | |             x = in(reg) x,
+18 | |             options(noreturn),
+19 | |         );
+   | |_________^
+   |
+   = note: resuming execution, without falling through the end of the `asm!` block, is always undefined behavior
+
+error: using `OpReturn` to return from within `asm!` is disallowed
+  --> $DIR/issue-1002.rs:25:9
+   |
+25 | /         asm!(
+26 | |             "OpReturn",          // close active block
+27 | |             "%unused = OpLabel", // open new block
+28 | |         );
+   | |_________^
+   |
+   = note: resuming execution, without falling through the end of the `asm!` block, is always undefined behavior
+
+error: using `OpReturnValue` to return from within `asm!` is disallowed
+  --> $DIR/issue-1002.rs:34:9
+   |
+34 | /         asm!(
+35 | |             "OpReturnValue {x}", // close active block
+36 | |             "%unused = OpLabel", // open new block
+37 | |             x = in(reg) x
+38 | |         );
+   | |_________^
+   |
+   = note: resuming execution, without falling through the end of the `asm!` block, is always undefined behavior
+
+error: aborting due to 4 previous errors
+