Skip to content
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

Add OwnedTargetMachine to manage llvm:TargetMachine #115911

Merged
merged 1 commit into from
Sep 24, 2023
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
101 changes: 101 additions & 0 deletions compiler/rustc_codegen_llvm/src/back/owned_target_machine.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
use std::{
ffi::{c_char, CStr},
marker::PhantomData,
ops::Deref,
ptr::NonNull,
};

use rustc_data_structures::small_c_str::SmallCStr;

use crate::{errors::LlvmError, llvm};

/// Responsible for safely creating and disposing llvm::TargetMachine via ffi functions.
/// Not cloneable as there is no clone function for llvm::TargetMachine.
#[repr(transparent)]
pub struct OwnedTargetMachine {
tm_unique: NonNull<llvm::TargetMachine>,
phantom: PhantomData<llvm::TargetMachine>,
}

impl OwnedTargetMachine {
pub fn new(
triple: &CStr,
cpu: &CStr,
features: &CStr,
abi: &CStr,
model: llvm::CodeModel,
reloc: llvm::RelocModel,
level: llvm::CodeGenOptLevel,
use_soft_fp: bool,
function_sections: bool,
data_sections: bool,
unique_section_names: bool,
trap_unreachable: bool,
singletree: bool,
asm_comments: bool,
emit_stack_size_section: bool,
relax_elf_relocations: bool,
use_init_array: bool,
split_dwarf_file: &CStr,
debug_info_compression: &CStr,
force_emulated_tls: bool,
args_cstr_buff: &[u8],
) -> Result<Self, LlvmError<'static>> {
assert!(args_cstr_buff.len() > 0);
assert!(
*args_cstr_buff.last().unwrap() == 0,
"The last character must be a null terminator."
);

// SAFETY: llvm::LLVMRustCreateTargetMachine copies pointed to data
let tm_ptr = unsafe {
llvm::LLVMRustCreateTargetMachine(
triple.as_ptr(),
cpu.as_ptr(),
features.as_ptr(),
abi.as_ptr(),
model,
reloc,
level,
use_soft_fp,
function_sections,
data_sections,
unique_section_names,
trap_unreachable,
singletree,
asm_comments,
emit_stack_size_section,
relax_elf_relocations,
use_init_array,
split_dwarf_file.as_ptr(),
debug_info_compression.as_ptr(),
force_emulated_tls,
args_cstr_buff.as_ptr() as *const c_char,
args_cstr_buff.len(),
)
};

NonNull::new(tm_ptr)
.map(|tm_unique| Self { tm_unique, phantom: PhantomData })
.ok_or_else(|| LlvmError::CreateTargetMachine { triple: SmallCStr::from(triple) })
}
}

impl Deref for OwnedTargetMachine {
type Target = llvm::TargetMachine;

fn deref(&self) -> &Self::Target {
// SAFETY: constructing ensures we have a valid pointer created by llvm::LLVMRustCreateTargetMachine
unsafe { self.tm_unique.as_ref() }
}
}

impl Drop for OwnedTargetMachine {
fn drop(&mut self) {
// SAFETY: constructing ensures we have a valid pointer created by llvm::LLVMRustCreateTargetMachine
// OwnedTargetMachine is not copyable so there is no double free or use after free
unsafe {
llvm::LLVMRustDisposeTargetMachine(self.tm_unique.as_mut());
}
}
}
56 changes: 26 additions & 30 deletions compiler/rustc_codegen_llvm/src/back/write.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::back::lto::ThinBuffer;
use crate::back::owned_target_machine::OwnedTargetMachine;
use crate::back::profiling::{
selfprofile_after_pass_callback, selfprofile_before_pass_callback, LlvmSelfProfiler,
};
Expand Down Expand Up @@ -98,7 +99,7 @@ pub fn write_output_file<'ll>(
}
}

