Skip to content

Commit

Permalink
Fix asm goto with outputs
Browse files Browse the repository at this point in the history
When outputs are used together with labels, they are considered
to be written for all destinations, not only when falling through.
  • Loading branch information
nbdd0121 committed Oct 11, 2024
1 parent 4cc494b commit 4bd723f
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 37 deletions.
35 changes: 18 additions & 17 deletions compiler/rustc_codegen_llvm/src/asm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,24 +333,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);
}
}
}
}
Expand Down
24 changes: 13 additions & 11 deletions tests/codegen/asm-goto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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 {
Expand Down
42 changes: 35 additions & 7 deletions tests/ui/asm/x86_64/goto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
}
4 changes: 2 additions & 2 deletions tests/ui/asm/x86_64/goto.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
warning: unreachable statement
--> $DIR/goto.rs:97:9
--> $DIR/goto.rs:124:9
|
LL | / asm!(
LL | | "jmp {}",
Expand All @@ -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)]
| ^^^^^^^^^^^^^^^^
Expand Down

0 comments on commit 4bd723f

Please sign in to comment.