Skip to content

Commit 5dcb678

Browse files
committed
Auto merge of rust-lang#122917 - saethlin:atomicptr-to-int, r=nikic
Add the missing inttoptr when we ptrtoint in ptr atomics Ralf noticed this here: rust-lang#122220 (comment) Our previous codegen forgot to add the cast back to integer type. The code compiles anyway, because of course all locals are in-memory to start with, so previous codegen would do the integer atomic, store the integer to a local, then load a pointer from that local. Which is definitely _not_ what we wanted: That's an integer-to-pointer transmute, so all pointers returned by these `AtomicPtr` methods didn't have provenance. Yikes. Here's the IR for `AtomicPtr::fetch_byte_add` on 1.76: https://godbolt.org/z/8qTEjeraY ```llvm define noundef ptr `@atomicptr_fetch_byte_add(ptr` noundef nonnull align 8 %a, i64 noundef %v) unnamed_addr #0 !dbg !7 { start: %0 = alloca ptr, align 8, !dbg !12 %val = inttoptr i64 %v to ptr, !dbg !12 call void `@llvm.lifetime.start.p0(i64` 8, ptr %0), !dbg !28 %1 = ptrtoint ptr %val to i64, !dbg !28 %2 = atomicrmw add ptr %a, i64 %1 monotonic, align 8, !dbg !28 store i64 %2, ptr %0, align 8, !dbg !28 %self = load ptr, ptr %0, align 8, !dbg !28 call void `@llvm.lifetime.end.p0(i64` 8, ptr %0), !dbg !28 ret ptr %self, !dbg !33 } ``` r? `@RalfJung` cc `@nikic`
2 parents 85b884b + 6b794f6 commit 5dcb678

File tree

2 files changed

+46
-4
lines changed

2 files changed

+46
-4
lines changed

compiler/rustc_codegen_llvm/src/builder.rs

+8-4
Original file line numberDiff line numberDiff line change
@@ -1149,12 +1149,12 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
11491149
order: rustc_codegen_ssa::common::AtomicOrdering,
11501150
) -> &'ll Value {
11511151
// The only RMW operation that LLVM supports on pointers is compare-exchange.
1152-
if self.val_ty(src) == self.type_ptr()
1153-
&& op != rustc_codegen_ssa::common::AtomicRmwBinOp::AtomicXchg
1154-
{
1152+
let requires_cast_to_int = self.val_ty(src) == self.type_ptr()
1153+
&& op != rustc_codegen_ssa::common::AtomicRmwBinOp::AtomicXchg;
1154+
if requires_cast_to_int {
11551155
src = self.ptrtoint(src, self.type_isize());
11561156
}
1157-
unsafe {
1157+
let mut res = unsafe {
11581158
llvm::LLVMBuildAtomicRMW(
11591159
self.llbuilder,
11601160
AtomicRmwBinOp::from_generic(op),
@@ -1163,7 +1163,11 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
11631163
AtomicOrdering::from_generic(order),
11641164
llvm::False, // SingleThreaded
11651165
)
1166+
};
1167+
if requires_cast_to_int {
1168+
res = self.inttoptr(res, self.type_ptr());
11661169
}
1170+
res
11671171
}
11681172

11691173
fn atomic_fence(

tests/codegen/atomicptr.rs

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// LLVM does not support some atomic RMW operations on pointers, so inside codegen we lower those
2+
// to integer atomics, surrounded by casts to and from integer type.
3+
// This test ensures that we do the round-trip correctly for AtomicPtr::fetch_byte_add, and also
4+
// ensures that we do not have such a round-trip for AtomicPtr::swap, because LLVM supports pointer
5+
// arguments to `atomicrmw xchg`.
6+
7+
//@ compile-flags: -O -Cno-prepopulate-passes
8+
#![crate_type = "lib"]
9+
10+
#![feature(strict_provenance)]
11+
#![feature(strict_provenance_atomic_ptr)]
12+
13+
use std::sync::atomic::AtomicPtr;
14+
use std::sync::atomic::Ordering::Relaxed;
15+
use std::ptr::without_provenance_mut;
16+
17+
// Portability hack so that we can say [[USIZE]] instead of i64/i32/i16 for usize.
18+
// CHECK: @helper([[USIZE:i[0-9]+]] noundef %_1)
19+
#[no_mangle]
20+
pub fn helper(_: usize) {}
21+
22+
// CHECK-LABEL: @atomicptr_fetch_byte_add
23+
#[no_mangle]
24+
pub fn atomicptr_fetch_byte_add(a: &AtomicPtr<u8>, v: usize) -> *mut u8 {
25+
// CHECK: %[[INTPTR:.*]] = ptrtoint ptr %{{.*}} to [[USIZE]]
26+
// CHECK-NEXT: %[[RET:.*]] = atomicrmw add ptr %{{.*}}, [[USIZE]] %[[INTPTR]]
27+
// CHECK-NEXT: inttoptr [[USIZE]] %[[RET]] to ptr
28+
a.fetch_byte_add(v, Relaxed)
29+
}
30+
31+
// CHECK-LABEL: @atomicptr_swap
32+
#[no_mangle]
33+
pub fn atomicptr_swap(a: &AtomicPtr<u8>, ptr: *mut u8) -> *mut u8 {
34+
// CHECK-NOT: ptrtoint
35+
// CHECK: atomicrmw xchg ptr %{{.*}}, ptr %{{.*}} monotonic
36+
// CHECK-NOT: inttoptr
37+
a.swap(ptr, Relaxed)
38+
}

0 commit comments

Comments
 (0)