Skip to content

Commit b8df869

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 f5d1857 commit b8df869

File tree

4 files changed

+68
-37
lines changed

4 files changed

+68
-37
lines changed

compiler/rustc_codegen_llvm/src/asm.rs

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -342,24 +342,25 @@ impl<'ll, 'tcx> AsmBuilderMethods<'tcx> for Builder<'_, 'll, 'tcx> {
342342
}
343343
attributes::apply_to_callsite(result, llvm::AttributePlace::Function, &{ attrs });
344344

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

350-
// Write results to outputs
351-
for (idx, op) in operands.iter().enumerate() {
352-
if let InlineAsmOperandRef::Out { reg, place: Some(place), .. }
353-
| InlineAsmOperandRef::InOut { reg, out_place: Some(place), .. } = *op
354-
{
355-
let value = if output_types.len() == 1 {
356-
result
357-
} else {
358-
self.extract_value(result, op_idx[&idx] as u64)
359-
};
360-
let value =
361-
llvm_fixup_output(self, value, reg.reg_class(), &place.layout, instance);
362-
OperandValue::Immediate(value).store(self, place);
351+
for (idx, op) in operands.iter().enumerate() {
352+
if let InlineAsmOperandRef::Out { reg, place: Some(place), .. }
353+
| InlineAsmOperandRef::InOut { reg, out_place: Some(place), .. } = *op
354+
{
355+
let value = if output_types.len() == 1 {
356+
result
357+
} else {
358+
self.extract_value(result, op_idx[&idx] as u64)
359+
};
360+
let value =
361+
llvm_fixup_output(self, value, reg.reg_class(), &place.layout, instance);
362+
OperandValue::Immediate(value).store(self, place);
363+
}
363364
}
364365
}
365366
}

tests/codegen/asm/goto.rs

Lines changed: 13 additions & 11 deletions
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 {

tests/ui/asm/x86_64/goto.rs

Lines changed: 35 additions & 7 deletions
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
}

tests/ui/asm/x86_64/goto.stderr

Lines changed: 2 additions & 2 deletions
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)