From f16f64b15a51789984d9014e92e86fb3a2fe8ce0 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 24 Feb 2025 14:33:55 +0000 Subject: Remove inherent function that has a trait method duplicate of a commonly imported trait --- compiler/rustc_codegen_llvm/src/asm.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src/asm.rs') diff --git a/compiler/rustc_codegen_llvm/src/asm.rs b/compiler/rustc_codegen_llvm/src/asm.rs index be5673eddf9..5fc99ced993 100644 --- a/compiler/rustc_codegen_llvm/src/asm.rs +++ b/compiler/rustc_codegen_llvm/src/asm.rs @@ -1,6 +1,5 @@ use std::assert_matches::assert_matches; -use libc::{c_char, c_uint}; use rustc_abi::{BackendRepr, Float, Integer, Primitive, Scalar}; use rustc_ast::{InlineAsmOptions, InlineAsmTemplatePiece}; use rustc_codegen_ssa::mir::operand::OperandValue; @@ -512,11 +511,7 @@ pub(crate) fn inline_asm_call<'ll>( // Store mark in a metadata node so we can map LLVM errors // back to source locations. See #17552. let key = "srcloc"; - let kind = llvm::LLVMGetMDKindIDInContext( - bx.llcx, - key.as_ptr().cast::(), - key.len() as c_uint, - ); + let kind = bx.get_md_kind_id(key); // `srcloc` contains one 64-bit integer for each line of assembly code, // where the lower 32 bits hold the lo byte position and the upper 32 bits -- cgit 1.4.1-3-g733a5 From 3565603d2522cb96f52b74dba3d64f5869a5ad4c Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 24 Feb 2025 14:45:16 +0000 Subject: Use a safe wrapper around an LLVM FFI function --- compiler/rustc_codegen_llvm/src/asm.rs | 2 +- compiler/rustc_codegen_llvm/src/consts.rs | 2 +- compiler/rustc_codegen_llvm/src/context.rs | 11 ++++++++--- compiler/rustc_codegen_llvm/src/intrinsic.rs | 4 ++-- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 2 +- 5 files changed, 13 insertions(+), 8 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src/asm.rs') diff --git a/compiler/rustc_codegen_llvm/src/asm.rs b/compiler/rustc_codegen_llvm/src/asm.rs index 5fc99ced993..6f344e1ea2e 100644 --- a/compiler/rustc_codegen_llvm/src/asm.rs +++ b/compiler/rustc_codegen_llvm/src/asm.rs @@ -533,7 +533,7 @@ pub(crate) fn inline_asm_call<'ll>( )) })); let md = llvm::LLVMMDNodeInContext2(bx.llcx, srcloc.as_ptr(), srcloc.len()); - let md = llvm::LLVMMetadataAsValue(&bx.llcx, md); + let md = bx.get_metadata_value(md); llvm::LLVMSetMetadata(call, kind, md); Some(call) diff --git a/compiler/rustc_codegen_llvm/src/consts.rs b/compiler/rustc_codegen_llvm/src/consts.rs index 330e8a8f406..0dec0d869b0 100644 --- a/compiler/rustc_codegen_llvm/src/consts.rs +++ b/compiler/rustc_codegen_llvm/src/consts.rs @@ -490,7 +490,7 @@ impl<'ll> CodegenCx<'ll, '_> { llvm::LLVMMDStringInContext2(self.llcx, bytes.as_c_char_ptr(), bytes.len()); let data = [section, alloc]; let meta = llvm::LLVMMDNodeInContext2(self.llcx, data.as_ptr(), data.len()); - let val = llvm::LLVMMetadataAsValue(self.llcx, meta); + let val = self.get_metadata_value(meta); llvm::LLVMAddNamedMetadataOperand( self.llmod, c"wasm.custom_sections".as_ptr(), diff --git a/compiler/rustc_codegen_llvm/src/context.rs b/compiler/rustc_codegen_llvm/src/context.rs index a4e88e5a23f..c8a55d5c03a 100644 --- a/compiler/rustc_codegen_llvm/src/context.rs +++ b/compiler/rustc_codegen_llvm/src/context.rs @@ -656,7 +656,7 @@ impl<'ll> SimpleCx<'ll> { impl<'ll, CX: Borrow>> GenericCx<'ll, CX> { pub(crate) fn get_metadata_value(&self, metadata: &'ll Metadata) -> &'ll Value { - unsafe { llvm::LLVMMetadataAsValue(self.llcx(), metadata) } + llvm::LLVMMetadataAsValue(self.llcx(), metadata) } pub(crate) fn get_function(&self, name: &str) -> Option<&'ll Value> { @@ -1225,9 +1225,14 @@ impl CodegenCx<'_, '_> { impl<'ll, CX: Borrow>> GenericCx<'ll, CX> { /// A wrapper for [`llvm::LLVMSetMetadata`], but it takes `Metadata` as a parameter instead of `Value`. - pub(crate) fn set_metadata<'a>(&self, val: &'a Value, kind_id: MetadataType, md: &'a Metadata) { + pub(crate) fn set_metadata<'a>( + &self, + val: &'a Value, + kind_id: MetadataType, + md: &'ll Metadata, + ) { + let node = self.get_metadata_value(md); unsafe { - let node = llvm::LLVMMetadataAsValue(self.llcx(), md); llvm::LLVMSetMetadata(val, kind_id as c_uint, node); } } diff --git a/compiler/rustc_codegen_llvm/src/intrinsic.rs b/compiler/rustc_codegen_llvm/src/intrinsic.rs index dfbb5bc1731..5e9f7f26f48 100644 --- a/compiler/rustc_codegen_llvm/src/intrinsic.rs +++ b/compiler/rustc_codegen_llvm/src/intrinsic.rs @@ -649,7 +649,7 @@ impl<'ll, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'_, 'll, 'tcx> { fn type_test(&mut self, pointer: Self::Value, typeid: Self::Metadata) -> Self::Value { // Test the called operand using llvm.type.test intrinsic. The LowerTypeTests link-time // optimization pass replaces calls to this intrinsic with code to test type membership. - let typeid = unsafe { llvm::LLVMMetadataAsValue(&self.llcx, typeid) }; + let typeid = self.get_metadata_value(typeid); self.call_intrinsic("llvm.type.test", &[pointer, typeid]) } @@ -659,7 +659,7 @@ impl<'ll, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'_, 'll, 'tcx> { vtable_byte_offset: u64, typeid: &'ll Metadata, ) -> Self::Value { - let typeid = unsafe { llvm::LLVMMetadataAsValue(&self.llcx, typeid) }; + let typeid = self.get_metadata_value(typeid); let vtable_byte_offset = self.const_i32(vtable_byte_offset as i32); let type_checked_load = self.call_intrinsic("llvm.type.checked.load", &[llvtable, vtable_byte_offset, typeid]); diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 2738fee64cf..7dc50eb6849 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -1680,7 +1680,7 @@ unsafe extern "C" { Packed: Bool, ); - pub(crate) fn LLVMMetadataAsValue<'a>(C: &'a Context, MD: &'a Metadata) -> &'a Value; + pub(crate) safe fn LLVMMetadataAsValue<'a>(C: &'a Context, MD: &'a Metadata) -> &'a Value; pub(crate) fn LLVMSetUnnamedAddress(Global: &Value, UnnamedAddr: UnnamedAddr); -- cgit 1.4.1-3-g733a5 From 553828c6f48adcf3f2443bf1972909db518e3c89 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 24 Feb 2025 14:38:55 +0000 Subject: Mark more LLVM FFI as safe --- compiler/rustc_codegen_llvm/src/asm.rs | 93 +++++++++++----------- compiler/rustc_codegen_llvm/src/builder.rs | 4 +- compiler/rustc_codegen_llvm/src/context.rs | 10 +-- compiler/rustc_codegen_llvm/src/llvm/enzyme_ffi.rs | 3 +- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 16 +++- 5 files changed, 68 insertions(+), 58 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src/asm.rs') diff --git a/compiler/rustc_codegen_llvm/src/asm.rs b/compiler/rustc_codegen_llvm/src/asm.rs index 6f344e1ea2e..6e931aa0bc7 100644 --- a/compiler/rustc_codegen_llvm/src/asm.rs +++ b/compiler/rustc_codegen_llvm/src/asm.rs @@ -482,12 +482,13 @@ pub(crate) fn inline_asm_call<'ll>( debug!("Asm Output Type: {:?}", output); let fty = bx.cx.type_func(&argtys, output); - unsafe { - // Ask LLVM to verify that the constraints are well-formed. - let constraints_ok = llvm::LLVMRustInlineAsmVerify(fty, cons.as_c_char_ptr(), cons.len()); - debug!("constraint verification result: {:?}", constraints_ok); - if constraints_ok { - let v = llvm::LLVMRustInlineAsm( + // Ask LLVM to verify that the constraints are well-formed. + let constraints_ok = + unsafe { llvm::LLVMRustInlineAsmVerify(fty, cons.as_c_char_ptr(), cons.len()) }; + debug!("constraint verification result: {:?}", constraints_ok); + if constraints_ok { + let v = unsafe { + llvm::LLVMRustInlineAsm( fty, asm.as_c_char_ptr(), asm.len(), @@ -497,50 +498,50 @@ pub(crate) fn inline_asm_call<'ll>( alignstack, dia, can_throw, - ); - - let call = if !labels.is_empty() { - assert!(catch_funclet.is_none()); - bx.callbr(fty, None, None, v, inputs, dest.unwrap(), labels, None, None) - } else if let Some((catch, funclet)) = catch_funclet { - bx.invoke(fty, None, None, v, inputs, dest.unwrap(), catch, funclet, None) - } else { - bx.call(fty, None, None, v, inputs, None, None) - }; + ) + }; - // Store mark in a metadata node so we can map LLVM errors - // back to source locations. See #17552. - let key = "srcloc"; - let kind = bx.get_md_kind_id(key); + let call = if !labels.is_empty() { + assert!(catch_funclet.is_none()); + bx.callbr(fty, None, None, v, inputs, dest.unwrap(), labels, None, None) + } else if let Some((catch, funclet)) = catch_funclet { + bx.invoke(fty, None, None, v, inputs, dest.unwrap(), catch, funclet, None) + } else { + bx.call(fty, None, None, v, inputs, None, None) + }; - // `srcloc` contains one 64-bit integer for each line of assembly code, - // where the lower 32 bits hold the lo byte position and the upper 32 bits - // hold the hi byte position. - let mut srcloc = vec![]; - if dia == llvm::AsmDialect::Intel && line_spans.len() > 1 { - // LLVM inserts an extra line to add the ".intel_syntax", so add - // a dummy srcloc entry for it. - // - // Don't do this if we only have 1 line span since that may be - // due to the asm template string coming from a macro. LLVM will - // default to the first srcloc for lines that don't have an - // associated srcloc. - srcloc.push(llvm::LLVMValueAsMetadata(bx.const_u64(0))); - } - srcloc.extend(line_spans.iter().map(|span| { - llvm::LLVMValueAsMetadata(bx.const_u64( - u64::from(span.lo().to_u32()) | (u64::from(span.hi().to_u32()) << 32), - )) - })); - let md = llvm::LLVMMDNodeInContext2(bx.llcx, srcloc.as_ptr(), srcloc.len()); - let md = bx.get_metadata_value(md); - llvm::LLVMSetMetadata(call, kind, md); + // Store mark in a metadata node so we can map LLVM errors + // back to source locations. See #17552. + let key = "srcloc"; + let kind = bx.get_md_kind_id(key); - Some(call) - } else { - // LLVM has detected an issue with our constraints, bail out - None + // `srcloc` contains one 64-bit integer for each line of assembly code, + // where the lower 32 bits hold the lo byte position and the upper 32 bits + // hold the hi byte position. + let mut srcloc = vec![]; + if dia == llvm::AsmDialect::Intel && line_spans.len() > 1 { + // LLVM inserts an extra line to add the ".intel_syntax", so add + // a dummy srcloc entry for it. + // + // Don't do this if we only have 1 line span since that may be + // due to the asm template string coming from a macro. LLVM will + // default to the first srcloc for lines that don't have an + // associated srcloc. + srcloc.push(llvm::LLVMValueAsMetadata(bx.const_u64(0))); } + srcloc.extend(line_spans.iter().map(|span| { + llvm::LLVMValueAsMetadata( + bx.const_u64(u64::from(span.lo().to_u32()) | (u64::from(span.hi().to_u32()) << 32)), + ) + })); + let md = unsafe { llvm::LLVMMDNodeInContext2(bx.llcx, srcloc.as_ptr(), srcloc.len()) }; + let md = bx.get_metadata_value(md); + llvm::LLVMSetMetadata(call, kind, md); + + Some(call) + } else { + // LLVM has detected an issue with our constraints, bail out + None } } diff --git a/compiler/rustc_codegen_llvm/src/builder.rs b/compiler/rustc_codegen_llvm/src/builder.rs index 17579ebe72c..7e8b2467989 100644 --- a/compiler/rustc_codegen_llvm/src/builder.rs +++ b/compiler/rustc_codegen_llvm/src/builder.rs @@ -311,8 +311,8 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { // This function handles switch instructions with more than 2 targets and it needs to // emit branch weights metadata instead of using the intrinsic. // The values 1 and 2000 are the same as the values used by the `llvm.expect` intrinsic. - let cold_weight = unsafe { llvm::LLVMValueAsMetadata(self.cx.const_u32(1)) }; - let hot_weight = unsafe { llvm::LLVMValueAsMetadata(self.cx.const_u32(2000)) }; + let cold_weight = llvm::LLVMValueAsMetadata(self.cx.const_u32(1)); + let hot_weight = llvm::LLVMValueAsMetadata(self.cx.const_u32(2000)); let weight = |is_cold: bool| -> &Metadata { if is_cold { cold_weight } else { hot_weight } }; diff --git a/compiler/rustc_codegen_llvm/src/context.rs b/compiler/rustc_codegen_llvm/src/context.rs index c8a55d5c03a..ecef54d3165 100644 --- a/compiler/rustc_codegen_llvm/src/context.rs +++ b/compiler/rustc_codegen_llvm/src/context.rs @@ -34,7 +34,7 @@ use crate::back::write::to_llvm_code_model; use crate::callee::get_fn; use crate::common::AsCCharPtr; use crate::debuginfo::metadata::apply_vcall_visibility_metadata; -use crate::llvm::{Metadata, MetadataType}; +use crate::llvm::Metadata; use crate::type_::Type; use crate::value::Value; use crate::{attributes, coverageinfo, debuginfo, llvm, llvm_util}; @@ -664,7 +664,7 @@ impl<'ll, CX: Borrow>> GenericCx<'ll, CX> { unsafe { llvm::LLVMGetNamedFunction((**self).borrow().llmod, name.as_ptr()) } } - pub(crate) fn get_md_kind_id(&self, name: &str) -> u32 { + pub(crate) fn get_md_kind_id(&self, name: &str) -> llvm::MetadataKindId { unsafe { llvm::LLVMGetMDKindIDInContext( self.llcx(), @@ -1228,13 +1228,11 @@ impl<'ll, CX: Borrow>> GenericCx<'ll, CX> { pub(crate) fn set_metadata<'a>( &self, val: &'a Value, - kind_id: MetadataType, + kind_id: impl Into, md: &'ll Metadata, ) { let node = self.get_metadata_value(md); - unsafe { - llvm::LLVMSetMetadata(val, kind_id as c_uint, node); - } + llvm::LLVMSetMetadata(val, kind_id.into(), node); } } diff --git a/compiler/rustc_codegen_llvm/src/llvm/enzyme_ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/enzyme_ffi.rs index 3c2c6964a3d..ac8d4e57283 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/enzyme_ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/enzyme_ffi.rs @@ -3,13 +3,14 @@ use libc::{c_char, c_uint}; +use super::MetadataKindId; use super::ffi::{BasicBlock, Metadata, Module, Type, Value}; use crate::llvm::Bool; #[link(name = "llvm-wrapper", kind = "static")] unsafe extern "C" { // Enzyme - pub(crate) fn LLVMRustHasMetadata(I: &Value, KindID: c_uint) -> bool; + pub(crate) safe fn LLVMRustHasMetadata(I: &Value, KindID: MetadataKindId) -> bool; pub(crate) fn LLVMRustEraseInstUntilInclusive(BB: &BasicBlock, I: &Value); pub(crate) fn LLVMRustGetLastInstruction<'a>(BB: &BasicBlock) -> Option<&'a Value>; pub(crate) fn LLVMRustDIGetInstMetadata(I: &Value) -> Option<&Metadata>; diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 7dc50eb6849..6801f21aa77 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -976,6 +976,16 @@ pub type SelfProfileAfterPassCallback = unsafe extern "C" fn(*mut c_void); pub type GetSymbolsCallback = unsafe extern "C" fn(*mut c_void, *const c_char) -> *mut c_void; pub type GetSymbolsErrorCallback = unsafe extern "C" fn(*const c_char) -> *mut c_void; +#[derive(Copy, Clone)] +#[repr(transparent)] +pub struct MetadataKindId(c_uint); + +impl From for MetadataKindId { + fn from(value: MetadataType) -> Self { + Self(value as c_uint) + } +} + unsafe extern "C" { // Create and destroy contexts. pub(crate) fn LLVMContextDispose(C: &'static mut Context); @@ -983,7 +993,7 @@ unsafe extern "C" { C: &Context, Name: *const c_char, SLen: c_uint, - ) -> c_uint; + ) -> MetadataKindId; // Create modules. pub(crate) fn LLVMModuleCreateWithNameInContext( @@ -1051,9 +1061,9 @@ unsafe extern "C" { pub(crate) fn LLVMGetValueName2(Val: &Value, Length: *mut size_t) -> *const c_char; pub(crate) fn LLVMSetValueName2(Val: &Value, Name: *const c_char, NameLen: size_t); pub(crate) fn LLVMReplaceAllUsesWith<'a>(OldVal: &'a Value, NewVal: &'a Value); - pub(crate) fn LLVMSetMetadata<'a>(Val: &'a Value, KindID: c_uint, Node: &'a Value); + pub(crate) safe fn LLVMSetMetadata<'a>(Val: &'a Value, KindID: MetadataKindId, Node: &'a Value); pub(crate) fn LLVMGlobalSetMetadata<'a>(Val: &'a Value, KindID: c_uint, Metadata: &'a Metadata); - pub(crate) fn LLVMValueAsMetadata(Node: &Value) -> &Metadata; + pub(crate) safe fn LLVMValueAsMetadata(Node: &Value) -> &Metadata; // Operations on constants of any type pub(crate) fn LLVMConstNull(Ty: &Type) -> &Value; -- cgit 1.4.1-3-g733a5