Skip to content

Commit 9b2ed76

Browse files
committed
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.
1 parent 55b7f8e commit 9b2ed76

File tree

4 files changed

+68
-37
lines changed

4 files changed

+68
-37
lines changed

Diff for: compiler/rustc_codegen_llvm/src/asm.rs

+18-17
Original file line numberDiff line numberDiff line change
@@ -333,24 +333,25 @@ impl<'ll, 'tcx> AsmBuilderMethods<'tcx> for Builder<'_, 'll, 'tcx> {
333333
}
334334
attributes::apply_to_callsite(result, llvm::AttributePlace::Function, &{ attrs });
335335

336-
// Switch to the 'normal' basic block if we did an `invoke` instead of a `call`
337-
if let Some(dest) = dest {
338-
self.switch_to_block(dest);
339-
}
336+
// Write results to outputs. We need to do this for all possible control flow.
337+
for block in Some(dest).into_iter().chain(labels.iter().copied().map(Some)) {
338+
if let Some(block) = block {
339+
self.switch_to_block(block);
340+
}
340341

341-
// Write results to outputs
342-
for (idx, op) in operands.iter().enumerate() {
343-
if let InlineAsmOperandRef::Out { reg, place: Some(place), .. }
344-
| InlineAsmOperandRef::InOut { reg, out_place: Some(place), .. } = *op
345-
{
346-
let value = if output_types.len() == 1 {
347-
result
348-
} else {
349-
self.extract_value(result, op_idx[&idx] as u64)
350-
};
351-
let value =
352-
llvm_fixup_output(self, value, reg.reg_class(), &place.layout, instance);
353-
OperandValue::Immediate(value).store(self, place);
342+
for (idx, op) in operands.iter().enumerate() {
343+
if let InlineAsmOperandRef::Out { reg, place: Some(place), .. }
344+
| InlineAsmOperandRef::InOut { reg, out_place: Some(place), .. } = *op
345+
{
346+
let value = if output_types.len() == 1 {
347+
result
348+
} else {
349+
self.extract_value(result, op_idx[&idx] as u64)
350+
};
351+
let value =
352+
llvm_fixup_output(self, value, reg.reg_class(), &place.layout, instance);
353+
OperandValue::Immediate(value).store(self, place);
354+
}
354355
}
355356
}
356357
}

Diff for: tests/codegen/asm-goto.rs

+13-11
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,6 @@
66

77
use std::arch::asm;
88

9-
#[no_mangle]
10-
pub extern "C" fn panicky() {}
11-
12-
struct Foo;
13-
14-
impl Drop for Foo {
15-
fn drop(&mut self) {
16-
println!();
17-
}
18-
}
19-
209
// CHECK-LABEL: @asm_goto
2110
#[no_mangle]
2211
pub unsafe fn asm_goto() {
@@ -38,6 +27,19 @@ pub unsafe fn asm_goto_with_outputs() -> u64 {
3827
out
3928
}
4029

30+
// CHECK-LABEL: @asm_goto_with_outputs_use_in_label
31+
#[no_mangle]
32+
pub unsafe fn asm_goto_with_outputs_use_in_label() -> u64 {
33+
let out: u64;
34+
// CHECK: [[RES:%[0-9]+]] = callbr i64 asm sideeffect alignstack inteldialect "
35+
// CHECK-NEXT: to label %[[FALLTHROUGHBB:[a-b0-9]+]] [label %[[JUMPBB:[a-b0-9]+]]]
36+
asm!("{} /* {} */", out(reg) out, label { return out; });
37+
// CHECK: [[JUMPBB]]:
38+
// CHECK-NEXT: [[RET:%.+]] = phi i64 [ 1, %[[FALLTHROUGHBB]] ], [ [[RES]], %start ]
39+
// CHECK-NEXT: ret i64 [[RET]]
40+
1
41+
}
42+
4143
// CHECK-LABEL: @asm_goto_noreturn
4244
#[no_mangle]
4345
pub unsafe fn asm_goto_noreturn() -> u64 {

Diff for: tests/ui/asm/x86_64/goto.rs

+35-7
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,6 @@ fn goto_jump() {
3131
}
3232
}
3333

34-
// asm goto with outputs cause miscompilation in LLVM. UB can be triggered
35-
// when outputs are used inside the label block when optimisation is enabled.
36-
// See: https://github.com/llvm/llvm-project/issues/74483
37-
/*
3834
fn goto_out_fallthrough() {
3935
unsafe {
4036
let mut out: usize;
@@ -68,7 +64,38 @@ fn goto_out_jump() {
6864
assert!(value);
6965
}
7066
}
71-
*/
67+
68+
// asm goto with outputs cause miscompilation in LLVM when multiple outputs are present.
69+
// The code sample below is adapted from https://github.com/llvm/llvm-project/issues/74483
70+
// and does not work with `-C opt-level=0`
71+
#[expect(unused)]
72+
fn goto_multi_out() {
73+
#[inline(never)]
74+
#[allow(unused)]
75+
fn goto_multi_out(a: usize, b: usize) -> usize {
76+
let mut x: usize;
77+
let mut y: usize;
78+
let mut z: usize;
79+
unsafe {
80+
core::arch::asm!(
81+
"mov {x}, {a}",
82+
"test {a}, {a}",
83+
"jnz {label1}",
84+
"/* {y} {z} {b} {label2} */",
85+
x = out(reg) x,
86+
y = out(reg) y,
87+
z = out(reg) z,
88+
a = in(reg) a,
89+
b = in(reg) b,
90+
label1 = label { return x },
91+
label2 = label { return 1 },
92+
);
93+
0
94+
}
95+
}
96+
97+
assert_eq!(goto_multi_out(11, 22), 11);
98+
}
7299

73100
fn goto_noreturn() {
74101
unsafe {
@@ -102,8 +129,9 @@ fn goto_noreturn_diverge() {
102129
fn main() {
103130
goto_fallthough();
104131
goto_jump();
105-
// goto_out_fallthrough();
106-
// goto_out_jump();
132+
goto_out_fallthrough();
133+
goto_out_jump();
134+
// goto_multi_out();
107135
goto_noreturn();
108136
goto_noreturn_diverge();
109137
}

Diff for: tests/ui/asm/x86_64/goto.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
warning: unreachable statement
2-
--> $DIR/goto.rs:97:9
2+
--> $DIR/goto.rs:124:9
33
|
44
LL | / asm!(
55
LL | | "jmp {}",
@@ -13,7 +13,7 @@ LL | unreachable!();
1313
| ^^^^^^^^^^^^^^ unreachable statement
1414
|
1515
note: the lint level is defined here
16-
--> $DIR/goto.rs:87:8
16+
--> $DIR/goto.rs:114:8
1717
|
1818
LL | #[warn(unreachable_code)]
1919
| ^^^^^^^^^^^^^^^^

0 commit comments

Comments
 (0)