pub fn create_informational_target_machine(sess: &Session) -> &'static mut llvm::TargetMachine {
Noratrieb marked this conversation as resolved.
Show resolved Hide resolved
pub fn create_informational_target_machine(sess: &Session) -> OwnedTargetMachine {
let config = TargetMachineFactoryConfig { split_dwarf_file: None };
// Can't use query system here quite yet because this function is invoked before the query
// system/tcx is set up.
Expand All @@ -107,7 +108,7 @@ pub fn create_informational_target_machine(sess: &Session) -> &'static mut llvm:
.unwrap_or_else(|err| llvm_err(sess.diagnostic(), err).raise())
}

pub fn create_target_machine(tcx: TyCtxt<'_>, mod_name: &str) -> &'static mut llvm::TargetMachine {
pub fn create_target_machine(tcx: TyCtxt<'_>, mod_name: &str) -> OwnedTargetMachine {
let split_dwarf_file = if tcx.sess.target_can_use_split_dwarf() {
tcx.output_filenames(()).split_dwarf_path(
tcx.sess.split_debuginfo(),
Expand Down Expand Up @@ -259,34 +260,29 @@ pub fn target_machine_factory(
path_mapping.map_prefix(config.split_dwarf_file.unwrap_or_default()).0;
let split_dwarf_file = CString::new(split_dwarf_file.to_str().unwrap()).unwrap();

let tm = unsafe {
llvm::LLVMRustCreateTargetMachine(
triple.as_ptr(),
cpu.as_ptr(),
features.as_ptr(),
abi.as_ptr(),
code_model,
reloc_model,
opt_level,
use_softfp,
ffunction_sections,
fdata_sections,
funique_section_names,
trap_unreachable,
singlethread,
asm_comments,
emit_stack_size_section,
relax_elf_relocations,
use_init_array,
split_dwarf_file.as_ptr(),
debuginfo_compression.as_ptr(),
force_emulated_tls,
args_cstr_buff.as_ptr() as *const c_char,
args_cstr_buff.len(),
)
};

tm.ok_or_else(|| LlvmError::CreateTargetMachine { triple: triple.clone() })
OwnedTargetMachine::new(
&triple,
&cpu,
&features,
&abi,
code_model,
reloc_model,
opt_level,
use_softfp,
ffunction_sections,
fdata_sections,
funique_section_names,
trap_unreachable,
singlethread,
asm_comments,
emit_stack_size_section,
relax_elf_relocations,
use_init_array,
&split_dwarf_file,
&debuginfo_compression,
force_emulated_tls,
&args_cstr_buff,
)
})
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_llvm/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,9 @@ pub unsafe fn create_module<'ll>(

// Ensure the data-layout values hardcoded remain the defaults.
if sess.target.is_builtin {
// tm is disposed by its drop impl
let tm = crate::back::write::create_informational_target_machine(tcx.sess);
Noratrieb marked this conversation as resolved.
Show resolved Hide resolved
llvm::LLVMRustSetDataLayoutFromTargetMachine(llmod, tm);
llvm::LLVMRustDisposeTargetMachine(tm);
llvm::LLVMRustSetDataLayoutFromTargetMachine(llmod, &tm);

let llvm_data_layout = llvm::LLVMGetDataLayoutStr(llmod);
let llvm_data_layout = str::from_utf8(CStr::from_ptr(llvm_data_layout).to_bytes())
Expand Down
9 changes: 6 additions & 3 deletions compiler/rustc_codegen_llvm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ extern crate rustc_macros;
#[macro_use]
extern crate tracing;

use back::owned_target_machine::OwnedTargetMachine;
use back::write::{create_informational_target_machine, create_target_machine};

use errors::ParseTargetMachineConfig;
Expand Down Expand Up @@ -52,6 +53,7 @@ use std::io::Write;
mod back {
pub mod archive;
pub mod lto;
pub mod owned_target_machine;
mod profiling;
pub mod write;
}
Expand Down Expand Up @@ -162,7 +164,7 @@ impl ExtraBackendMethods for LlvmCodegenBackend {
impl WriteBackendMethods for LlvmCodegenBackend {
type Module = ModuleLlvm;
type ModuleBuffer = back::lto::ModuleBuffer;
type TargetMachine = &'static mut llvm::TargetMachine;
type TargetMachine = OwnedTargetMachine;
type TargetMachineError = crate::errors::LlvmError<'static>;
type ThinData = back::lto::ThinData;
type ThinBuffer = back::lto::ThinBuffer;
Expand Down Expand Up @@ -401,7 +403,9 @@ impl CodegenBackend for LlvmCodegenBackend {
pub struct ModuleLlvm {
llcx: &'static mut llvm::Context,
llmod_raw: *const llvm::Module,
tm: &'static mut llvm::TargetMachine,

// independent from llcx and llmod_raw, resources get disposed by drop impl
tm: OwnedTargetMachine,
}

unsafe impl Send for ModuleLlvm {}
Expand Down Expand Up @@ -453,7 +457,6 @@ impl ModuleLlvm {
impl Drop for ModuleLlvm {
fn drop(&mut self) {
unsafe {
llvm::LLVMRustDisposeTargetMachine(&mut *(self.tm as *mut _));
llvm::LLVMContextDispose(&mut *(self.llcx as *mut _));
}
}
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_codegen_llvm/src/llvm/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2112,6 +2112,8 @@ extern "C" {
);

pub fn LLVMRustGetHostCPUName(len: *mut usize) -> *const c_char;

// This function makes copies of pointed to data, so the data's lifetime may end after this function returns
pub fn LLVMRustCreateTargetMachine(
Triple: *const c_char,
CPU: *const c_char,
Expand All @@ -2135,9 +2137,9 @@ extern "C" {
ForceEmulatedTls: bool,
ArgsCstrBuff: *const c_char,
ArgsCstrBuffLen: usize,
) -> Option<&'static mut TargetMachine>;
) -> *mut TargetMachine;

pub fn LLVMRustDisposeTargetMachine(T: &'static mut TargetMachine);
pub fn LLVMRustDisposeTargetMachine(T: *mut TargetMachine);
pub fn LLVMRustAddLibraryInfo<'a>(
PM: &PassManager<'a>,
M: &'a Module,
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_codegen_llvm/src/llvm_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ pub fn target_features(sess: &Session, allow_unstable: bool) -> Vec<Symbol> {
// check that all features in a given smallvec are enabled
for llvm_feature in to_llvm_features(sess, feature) {
let cstr = SmallCStr::new(llvm_feature);
if !unsafe { llvm::LLVMRustHasFeature(target_machine, cstr.as_ptr()) } {
if !unsafe { llvm::LLVMRustHasFeature(&target_machine, cstr.as_ptr()) } {
return false;
}
}
Expand Down Expand Up @@ -422,14 +422,14 @@ pub(crate) fn print(req: &PrintRequest, mut out: &mut dyn PrintBackendInfo, sess
}
unsafe {
llvm::LLVMRustPrintTargetCPUs(
tm,
&tm,
cpu_cstring.as_ptr(),
callback,
&mut out as *mut &mut dyn PrintBackendInfo as *mut c_void,
);
}
}
PrintKind::TargetFeatures => print_target_features(out, sess, tm),
PrintKind::TargetFeatures => print_target_features(out, sess, &tm),
_ => bug!("rustc_codegen_llvm can't handle print request: {:?}", req),
}
}
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_data_structures/src/small_c_str.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,9 @@ impl<'a> FromIterator<&'a str> for SmallCStr {
Self { data }
}
}

impl From<&ffi::CStr> for SmallCStr {
fn from(s: &ffi::CStr) -> Self {
Self { data: SmallVec::from_slice(s.to_bytes()) }
}
}