From 4bd84b23a8537314132e98b9fb2c3fea2cb57496 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Mon, 28 Oct 2024 18:52:39 +1100 Subject: Use a type-safe helper to cast `&str` and `&[u8]` to `*const c_char` --- compiler/rustc_codegen_llvm/src/llvm/mod.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src/llvm/mod.rs') diff --git a/compiler/rustc_codegen_llvm/src/llvm/mod.rs b/compiler/rustc_codegen_llvm/src/llvm/mod.rs index 6aac2eea81d..cabe6c031d3 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/mod.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/mod.rs @@ -17,13 +17,13 @@ pub use self::IntPredicate::*; pub use self::Linkage::*; pub use self::MetadataType::*; pub use self::RealPredicate::*; +pub use self::ffi::*; +use crate::common::AsCCharPtr; pub mod archive_ro; pub mod diagnostic; mod ffi; -pub use self::ffi::*; - impl LLVMRustResult { pub fn into_result(self) -> Result<(), ()> { match self { @@ -53,9 +53,9 @@ pub fn CreateAttrStringValue<'ll>(llcx: &'ll Context, attr: &str, value: &str) - unsafe { LLVMCreateStringAttribute( llcx, - attr.as_ptr().cast(), + attr.as_c_char_ptr(), attr.len().try_into().unwrap(), - value.as_ptr().cast(), + value.as_c_char_ptr(), value.len().try_into().unwrap(), ) } @@ -65,7 +65,7 @@ pub fn CreateAttrString<'ll>(llcx: &'ll Context, attr: &str) -> &'ll Attribute { unsafe { LLVMCreateStringAttribute( llcx, - attr.as_ptr().cast(), + attr.as_c_char_ptr(), attr.len().try_into().unwrap(), std::ptr::null(), 0, @@ -294,7 +294,7 @@ pub fn get_value_name(value: &Value) -> &[u8] { /// Safe wrapper for `LLVMSetValueName2` from a byte slice pub fn set_value_name(value: &Value, name: &[u8]) { unsafe { - let data = name.as_ptr().cast(); + let data = name.as_c_char_ptr(); LLVMSetValueName2(value, data, name.len()); } } -- cgit 1.4.1-3-g733a5 From 8d2ed4f0f3f79c402d2ebe585e0904baf4f8d634 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 29 Oct 2024 13:38:17 +1100 Subject: Clean up FFI calls for setting module flags - Don't rely on enum values defined by LLVM's C++ API - Use safe wrapper functions instead of direct `unsafe` calls - Consistently pass pointer/length strings instead of C strings --- compiler/rustc_codegen_llvm/src/context.rs | 278 ++++++++++------------- compiler/rustc_codegen_llvm/src/debuginfo/mod.rs | 66 +++--- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 24 +- compiler/rustc_codegen_llvm/src/llvm/mod.rs | 29 +++ compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp | 57 ++++- 5 files changed, 242 insertions(+), 212 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src/llvm/mod.rs') diff --git a/compiler/rustc_codegen_llvm/src/context.rs b/compiler/rustc_codegen_llvm/src/context.rs index 1a2197248b1..f33da42d63e 100644 --- a/compiler/rustc_codegen_llvm/src/context.rs +++ b/compiler/rustc_codegen_llvm/src/context.rs @@ -210,133 +210,111 @@ pub(crate) unsafe fn create_module<'ll>( // If skipping the PLT is enabled, we need to add some module metadata // to ensure intrinsic calls don't use it. if !sess.needs_plt() { - let avoid_plt = c"RtLibUseGOT".as_ptr(); - unsafe { - llvm::LLVMRustAddModuleFlagU32(llmod, llvm::LLVMModFlagBehavior::Warning, avoid_plt, 1); - } + llvm::add_module_flag_u32(llmod, llvm::ModuleFlagMergeBehavior::Warning, "RtLibUseGOT", 1); } // Enable canonical jump tables if CFI is enabled. (See https://reviews.llvm.org/D65629.) if sess.is_sanitizer_cfi_canonical_jump_tables_enabled() && sess.is_sanitizer_cfi_enabled() { - let canonical_jump_tables = c"CFI Canonical Jump Tables".as_ptr(); - unsafe { - llvm::LLVMRustAddModuleFlagU32( - llmod, - llvm::LLVMModFlagBehavior::Override, - canonical_jump_tables, - 1, - ); - } + llvm::add_module_flag_u32( + llmod, + llvm::ModuleFlagMergeBehavior::Override, + "CFI Canonical Jump Tables", + 1, + ); } // If we're normalizing integers with CFI, ensure LLVM generated functions do the same. // See https://github.com/llvm/llvm-project/pull/104826 if sess.is_sanitizer_cfi_normalize_integers_enabled() { - let cfi_normalize_integers = c"cfi-normalize-integers".as_ptr(); - unsafe { - llvm::LLVMRustAddModuleFlagU32( - llmod, - llvm::LLVMModFlagBehavior::Override, - cfi_normalize_integers, - 1, - ); - } + llvm::add_module_flag_u32( + llmod, + llvm::ModuleFlagMergeBehavior::Override, + "cfi-normalize-integers", + 1, + ); } // Enable LTO unit splitting if specified or if CFI is enabled. (See // https://reviews.llvm.org/D53891.) if sess.is_split_lto_unit_enabled() || sess.is_sanitizer_cfi_enabled() { - let enable_split_lto_unit = c"EnableSplitLTOUnit".as_ptr(); - unsafe { - llvm::LLVMRustAddModuleFlagU32( - llmod, - llvm::LLVMModFlagBehavior::Override, - enable_split_lto_unit, - 1, - ); - } + llvm::add_module_flag_u32( + llmod, + llvm::ModuleFlagMergeBehavior::Override, + "EnableSplitLTOUnit", + 1, + ); } // Add "kcfi" module flag if KCFI is enabled. (See https://reviews.llvm.org/D119296.) if sess.is_sanitizer_kcfi_enabled() { - let kcfi = c"kcfi".as_ptr(); - unsafe { - llvm::LLVMRustAddModuleFlagU32(llmod, llvm::LLVMModFlagBehavior::Override, kcfi, 1); - } + llvm::add_module_flag_u32(llmod, llvm::ModuleFlagMergeBehavior::Override, "kcfi", 1); // Add "kcfi-offset" module flag with -Z patchable-function-entry (See // https://reviews.llvm.org/D141172). let pfe = PatchableFunctionEntry::from_config(sess.opts.unstable_opts.patchable_function_entry); if pfe.prefix() > 0 { - let kcfi_offset = c"kcfi-offset".as_ptr(); - unsafe { - llvm::LLVMRustAddModuleFlagU32( - llmod, - llvm::LLVMModFlagBehavior::Override, - kcfi_offset, - pfe.prefix().into(), - ); - } + llvm::add_module_flag_u32( + llmod, + llvm::ModuleFlagMergeBehavior::Override, + "kcfi-offset", + pfe.prefix().into(), + ); } } // Control Flow Guard is currently only supported by the MSVC linker on Windows. if sess.target.is_like_msvc { - unsafe { - match sess.opts.cg.control_flow_guard { - CFGuard::Disabled => {} - CFGuard::NoChecks => { - // Set `cfguard=1` module flag to emit metadata only. - llvm::LLVMRustAddModuleFlagU32( - llmod, - llvm::LLVMModFlagBehavior::Warning, - c"cfguard".as_ptr() as *const _, - 1, - ) - } - CFGuard::Checks => { - // Set `cfguard=2` module flag to emit metadata and checks. - llvm::LLVMRustAddModuleFlagU32( - llmod, - llvm::LLVMModFlagBehavior::Warning, - c"cfguard".as_ptr() as *const _, - 2, - ) - } + match sess.opts.cg.control_flow_guard { + CFGuard::Disabled => {} + CFGuard::NoChecks => { + // Set `cfguard=1` module flag to emit metadata only. + llvm::add_module_flag_u32( + llmod, + llvm::ModuleFlagMergeBehavior::Warning, + "cfguard", + 1, + ); + } + CFGuard::Checks => { + // Set `cfguard=2` module flag to emit metadata and checks. + llvm::add_module_flag_u32( + llmod, + llvm::ModuleFlagMergeBehavior::Warning, + "cfguard", + 2, + ); } } } if let Some(BranchProtection { bti, pac_ret }) = sess.opts.unstable_opts.branch_protection { if sess.target.arch == "aarch64" { - unsafe { - llvm::LLVMRustAddModuleFlagU32( - llmod, - llvm::LLVMModFlagBehavior::Min, - c"branch-target-enforcement".as_ptr(), - bti.into(), - ); - llvm::LLVMRustAddModuleFlagU32( - llmod, - llvm::LLVMModFlagBehavior::Min, - c"sign-return-address".as_ptr(), - pac_ret.is_some().into(), - ); - let pac_opts = pac_ret.unwrap_or(PacRet { leaf: false, key: PAuthKey::A }); - llvm::LLVMRustAddModuleFlagU32( - llmod, - llvm::LLVMModFlagBehavior::Min, - c"sign-return-address-all".as_ptr(), - pac_opts.leaf.into(), - ); - llvm::LLVMRustAddModuleFlagU32( - llmod, - llvm::LLVMModFlagBehavior::Min, - c"sign-return-address-with-bkey".as_ptr(), - u32::from(pac_opts.key == PAuthKey::B), - ); - } + llvm::add_module_flag_u32( + llmod, + llvm::ModuleFlagMergeBehavior::Min, + "branch-target-enforcement", + bti.into(), + ); + llvm::add_module_flag_u32( + llmod, + llvm::ModuleFlagMergeBehavior::Min, + "sign-return-address", + pac_ret.is_some().into(), + ); + let pac_opts = pac_ret.unwrap_or(PacRet { leaf: false, key: PAuthKey::A }); + llvm::add_module_flag_u32( + llmod, + llvm::ModuleFlagMergeBehavior::Min, + "sign-return-address-all", + pac_opts.leaf.into(), + ); + llvm::add_module_flag_u32( + llmod, + llvm::ModuleFlagMergeBehavior::Min, + "sign-return-address-with-bkey", + u32::from(pac_opts.key == PAuthKey::B), + ); } else { bug!( "branch-protection used on non-AArch64 target; \ @@ -347,59 +325,46 @@ pub(crate) unsafe fn create_module<'ll>( // Pass on the control-flow protection flags to LLVM (equivalent to `-fcf-protection` in Clang). if let CFProtection::Branch | CFProtection::Full = sess.opts.unstable_opts.cf_protection { - unsafe { - llvm::LLVMRustAddModuleFlagU32( - llmod, - llvm::LLVMModFlagBehavior::Override, - c"cf-protection-branch".as_ptr(), - 1, - ); - } + llvm::add_module_flag_u32( + llmod, + llvm::ModuleFlagMergeBehavior::Override, + "cf-protection-branch", + 1, + ); } if let CFProtection::Return | CFProtection::Full = sess.opts.unstable_opts.cf_protection { - unsafe { - llvm::LLVMRustAddModuleFlagU32( - llmod, - llvm::LLVMModFlagBehavior::Override, - c"cf-protection-return".as_ptr(), - 1, - ); - } + llvm::add_module_flag_u32( + llmod, + llvm::ModuleFlagMergeBehavior::Override, + "cf-protection-return", + 1, + ); } if sess.opts.unstable_opts.virtual_function_elimination { - unsafe { - llvm::LLVMRustAddModuleFlagU32( - llmod, - llvm::LLVMModFlagBehavior::Error, - c"Virtual Function Elim".as_ptr(), - 1, - ); - } + llvm::add_module_flag_u32( + llmod, + llvm::ModuleFlagMergeBehavior::Error, + "Virtual Function Elim", + 1, + ); } // Set module flag to enable Windows EHCont Guard (/guard:ehcont). if sess.opts.unstable_opts.ehcont_guard { - unsafe { - llvm::LLVMRustAddModuleFlagU32( - llmod, - llvm::LLVMModFlagBehavior::Warning, - c"ehcontguard".as_ptr() as *const _, - 1, - ) - } + llvm::add_module_flag_u32(llmod, llvm::ModuleFlagMergeBehavior::Warning, "ehcontguard", 1); } match sess.opts.unstable_opts.function_return { FunctionReturn::Keep => {} - FunctionReturn::ThunkExtern => unsafe { - llvm::LLVMRustAddModuleFlagU32( + FunctionReturn::ThunkExtern => { + llvm::add_module_flag_u32( llmod, - llvm::LLVMModFlagBehavior::Override, - c"function_return_thunk_extern".as_ptr(), + llvm::ModuleFlagMergeBehavior::Override, + "function_return_thunk_extern", 1, - ) - }, + ); + } } match (sess.opts.unstable_opts.small_data_threshold, sess.target.small_data_threshold_support()) @@ -407,15 +372,12 @@ pub(crate) unsafe fn create_module<'ll>( // Set up the small-data optimization limit for architectures that use // an LLVM module flag to control this. (Some(threshold), SmallDataThresholdSupport::LlvmModuleFlag(flag)) => { - let flag = SmallCStr::new(flag.as_ref()); - unsafe { - llvm::LLVMRustAddModuleFlagU32( - llmod, - llvm::LLVMModFlagBehavior::Error, - flag.as_c_str().as_ptr(), - threshold as u32, - ) - } + llvm::add_module_flag_u32( + llmod, + llvm::ModuleFlagMergeBehavior::Error, + &flag, + threshold as u32, + ); } _ => (), }; @@ -449,33 +411,29 @@ pub(crate) unsafe fn create_module<'ll>( // If llvm_abiname is empty, emit nothing. let llvm_abiname = &sess.target.options.llvm_abiname; if matches!(sess.target.arch.as_ref(), "riscv32" | "riscv64") && !llvm_abiname.is_empty() { - unsafe { - llvm::LLVMRustAddModuleFlagString( - llmod, - llvm::LLVMModFlagBehavior::Error, - c"target-abi".as_ptr(), - llvm_abiname.as_c_char_ptr(), - llvm_abiname.len(), - ); - } + llvm::add_module_flag_str( + llmod, + llvm::ModuleFlagMergeBehavior::Error, + "target-abi", + llvm_abiname, + ); } // Add module flags specified via -Z llvm_module_flag - for (key, value, behavior) in &sess.opts.unstable_opts.llvm_module_flag { - let key = format!("{key}\0"); - let behavior = match behavior.as_str() { - "error" => llvm::LLVMModFlagBehavior::Error, - "warning" => llvm::LLVMModFlagBehavior::Warning, - "require" => llvm::LLVMModFlagBehavior::Require, - "override" => llvm::LLVMModFlagBehavior::Override, - "append" => llvm::LLVMModFlagBehavior::Append, - "appendunique" => llvm::LLVMModFlagBehavior::AppendUnique, - "max" => llvm::LLVMModFlagBehavior::Max, - "min" => llvm::LLVMModFlagBehavior::Min, + for (key, value, merge_behavior) in &sess.opts.unstable_opts.llvm_module_flag { + let merge_behavior = match merge_behavior.as_str() { + "error" => llvm::ModuleFlagMergeBehavior::Error, + "warning" => llvm::ModuleFlagMergeBehavior::Warning, + "require" => llvm::ModuleFlagMergeBehavior::Require, + "override" => llvm::ModuleFlagMergeBehavior::Override, + "append" => llvm::ModuleFlagMergeBehavior::Append, + "appendunique" => llvm::ModuleFlagMergeBehavior::AppendUnique, + "max" => llvm::ModuleFlagMergeBehavior::Max, + "min" => llvm::ModuleFlagMergeBehavior::Min, // We already checked this during option parsing _ => unreachable!(), }; - unsafe { llvm::LLVMRustAddModuleFlagU32(llmod, behavior, key.as_c_char_ptr(), *value) } + llvm::add_module_flag_u32(llmod, merge_behavior, key, *value); } llmod diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs index 85eb1c8509d..72e723aa849 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs @@ -91,45 +91,39 @@ impl<'ll, 'tcx> CodegenUnitDebugContext<'ll, 'tcx> { } pub(crate) fn finalize(&self, sess: &Session) { - unsafe { - llvm::LLVMRustDIBuilderFinalize(self.builder); - - if !sess.target.is_like_msvc { - // Debuginfo generation in LLVM by default uses a higher - // version of dwarf than macOS currently understands. We can - // instruct LLVM to emit an older version of dwarf, however, - // for macOS to understand. For more info see #11352 - // This can be overridden using --llvm-opts -dwarf-version,N. - // Android has the same issue (#22398) - let dwarf_version = sess - .opts - .unstable_opts - .dwarf_version - .unwrap_or(sess.target.default_dwarf_version); - llvm::LLVMRustAddModuleFlagU32( - self.llmod, - llvm::LLVMModFlagBehavior::Warning, - c"Dwarf Version".as_ptr(), - dwarf_version, - ); - } else { - // Indicate that we want CodeView debug information on MSVC - llvm::LLVMRustAddModuleFlagU32( - self.llmod, - llvm::LLVMModFlagBehavior::Warning, - c"CodeView".as_ptr(), - 1, - ) - } - - // Prevent bitcode readers from deleting the debug info. - llvm::LLVMRustAddModuleFlagU32( + unsafe { llvm::LLVMRustDIBuilderFinalize(self.builder) }; + if !sess.target.is_like_msvc { + // Debuginfo generation in LLVM by default uses a higher + // version of dwarf than macOS currently understands. We can + // instruct LLVM to emit an older version of dwarf, however, + // for macOS to understand. For more info see #11352 + // This can be overridden using --llvm-opts -dwarf-version,N. + // Android has the same issue (#22398) + let dwarf_version = + sess.opts.unstable_opts.dwarf_version.unwrap_or(sess.target.default_dwarf_version); + llvm::add_module_flag_u32( self.llmod, - llvm::LLVMModFlagBehavior::Warning, - c"Debug Info Version".as_ptr(), - llvm::LLVMRustDebugMetadataVersion(), + llvm::ModuleFlagMergeBehavior::Warning, + "Dwarf Version", + dwarf_version, + ); + } else { + // Indicate that we want CodeView debug information on MSVC + llvm::add_module_flag_u32( + self.llmod, + llvm::ModuleFlagMergeBehavior::Warning, + "CodeView", + 1, ); } + + // Prevent bitcode readers from deleting the debug info. + llvm::add_module_flag_u32( + self.llmod, + llvm::ModuleFlagMergeBehavior::Warning, + "Debug Info Version", + unsafe { llvm::LLVMRustDebugMetadataVersion() }, + ); } } diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index c204fbe32df..fffbae19c9c 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -85,7 +85,7 @@ pub enum LLVMMachineType { ARM = 0x01c0, } -/// LLVM's Module::ModFlagBehavior, defined in llvm/include/llvm/IR/Module.h. +/// Must match the layout of `LLVMRustModuleFlagMergeBehavior`. /// /// When merging modules (e.g. during LTO), their metadata flags are combined. Conflicts are /// resolved according to the merge behaviors specified here. Flags differing only in merge @@ -93,9 +93,13 @@ pub enum LLVMMachineType { /// /// In order for Rust-C LTO to work, we must specify behaviors compatible with Clang. Notably, /// 'Error' and 'Warning' cannot be mixed for a given flag. +/// +/// There is a stable LLVM-C version of this enum (`LLVMModuleFlagBehavior`), +/// but as of LLVM 19 it does not support all of the enum values in the unstable +/// C++ API. #[derive(Copy, Clone, PartialEq)] #[repr(C)] -pub enum LLVMModFlagBehavior { +pub enum ModuleFlagMergeBehavior { Error = 1, Warning = 2, Require = 3, @@ -1829,17 +1833,19 @@ unsafe extern "C" { /// "compatible" means depends on the merge behaviors involved. pub fn LLVMRustAddModuleFlagU32( M: &Module, - merge_behavior: LLVMModFlagBehavior, - name: *const c_char, - value: u32, + MergeBehavior: ModuleFlagMergeBehavior, + Name: *const c_char, + NameLen: size_t, + Value: u32, ); pub fn LLVMRustAddModuleFlagString( M: &Module, - merge_behavior: LLVMModFlagBehavior, - name: *const c_char, - value: *const c_char, - value_len: size_t, + MergeBehavior: ModuleFlagMergeBehavior, + Name: *const c_char, + NameLen: size_t, + Value: *const c_char, + ValueLen: size_t, ); pub fn LLVMRustDIBuilderCreate(M: &Module) -> &mut DIBuilder<'_>; diff --git a/compiler/rustc_codegen_llvm/src/llvm/mod.rs b/compiler/rustc_codegen_llvm/src/llvm/mod.rs index cabe6c031d3..acd425bbb8e 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/mod.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/mod.rs @@ -352,3 +352,32 @@ impl Drop for OperandBundleDef<'_> { } } } + +pub(crate) fn add_module_flag_u32( + module: &Module, + merge_behavior: ModuleFlagMergeBehavior, + key: &str, + value: u32, +) { + unsafe { + LLVMRustAddModuleFlagU32(module, merge_behavior, key.as_c_char_ptr(), key.len(), value); + } +} + +pub(crate) fn add_module_flag_str( + module: &Module, + merge_behavior: ModuleFlagMergeBehavior, + key: &str, + value: &str, +) { + unsafe { + LLVMRustAddModuleFlagString( + module, + merge_behavior, + key.as_c_char_ptr(), + key.len(), + value.as_c_char_ptr(), + value.len(), + ); + } +} diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index 634e46aaf8b..690f808c5f1 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -853,17 +853,60 @@ extern "C" uint32_t LLVMRustVersionMinor() { return LLVM_VERSION_MINOR; } extern "C" uint32_t LLVMRustVersionMajor() { return LLVM_VERSION_MAJOR; } -extern "C" void LLVMRustAddModuleFlagU32(LLVMModuleRef M, - Module::ModFlagBehavior MergeBehavior, - const char *Name, uint32_t Value) { - unwrap(M)->addModuleFlag(MergeBehavior, Name, Value); +// FFI equivalent of LLVM's `llvm::Module::ModFlagBehavior`. +// Must match the layout of +// `rustc_codegen_llvm::llvm::ffi::ModuleFlagMergeBehavior`. +// +// There is a stable LLVM-C version of this enum (`LLVMModuleFlagBehavior`), +// but as of LLVM 19 it does not support all of the enum values in the unstable +// C++ API. +enum class LLVMRustModuleFlagMergeBehavior { + Error = 1, + Warning = 2, + Require = 3, + Override = 4, + Append = 5, + AppendUnique = 6, + Max = 7, + Min = 8, +}; + +static Module::ModFlagBehavior +fromRust(LLVMRustModuleFlagMergeBehavior Behavior) { + switch (Behavior) { + case LLVMRustModuleFlagMergeBehavior::Error: + return Module::ModFlagBehavior::Error; + case LLVMRustModuleFlagMergeBehavior::Warning: + return Module::ModFlagBehavior::Warning; + case LLVMRustModuleFlagMergeBehavior::Require: + return Module::ModFlagBehavior::Require; + case LLVMRustModuleFlagMergeBehavior::Override: + return Module::ModFlagBehavior::Override; + case LLVMRustModuleFlagMergeBehavior::Append: + return Module::ModFlagBehavior::Append; + case LLVMRustModuleFlagMergeBehavior::AppendUnique: + return Module::ModFlagBehavior::AppendUnique; + case LLVMRustModuleFlagMergeBehavior::Max: + return Module::ModFlagBehavior::Max; + case LLVMRustModuleFlagMergeBehavior::Min: + return Module::ModFlagBehavior::Min; + } + report_fatal_error("bad LLVMRustModuleFlagMergeBehavior"); +} + +extern "C" void +LLVMRustAddModuleFlagU32(LLVMModuleRef M, + LLVMRustModuleFlagMergeBehavior MergeBehavior, + const char *Name, size_t NameLen, uint32_t Value) { + unwrap(M)->addModuleFlag(fromRust(MergeBehavior), StringRef(Name, NameLen), + Value); } extern "C" void LLVMRustAddModuleFlagString( - LLVMModuleRef M, Module::ModFlagBehavior MergeBehavior, const char *Name, - const char *Value, size_t ValueLen) { + LLVMModuleRef M, LLVMRustModuleFlagMergeBehavior MergeBehavior, + const char *Name, size_t NameLen, const char *Value, size_t ValueLen) { unwrap(M)->addModuleFlag( - MergeBehavior, Name, + fromRust(MergeBehavior), StringRef(Name, NameLen), MDString::get(unwrap(M)->getContext(), StringRef(Value, ValueLen))); } -- cgit 1.4.1-3-g733a5 From c3071590ab70f016380fe16030057e84b76e59eb Mon Sep 17 00:00:00 2001 From: Zalathar Date: Mon, 28 Oct 2024 17:25:40 +1100 Subject: Clean up FFI calls for operand bundles --- compiler/rustc_codegen_llvm/src/allocator.rs | 3 +- compiler/rustc_codegen_llvm/src/builder.rs | 23 +++--- compiler/rustc_codegen_llvm/src/common.rs | 8 +-- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 91 ++++++++++++------------ compiler/rustc_codegen_llvm/src/llvm/mod.rs | 38 +++++++--- compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp | 78 ++++---------------- 6 files changed, 103 insertions(+), 138 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src/llvm/mod.rs') diff --git a/compiler/rustc_codegen_llvm/src/allocator.rs b/compiler/rustc_codegen_llvm/src/allocator.rs index a2e6c7eb856..149ded28356 100644 --- a/compiler/rustc_codegen_llvm/src/allocator.rs +++ b/compiler/rustc_codegen_llvm/src/allocator.rs @@ -154,7 +154,7 @@ fn create_wrapper_function( .enumerate() .map(|(i, _)| llvm::LLVMGetParam(llfn, i as c_uint)) .collect::>(); - let ret = llvm::LLVMRustBuildCall( + let ret = llvm::LLVMBuildCallWithOperandBundles( llbuilder, ty, callee, @@ -162,6 +162,7 @@ fn create_wrapper_function( args.len() as c_uint, [].as_ptr(), 0 as c_uint, + c"".as_ptr(), ); llvm::LLVMSetTailCall(ret, True); if output.is_some() { diff --git a/compiler/rustc_codegen_llvm/src/builder.rs b/compiler/rustc_codegen_llvm/src/builder.rs index 8702532c36e..f8eb3e0c982 100644 --- a/compiler/rustc_codegen_llvm/src/builder.rs +++ b/compiler/rustc_codegen_llvm/src/builder.rs @@ -239,7 +239,6 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { let args = self.check_call("invoke", llty, llfn, args); let funclet_bundle = funclet.map(|funclet| funclet.bundle()); - let funclet_bundle = funclet_bundle.as_ref().map(|b| &*b.raw); let mut bundles: SmallVec<[_; 2]> = SmallVec::new(); if let Some(funclet_bundle) = funclet_bundle { bundles.push(funclet_bundle); @@ -250,13 +249,12 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { // Emit KCFI operand bundle let kcfi_bundle = self.kcfi_operand_bundle(fn_attrs, fn_abi, instance, llfn); - let kcfi_bundle = kcfi_bundle.as_ref().map(|b| &*b.raw); - if let Some(kcfi_bundle) = kcfi_bundle { + if let Some(kcfi_bundle) = kcfi_bundle.as_deref() { bundles.push(kcfi_bundle); } let invoke = unsafe { - llvm::LLVMRustBuildInvoke( + llvm::LLVMBuildInvokeWithOperandBundles( self.llbuilder, llty, llfn, @@ -1179,7 +1177,6 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { let args = self.check_call("call", llty, llfn, args); let funclet_bundle = funclet.map(|funclet| funclet.bundle()); - let funclet_bundle = funclet_bundle.as_ref().map(|b| &*b.raw); let mut bundles: SmallVec<[_; 2]> = SmallVec::new(); if let Some(funclet_bundle) = funclet_bundle { bundles.push(funclet_bundle); @@ -1190,13 +1187,12 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { // Emit KCFI operand bundle let kcfi_bundle = self.kcfi_operand_bundle(fn_attrs, fn_abi, instance, llfn); - let kcfi_bundle = kcfi_bundle.as_ref().map(|b| &*b.raw); - if let Some(kcfi_bundle) = kcfi_bundle { + if let Some(kcfi_bundle) = kcfi_bundle.as_deref() { bundles.push(kcfi_bundle); } let call = unsafe { - llvm::LLVMRustBuildCall( + llvm::LLVMBuildCallWithOperandBundles( self.llbuilder, llty, llfn, @@ -1204,6 +1200,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { args.len() as c_uint, bundles.as_ptr(), bundles.len() as c_uint, + c"".as_ptr(), ) }; if let Some(fn_abi) = fn_abi { @@ -1509,7 +1506,6 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> { let args = self.check_call("callbr", llty, llfn, args); let funclet_bundle = funclet.map(|funclet| funclet.bundle()); - let funclet_bundle = funclet_bundle.as_ref().map(|b| &*b.raw); let mut bundles: SmallVec<[_; 2]> = SmallVec::new(); if let Some(funclet_bundle) = funclet_bundle { bundles.push(funclet_bundle); @@ -1520,13 +1516,12 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> { // Emit KCFI operand bundle let kcfi_bundle = self.kcfi_operand_bundle(fn_attrs, fn_abi, instance, llfn); - let kcfi_bundle = kcfi_bundle.as_ref().map(|b| &*b.raw); - if let Some(kcfi_bundle) = kcfi_bundle { + if let Some(kcfi_bundle) = kcfi_bundle.as_deref() { bundles.push(kcfi_bundle); } let callbr = unsafe { - llvm::LLVMRustBuildCallBr( + llvm::LLVMBuildCallBr( self.llbuilder, llty, llfn, @@ -1601,7 +1596,7 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> { fn_abi: Option<&FnAbi<'tcx, Ty<'tcx>>>, instance: Option>, llfn: &'ll Value, - ) -> Option> { + ) -> Option> { let is_indirect_call = unsafe { llvm::LLVMRustIsNonGVFunctionPointerTy(llfn) }; let kcfi_bundle = if self.tcx.sess.is_sanitizer_kcfi_enabled() && let Some(fn_abi) = fn_abi @@ -1627,7 +1622,7 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> { kcfi::typeid_for_fnabi(self.tcx, fn_abi, options) }; - Some(llvm::OperandBundleDef::new("kcfi", &[self.const_u32(kcfi_typeid)])) + Some(llvm::OperandBundleOwned::new("kcfi", &[self.const_u32(kcfi_typeid)])) } else { None }; diff --git a/compiler/rustc_codegen_llvm/src/common.rs b/compiler/rustc_codegen_llvm/src/common.rs index 29adc616ee2..8852dec7d9f 100644 --- a/compiler/rustc_codegen_llvm/src/common.rs +++ b/compiler/rustc_codegen_llvm/src/common.rs @@ -17,7 +17,7 @@ use tracing::debug; use crate::consts::const_alloc_to_llvm; pub(crate) use crate::context::CodegenCx; -use crate::llvm::{self, BasicBlock, Bool, ConstantInt, False, Metadata, OperandBundleDef, True}; +use crate::llvm::{self, BasicBlock, Bool, ConstantInt, False, Metadata, True}; use crate::type_::Type; use crate::value::Value; @@ -63,19 +63,19 @@ use crate::value::Value; /// the `OperandBundleDef` value created for MSVC landing pads. pub(crate) struct Funclet<'ll> { cleanuppad: &'ll Value, - operand: OperandBundleDef<'ll>, + operand: llvm::OperandBundleOwned<'ll>, } impl<'ll> Funclet<'ll> { pub(crate) fn new(cleanuppad: &'ll Value) -> Self { - Funclet { cleanuppad, operand: OperandBundleDef::new("funclet", &[cleanuppad]) } + Funclet { cleanuppad, operand: llvm::OperandBundleOwned::new("funclet", &[cleanuppad]) } } pub(crate) fn cleanuppad(&self) -> &'ll Value { self.cleanuppad } - pub(crate) fn bundle(&self) -> &OperandBundleDef<'ll> { + pub(crate) fn bundle(&self) -> &llvm::OperandBundle<'ll> { &self.operand } } diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 888d41d4726..d67f76b51a2 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -3,6 +3,7 @@ use std::fmt::Debug; use std::marker::PhantomData; +use std::ptr; use libc::{c_char, c_int, c_uint, c_ulonglong, c_void, size_t}; use rustc_macros::TryFromU32; @@ -708,8 +709,9 @@ unsafe extern "C" { } #[repr(C)] pub struct RustArchiveMember<'a>(InvariantOpaque<'a>); +/// Opaque pointee of `LLVMOperandBundleRef`. #[repr(C)] -pub struct OperandBundleDef<'a>(InvariantOpaque<'a>); +pub(crate) struct OperandBundle<'a>(InvariantOpaque<'a>); #[repr(C)] pub struct Linker<'a>(InvariantOpaque<'a>); @@ -1538,6 +1540,50 @@ unsafe extern "C" { pub fn LLVMGetOrInsertComdat(M: &Module, Name: *const c_char) -> &Comdat; pub fn LLVMSetComdat(V: &Value, C: &Comdat); + + pub(crate) fn LLVMCreateOperandBundle( + Tag: *const c_char, + TagLen: size_t, + Args: *const &'_ Value, + NumArgs: c_uint, + ) -> *mut OperandBundle<'_>; + pub(crate) fn LLVMDisposeOperandBundle(Bundle: ptr::NonNull>); + + pub(crate) fn LLVMBuildCallWithOperandBundles<'a>( + B: &Builder<'a>, + Ty: &'a Type, + Fn: &'a Value, + Args: *const &'a Value, + NumArgs: c_uint, + Bundles: *const &OperandBundle<'a>, + NumBundles: c_uint, + Name: *const c_char, + ) -> &'a Value; + pub(crate) fn LLVMBuildInvokeWithOperandBundles<'a>( + B: &Builder<'a>, + Ty: &'a Type, + Fn: &'a Value, + Args: *const &'a Value, + NumArgs: c_uint, + Then: &'a BasicBlock, + Catch: &'a BasicBlock, + Bundles: *const &OperandBundle<'a>, + NumBundles: c_uint, + Name: *const c_char, + ) -> &'a Value; + pub(crate) fn LLVMBuildCallBr<'a>( + B: &Builder<'a>, + Ty: &'a Type, + Fn: &'a Value, + DefaultDest: &'a BasicBlock, + IndirectDests: *const &'a BasicBlock, + NumIndirectDests: c_uint, + Args: *const &'a Value, + NumArgs: c_uint, + Bundles: *const &OperandBundle<'a>, + NumBundles: c_uint, + Name: *const c_char, + ) -> &'a Value; } #[link(name = "llvm-wrapper", kind = "static")] @@ -1623,47 +1669,11 @@ unsafe extern "C" { AttrsLen: size_t, ); - pub fn LLVMRustBuildInvoke<'a>( - B: &Builder<'a>, - Ty: &'a Type, - Fn: &'a Value, - Args: *const &'a Value, - NumArgs: c_uint, - Then: &'a BasicBlock, - Catch: &'a BasicBlock, - OpBundles: *const &OperandBundleDef<'a>, - NumOpBundles: c_uint, - Name: *const c_char, - ) -> &'a Value; - - pub fn LLVMRustBuildCallBr<'a>( - B: &Builder<'a>, - Ty: &'a Type, - Fn: &'a Value, - DefaultDest: &'a BasicBlock, - IndirectDests: *const &'a BasicBlock, - NumIndirectDests: c_uint, - Args: *const &'a Value, - NumArgs: c_uint, - OpBundles: *const &OperandBundleDef<'a>, - NumOpBundles: c_uint, - Name: *const c_char, - ) -> &'a Value; - pub fn LLVMRustSetFastMath(Instr: &Value); pub fn LLVMRustSetAlgebraicMath(Instr: &Value); pub fn LLVMRustSetAllowReassoc(Instr: &Value); // Miscellaneous instructions - pub fn LLVMRustBuildCall<'a>( - B: &Builder<'a>, - Ty: &'a Type, - Fn: &'a Value, - Args: *const &'a Value, - NumArgs: c_uint, - OpBundles: *const &OperandBundleDef<'a>, - NumOpBundles: c_uint, - ) -> &'a Value; pub fn LLVMRustBuildMemCpy<'a>( B: &Builder<'a>, Dst: &'a Value, @@ -2357,13 +2367,6 @@ unsafe extern "C" { pub fn LLVMRustSetDataLayoutFromTargetMachine<'a>(M: &'a Module, TM: &'a TargetMachine); - pub fn LLVMRustBuildOperandBundleDef( - Name: *const c_char, - Inputs: *const &'_ Value, - NumInputs: c_uint, - ) -> &mut OperandBundleDef<'_>; - pub fn LLVMRustFreeOperandBundleDef<'a>(Bundle: &'a mut OperandBundleDef<'a>); - pub fn LLVMRustPositionBuilderAtStart<'a>(B: &Builder<'a>, BB: &'a BasicBlock); pub fn LLVMRustSetModulePICLevel(M: &Module); diff --git a/compiler/rustc_codegen_llvm/src/llvm/mod.rs b/compiler/rustc_codegen_llvm/src/llvm/mod.rs index acd425bbb8e..00a5cd3b859 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/mod.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/mod.rs @@ -2,11 +2,12 @@ use std::cell::RefCell; use std::ffi::{CStr, CString}; +use std::ops::Deref; +use std::ptr; use std::str::FromStr; use std::string::FromUtf8Error; use libc::c_uint; -use rustc_data_structures::small_c_str::SmallCStr; use rustc_llvm::RustString; use rustc_target::abi::{Align, Size, WrappingRange}; @@ -331,28 +332,43 @@ pub fn last_error() -> Option { } } -pub struct OperandBundleDef<'a> { - pub raw: &'a mut ffi::OperandBundleDef<'a>, +/// Owns an [`OperandBundle`], and will dispose of it when dropped. +pub(crate) struct OperandBundleOwned<'a> { + raw: ptr::NonNull>, } -impl<'a> OperandBundleDef<'a> { - pub fn new(name: &str, vals: &[&'a Value]) -> Self { - let name = SmallCStr::new(name); - let def = unsafe { - LLVMRustBuildOperandBundleDef(name.as_ptr(), vals.as_ptr(), vals.len() as c_uint) +impl<'a> OperandBundleOwned<'a> { + pub(crate) fn new(name: &str, vals: &[&'a Value]) -> Self { + let raw = unsafe { + LLVMCreateOperandBundle( + name.as_c_char_ptr(), + name.len(), + vals.as_ptr(), + vals.len() as c_uint, + ) }; - OperandBundleDef { raw: def } + OperandBundleOwned { raw: ptr::NonNull::new(raw).unwrap() } } } -impl Drop for OperandBundleDef<'_> { +impl Drop for OperandBundleOwned<'_> { fn drop(&mut self) { unsafe { - LLVMRustFreeOperandBundleDef(&mut *(self.raw as *mut _)); + LLVMDisposeOperandBundle(self.raw); } } } +impl<'a> Deref for OperandBundleOwned<'a> { + type Target = OperandBundle<'a>; + + fn deref(&self) -> &Self::Target { + // SAFETY: The returned reference is opaque and can only used for FFI. + // It is valid for as long as `&self` is. + unsafe { self.raw.as_ref() } + } +} + pub(crate) fn add_module_flag_u32( module: &Module, merge_behavior: ModuleFlagMergeBehavior, diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index b3dab6d512e..645b4082be5 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -1537,38 +1537,6 @@ LLVMRustUnpackSMDiagnostic(LLVMSMDiagnosticRef DRef, RustStringRef MessageOut, return true; } -extern "C" OperandBundleDef *LLVMRustBuildOperandBundleDef(const char *Name, - LLVMValueRef *Inputs, - unsigned NumInputs) { - return new OperandBundleDef(Name, - ArrayRef(unwrap(Inputs), NumInputs)); -} - -extern "C" void LLVMRustFreeOperandBundleDef(OperandBundleDef *Bundle) { - delete Bundle; -} - -// OpBundlesIndirect is an array of pointers (*not* a pointer to an array). -extern "C" LLVMValueRef LLVMRustBuildCall(LLVMBuilderRef B, LLVMTypeRef Ty, - LLVMValueRef Fn, LLVMValueRef *Args, - unsigned NumArgs, - OperandBundleDef **OpBundlesIndirect, - unsigned NumOpBundles) { - Value *Callee = unwrap(Fn); - FunctionType *FTy = unwrap(Ty); - - // FIXME: Is there a way around this? - SmallVector OpBundles; - OpBundles.reserve(NumOpBundles); - for (unsigned i = 0; i < NumOpBundles; ++i) { - OpBundles.push_back(*OpBundlesIndirect[i]); - } - - return wrap(unwrap(B)->CreateCall(FTy, Callee, - ArrayRef(unwrap(Args), NumArgs), - ArrayRef(OpBundles))); -} - extern "C" LLVMValueRef LLVMRustBuildMemCpy(LLVMBuilderRef B, LLVMValueRef Dst, unsigned DstAlign, LLVMValueRef Src, unsigned SrcAlign, @@ -1596,37 +1564,18 @@ extern "C" LLVMValueRef LLVMRustBuildMemSet(LLVMBuilderRef B, LLVMValueRef Dst, MaybeAlign(DstAlign), IsVolatile)); } -// OpBundlesIndirect is an array of pointers (*not* a pointer to an array). -extern "C" LLVMValueRef -LLVMRustBuildInvoke(LLVMBuilderRef B, LLVMTypeRef Ty, LLVMValueRef Fn, - LLVMValueRef *Args, unsigned NumArgs, - LLVMBasicBlockRef Then, LLVMBasicBlockRef Catch, - OperandBundleDef **OpBundlesIndirect, unsigned NumOpBundles, - const char *Name) { - Value *Callee = unwrap(Fn); - FunctionType *FTy = unwrap(Ty); - - // FIXME: Is there a way around this? - SmallVector OpBundles; - OpBundles.reserve(NumOpBundles); - for (unsigned i = 0; i < NumOpBundles; ++i) { - OpBundles.push_back(*OpBundlesIndirect[i]); - } - - return wrap(unwrap(B)->CreateInvoke(FTy, Callee, unwrap(Then), unwrap(Catch), - ArrayRef(unwrap(Args), NumArgs), - ArrayRef(OpBundles), - Name)); -} +// Polyfill for `LLVMBuildCallBr`, which was added in LLVM 19. +// +// FIXME: Remove when Rust's minimum supported LLVM version reaches 19. +#if LLVM_VERSION_LT(19, 0) +DEFINE_SIMPLE_CONVERSION_FUNCTIONS(OperandBundleDef, LLVMOperandBundleRef) -// OpBundlesIndirect is an array of pointers (*not* a pointer to an array). extern "C" LLVMValueRef -LLVMRustBuildCallBr(LLVMBuilderRef B, LLVMTypeRef Ty, LLVMValueRef Fn, - LLVMBasicBlockRef DefaultDest, - LLVMBasicBlockRef *IndirectDests, unsigned NumIndirectDests, - LLVMValueRef *Args, unsigned NumArgs, - OperandBundleDef **OpBundlesIndirect, unsigned NumOpBundles, - const char *Name) { +LLVMBuildCallBr(LLVMBuilderRef B, LLVMTypeRef Ty, LLVMValueRef Fn, + LLVMBasicBlockRef DefaultDest, LLVMBasicBlockRef *IndirectDests, + unsigned NumIndirectDests, LLVMValueRef *Args, unsigned NumArgs, + LLVMOperandBundleRef *Bundles, unsigned NumBundles, + const char *Name) { Value *Callee = unwrap(Fn); FunctionType *FTy = unwrap(Ty); @@ -1639,9 +1588,9 @@ LLVMRustBuildCallBr(LLVMBuilderRef B, LLVMTypeRef Ty, LLVMValueRef Fn, // FIXME: Is there a way around this? SmallVector OpBundles; - OpBundles.reserve(NumOpBundles); - for (unsigned i = 0; i < NumOpBundles; ++i) { - OpBundles.push_back(*OpBundlesIndirect[i]); + OpBundles.reserve(NumBundles); + for (unsigned i = 0; i < NumBundles; ++i) { + OpBundles.push_back(*unwrap(Bundles[i])); } return wrap( @@ -1650,6 +1599,7 @@ LLVMRustBuildCallBr(LLVMBuilderRef B, LLVMTypeRef Ty, LLVMValueRef Fn, ArrayRef(unwrap(Args), NumArgs), ArrayRef(OpBundles), Name)); } +#endif extern "C" void LLVMRustPositionBuilderAtStart(LLVMBuilderRef B, LLVMBasicBlockRef BB) { -- cgit 1.4.1-3-g733a5