Skip to content

fix clobbered or lateout registers overlapping with input registers #628

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion libgccjit.version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
e607be166673a8de9fc07f6f02c60426e556c5f2
d6f5a708104a98199ac0f01a3b6b279a0f7c66d3
74 changes: 48 additions & 26 deletions src/asm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ use crate::type_of::LayoutGccExt;
//
// 3. Clobbers. GCC has a separate list of clobbers, and clobbers don't have indexes.
// Contrary, Rust expresses clobbers through "out" operands that aren't tied to
// a variable (`_`), and such "clobbers" do have index.
// a variable (`_`), and such "clobbers" do have index. Input operands cannot also
// be clobbered.
//
// 4. Furthermore, GCC Extended Asm does not support explicit register constraints
// (like `out("eax")`) directly, offering so-called "local register variables"
Expand Down Expand Up @@ -161,6 +162,16 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
// Also, we don't emit any asm operands immediately; we save them to
// the one of the buffers to be emitted later.

let mut input_registers = vec![];

for op in rust_operands {
if let InlineAsmOperandRef::In { reg, .. } = *op {
if let ConstraintOrRegister::Register(reg_name) = reg_to_gcc(reg) {
input_registers.push(reg_name);
}
}
}

// 1. Normal variables (and saving operands to buffers).
for (rust_idx, op) in rust_operands.iter().enumerate() {
match *op {
Expand All @@ -183,25 +194,39 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
continue;
}
(Register(reg_name), None) => {
// `clobber_abi` can add lots of clobbers that are not supported by the target,
// such as AVX-512 registers, so we just ignore unsupported registers
let is_target_supported =
reg.reg_class().supported_types(asm_arch, true).iter().any(
|&(_, feature)| {
if let Some(feature) = feature {
self.tcx
.asm_target_features(instance.def_id())
.contains(&feature)
} else {
true // Register class is unconditionally supported
}
},
);

if is_target_supported && !clobbers.contains(&reg_name) {
clobbers.push(reg_name);
if input_registers.contains(&reg_name) {
// the `clobber_abi` operand is converted into a series of
// `lateout("reg") _` operands. Of course, a user could also
// explicitly define such an output operand.
//
// GCC does not allow input registers to be clobbered, so if this out register
// is also used as an in register, do not add it to the clobbers list.
// it will be treated as a lateout register with `out_place: None`
if !late {
bug!("input registers can only be used as lateout regisers");
}
("r", dummy_output_type(self.cx, reg.reg_class()))
} else {
// `clobber_abi` can add lots of clobbers that are not supported by the target,
// such as AVX-512 registers, so we just ignore unsupported registers
let is_target_supported =
reg.reg_class().supported_types(asm_arch, true).iter().any(
|&(_, feature)| {
if let Some(feature) = feature {
self.tcx
.asm_target_features(instance.def_id())
.contains(&feature)
} else {
true // Register class is unconditionally supported
}
},
);

if is_target_supported && !clobbers.contains(&reg_name) {
clobbers.push(reg_name);
}
continue;
}
continue;
}
};

Expand Down Expand Up @@ -230,13 +255,10 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
}

InlineAsmOperandRef::InOut { reg, late, in_value, out_place } => {
let constraint =
if let ConstraintOrRegister::Constraint(constraint) = reg_to_gcc(reg) {
constraint
} else {
// left for the next pass
continue;
};
let ConstraintOrRegister::Constraint(constraint) = reg_to_gcc(reg) else {
// left for the next pass
continue;
};

// Rustc frontend guarantees that input and output types are "compatible",
// so we can just use input var's type for the output variable.
Expand Down
1 change: 0 additions & 1 deletion tests/failing-ui-tests.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
tests/ui/allocator/no_std-alloc-error-handler-custom.rs
tests/ui/allocator/no_std-alloc-error-handler-default.rs
tests/ui/asm/may_unwind.rs
tests/ui/asm/x86_64/multiple-clobber-abi.rs
tests/ui/functions-closures/parallel-codegen-closures.rs
tests/ui/linkage-attr/linkage1.rs
tests/ui/lto/dylib-works.rs
Expand Down
31 changes: 31 additions & 0 deletions tests/run/asm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,37 @@ fn asm() {
mem_cpy(array2.as_mut_ptr(), array1.as_ptr(), 3);
}
assert_eq!(array1, array2);

// in and clobber registers cannot overlap. This tests that the lateout register without an
// output place (indicated by the `_`) is not added to the list of clobbered registers
let x = 8;
let y: i32;
unsafe {
asm!(
"mov rax, rdi",
in("rdi") x,
lateout("rdi") _,
out("rax") y,
);
}
assert_eq!((x, y), (8, 8));

// sysv64 is the default calling convention on unix systems. The rdi register is
// used to pass arguments in the sysv64 calling convention, so this register will be clobbered
#[cfg(unix)]
{
let x = 16;
let y: i32;
unsafe {
asm!(
"mov rax, rdi",
in("rdi") x,
out("rax") y,
clobber_abi("sysv64"),
);
}
assert_eq!((x, y), (16, 16));
}
}

#[cfg(not(target_arch = "x86_64"))]
Expand Down