From 04bb68ac869bc4d55bcb260398470b0c5eea4a40 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Wed, 9 Jul 2025 20:50:17 -0700 Subject: [PATCH 1/2] compiler: recomment `needs_fn_once_adapter_shim` This requires digging up ffee9566bbd7728e6411e6094105d6905373255d and reading the comments there to understand that the callee in resolve_closure previously directly handled a function pointer value. --- compiler/rustc_middle/src/ty/instance.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_middle/src/ty/instance.rs b/compiler/rustc_middle/src/ty/instance.rs index 21b7500e46fe0..d5767ca3786e8 100644 --- a/compiler/rustc_middle/src/ty/instance.rs +++ b/compiler/rustc_middle/src/ty/instance.rs @@ -991,18 +991,16 @@ fn needs_fn_once_adapter_shim( Ok(false) } (ty::ClosureKind::Fn, ty::ClosureKind::FnMut) => { - // The closure fn `llfn` is a `fn(&self, ...)`. We want a - // `fn(&mut self, ...)`. In fact, at codegen time, these are - // basically the same thing, so we can just return llfn. + // The closure fn is a `fn(&self, ...)`, but we want a `fn(&mut self, ...)`. + // At codegen time, these are basically the same, so we can just return the closure. Ok(false) } (ty::ClosureKind::Fn | ty::ClosureKind::FnMut, ty::ClosureKind::FnOnce) => { - // The closure fn `llfn` is a `fn(&self, ...)` or `fn(&mut - // self, ...)`. We want a `fn(self, ...)`. We can produce - // this by doing something like: + // The closure fn is a `fn(&self, ...)` or `fn(&mut self, ...)`, but + // we want a `fn(self, ...)`. We can produce this by doing something like: // - // fn call_once(self, ...) { call_mut(&self, ...) } - // fn call_once(mut self, ...) { call_mut(&mut self, ...) } + // fn call_once(self, ...) { Fn::call(&self, ...) } + // fn call_once(mut self, ...) { FnMut::call_mut(&mut self, ...) } // // These are both the same at codegen time. Ok(true) From 39f7707feac629defbd5a0cc9e2a1f7c18e843ea Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Wed, 9 Jul 2025 20:53:13 -0700 Subject: [PATCH 2/2] compiler: comment on some call-related codegen fn in cg_ssa Partially documents the situation due to LLVM CFI. --- .../rustc_codegen_ssa/src/traits/builder.rs | 23 ++++++++++++++++++- .../rustc_codegen_ssa/src/traits/intrinsic.rs | 8 ++++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/traits/builder.rs b/compiler/rustc_codegen_ssa/src/traits/builder.rs index 9d367748c2a8a..829b8899398ab 100644 --- a/compiler/rustc_codegen_ssa/src/traits/builder.rs +++ b/compiler/rustc_codegen_ssa/src/traits/builder.rs @@ -555,12 +555,33 @@ pub trait BuilderMethods<'a, 'tcx>: /// Called for `StorageDead` fn lifetime_end(&mut self, ptr: Self::Value, size: Size); + /// "Finally codegen the call" + /// + /// ## Arguments + /// + /// The `fn_attrs`, `fn_abi`, and `instance` arguments are Options because they are advisory. + /// They relate to optional codegen enhancements like LLVM CFI, and do not affect ABI per se. + /// Any ABI-related transformations should be handled by different, earlier stages of codegen. + /// For instance, in the caller of `BuilderMethods::call`. + /// + /// This means that a codegen backend which disregards `fn_attrs`, `fn_abi`, and `instance` + /// should still do correct codegen, and code should not be miscompiled if they are omitted. + /// It is not a miscompilation in this sense if it fails to run under CFI, other sanitizers, or + /// in the context of other compiler-enhanced security features. + /// + /// The typical case that they are None is during the codegen of intrinsics and lang-items, + /// as those are "fake functions" with only a trivial ABI if any, et cetera. + /// + /// ## Return + /// + /// Must return the value the function will return so it can be written to the destination, + /// assuming the function does not explicitly pass the destination as a pointer in `args`. fn call( &mut self, llty: Self::Type, fn_attrs: Option<&CodegenFnAttrs>, fn_abi: Option<&FnAbi<'tcx, Ty<'tcx>>>, - llfn: Self::Value, + fn_val: Self::Value, args: &[Self::Value], funclet: Option<&Self::Funclet>, instance: Option>, diff --git a/compiler/rustc_codegen_ssa/src/traits/intrinsic.rs b/compiler/rustc_codegen_ssa/src/traits/intrinsic.rs index 7d0c6be4c650d..c5ecf43046c74 100644 --- a/compiler/rustc_codegen_ssa/src/traits/intrinsic.rs +++ b/compiler/rustc_codegen_ssa/src/traits/intrinsic.rs @@ -6,16 +6,22 @@ use crate::mir::operand::OperandRef; use crate::mir::place::PlaceRef; pub trait IntrinsicCallBuilderMethods<'tcx>: BackendTypes { + /// Higher-level interface to emitting calls to intrinsics + /// /// Remember to add all intrinsics here, in `compiler/rustc_hir_analysis/src/check/mod.rs`, /// and in `library/core/src/intrinsics.rs`; if you need access to any LLVM intrinsics, /// add them to `compiler/rustc_codegen_llvm/src/context.rs`. /// Returns `Err` if another instance should be called instead. This is used to invoke /// intrinsic default bodies in case an intrinsic is not implemented by the backend. + /// + /// NOTE: allowed to call [`BuilderMethods::call`] + /// + /// [`BuilderMethods::call`]: super::builder::BuilderMethods::call fn codegen_intrinsic_call( &mut self, instance: ty::Instance<'tcx>, args: &[OperandRef<'tcx, Self::Value>], - result: PlaceRef<'tcx, Self::Value>, + result_dest: PlaceRef<'tcx, Self::Value>, span: Span, ) -> Result<(), ty::Instance<'tcx>>;