rust-lang/rust · #161903

Fix initialization cycle in `target_config`

nnethercote · merged Sep 14, 202617 files · 163 + / 44
compiler/rustc_codegen_llvm/src/back/mod.rs1 + / 0
@@ -1,5 +1,6 @@ pub(crate) mod archive; pub(crate) mod lto;+pub(crate) mod owned_mc_subtarget_info; pub(crate) mod owned_target_machine; mod profiling; pub(crate) mod write;
compiler/rustc_codegen_llvm/src/back/owned_mc_subtarget_info.rsadded49 + / 0
@@ -0,0 +1,49 @@+use std::ffi::CStr;+use std::ptr::NonNull;++use rustc_data_structures::small_c_str::SmallCStr;++use crate::diagnostics::LlvmError;+use crate::llvm;++/// Responsible for safely creating and disposing llvm::MCSubtargetInfo via ffi functions.+/// Not cloneable as there is no clone function for llvm::MCSubtargetInfo.+pub(crate) struct OwnedMCSubtargetInfo {+    info_unique: NonNull<llvm::MCSubtargetInfo>,+}++impl OwnedMCSubtargetInfo {+    pub(crate) fn new(+        triple: &CStr,+        cpu: &CStr,+        features: &CStr,+    ) -> Result<Self, LlvmError<'static>> {+        // SAFETY: llvm::LLVMRustCreateMCSubtargetInfo copies pointed-to data.+        let info_ptr = unsafe {+            llvm::LLVMRustCreateMCSubtargetInfo(triple.as_ptr(), cpu.as_ptr(), features.as_ptr())+        };++        NonNull::new(info_ptr)+            .map(|info_unique| Self { info_unique })+            .ok_or_else(|| LlvmError::CreateMCSubtargetInfo { triple: SmallCStr::from(triple) })+    }++    pub(crate) fn has_feature(&self, feature: &CStr) -> bool {+        // SAFETY: `new` ensures we have a valid pointer created by+        // `llvm::LLVMRustCreateMCSubtargetInfo`.+        unsafe {+            llvm::LLVMRustMCSubtargetInfoHasFeature(self.info_unique.as_ref(), feature.as_ptr())+        }+    }+}++impl Drop for OwnedMCSubtargetInfo {+    fn drop(&mut self) {+        // SAFETY: `new` ensures we have a valid pointer created by+        // `llvm::LLVMRustCreateMCSubtargetInfo` and `OwnedMCSubtargetInfo` is not copyable so+        // there is no double free or use after free.+        unsafe {+            llvm::LLVMRustDisposeMCSubtargetInfo(self.info_unique);+        }+    }+}
compiler/rustc_codegen_llvm/src/back/owned_target_machine.rs2 + / 5
@@ -1,5 +1,4 @@ use std::ffi::CStr;-use std::marker::PhantomData; use std::ptr::NonNull;  use rustc_data_structures::small_c_str::SmallCStr;@@ -9,10 +8,8 @@ use crate::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 {@@ -41,7 +38,7 @@ impl OwnedTargetMachine {         use_wasm_eh: bool,         large_data_threshold: u64,     ) -> Result<Self, LlvmError<'static>> {-        // SAFETY: llvm::LLVMRustCreateTargetMachine copies pointed to data+        // SAFETY: llvm::LLVMRustCreateTargetMachine copies pointed-to data.         let tm_ptr = unsafe {             llvm::LLVMRustCreateTargetMachine(                 triple.as_ptr(),@@ -71,7 +68,7 @@ impl OwnedTargetMachine {         };          NonNull::new(tm_ptr)-            .map(|tm_unique| Self { tm_unique, phantom: PhantomData })+            .map(|tm_unique| Self { tm_unique })             .ok_or_else(|| LlvmError::CreateTargetMachine { triple: SmallCStr::from(triple) })     } 
compiler/rustc_codegen_llvm/src/back/write.rs3 + / 9
@@ -100,17 +100,12 @@ fn write_output_file<'ll>(     result.into_result().unwrap_or_else(|()| llvm_err(dcx, LlvmError::WriteOutput { path: output })) } -/// If `for_cfg` is `true` then we are creating this machine for the purpose of populating-/// [`rustc_codegen_ssa::TargetConfig`] based on what LLVM actually enables in this configuration.-/// `-Ctarget-feature` should be ignored in that case since it is already processed separately.-pub(crate) fn create_informational_target_machine(-    sess: &Session,-    for_cfg: bool,-) -> OwnedTargetMachine {+pub(crate) fn create_informational_target_machine(sess: &Session) -> OwnedTargetMachine {     let config = TargetMachineFactoryConfig { split_dwarf_file: None, output_obj_file: None };     // Can't use query system here quite yet because this function is invoked before the query     // system/tcx is set up.-    let features = llvm_util::global_llvm_features(sess, for_cfg);+    let features = llvm_util::global_llvm_features(sess, /* for_cfg */ false);+     target_machine_factory(sess, config::OptLevel::No, &features)(sess.dcx(), config) } @@ -212,7 +207,6 @@ pub(crate) fn target_machine_factory(      let code_model = to_llvm_code_model(sess.code_model()); -    // This is used to set cfg_has_threads, so all logic must be in this method.     let singlethread = sess.target.singlethread(&sess.internal_target_features);      let triple = SmallCStr::new(&versioned_llvm_target(sess));
compiler/rustc_codegen_llvm/src/context.rs1 + / 1
@@ -228,7 +228,7 @@ pub(crate) unsafe fn create_module<'ll>(      // Ensure the data-layout values hardcoded remain the defaults.     {-        let tm = crate::back::write::create_informational_target_machine(sess, false);+        let tm = crate::back::write::create_informational_target_machine(sess);         unsafe {             llvm::LLVMRustSetDataLayoutFromTargetMachine(llmod, tm.raw());         }
compiler/rustc_codegen_llvm/src/diagnostics.rs5 + / 0
@@ -119,6 +119,8 @@ pub(crate) enum LlvmError<'a> {     WriteOutput { path: &'a Path },     #[diag("could not create LLVM TargetMachine for triple: {$triple}")]     CreateTargetMachine { triple: SmallCStr },+    #[diag("could not create LLVM MCSubtargetInfo for triple: {$triple}")]+    CreateMCSubtargetInfo { triple: SmallCStr },     #[diag("failed to run LLVM passes")]     RunLlvmPasses,     #[diag("failed to write LLVM IR to {$path}")]@@ -145,6 +147,9 @@ impl<G: EmissionGuarantee> Diagnostic<'_, G> for WithLlvmError<'_> {             CreateTargetMachine { .. } => {                 msg!("could not create LLVM TargetMachine for triple: {$triple}: {$llvm_err}")             }+            CreateMCSubtargetInfo { .. } => {+                msg!("could not create LLVM MCSubtargetInfo for triple: {$triple}: {$llvm_err}")+            }             RunLlvmPasses => msg!("failed to run LLVM passes: {$llvm_err}"),             WriteIr { .. } => msg!("failed to write LLVM IR to {$path}: {$llvm_err}"),             PrepareThinLtoContext => {
compiler/rustc_codegen_llvm/src/lib.rs2 + / 2
@@ -247,7 +247,7 @@ impl CodegenBackend for LlvmCodegenBackend {      fn provide(&self, providers: &mut Providers) {         providers.queries.global_backend_features =-            |tcx, ()| llvm_util::global_llvm_features(tcx.sess, false)+            |tcx, ()| llvm_util::global_llvm_features(tcx.sess, /* for_cfg */ false)     }      fn print(&self, req: &PrintRequest, out: &mut String, sess: &Session) {@@ -493,7 +493,7 @@ impl ModuleLlvm {             ModuleLlvm {                 llmod_raw,                 llcx,-                tm: ManuallyDrop::new(create_informational_target_machine(tcx.sess, false)),+                tm: ManuallyDrop::new(create_informational_target_machine(tcx.sess)),             }         }     }
compiler/rustc_codegen_llvm/src/llvm/ffi.rs14 + / 1
@@ -720,6 +720,7 @@ unsafe extern "C" {     pub type TargetMachine; } unsafe extern "C" {+    pub(crate) type MCSubtargetInfo;     pub(crate) type Twine;     pub(crate) type DiagnosticInfo;     pub(crate) type SMDiagnostic;@@ -2362,7 +2363,6 @@ unsafe extern "C" {     pub(crate) fn LLVMRustWriteTypeToString(Type: &Type, s: &RustString);     pub(crate) fn LLVMRustWriteValueToString(value_ref: &Value, s: &RustString); -    pub(crate) fn LLVMRustHasFeature(T: &TargetMachine, s: *const c_char) -> bool;     pub(crate) fn LLVMRustTargetHasMnemonic(T: &TargetMachine, s: *const c_char) -> bool;      pub(crate) fn LLVMRustPrintTargetCPUs(TM: &TargetMachine, OutStr: &RustString);@@ -2404,6 +2404,19 @@ unsafe extern "C" {         LargeDataThreshold: u64,     ) -> *mut TargetMachine; +    pub(crate) fn LLVMRustCreateMCSubtargetInfo(+        TripleStr: *const c_char,+        CPU: *const c_char,+        Features: *const c_char,+    ) -> *mut MCSubtargetInfo;++    pub(crate) fn LLVMRustMCSubtargetInfoHasFeature(+        MCInfo: &MCSubtargetInfo,+        Feature: *const c_char,+    ) -> bool;++    pub(crate) fn LLVMRustDisposeMCSubtargetInfo(MCInfo: ptr::NonNull<MCSubtargetInfo>);+     pub(crate) fn LLVMRustAddLibraryInfo<'a>(         T: &TargetMachine,         PM: &PassManager<'a>,
compiler/rustc_codegen_llvm/src/llvm_util.rs20 + / 9
@@ -6,6 +6,7 @@ use std::sync::Once; use std::{ptr, slice, str};  use libc::c_int;+use rustc_codegen_ssa::back::versioned_llvm_target; use rustc_codegen_ssa::base::wants_wasm_eh; use rustc_codegen_ssa::target_features::internal_target_features; use rustc_codegen_ssa::{TargetConfig, target_features};@@ -20,7 +21,8 @@ use rustc_target::spec::{ }; use smallvec::{SmallVec, smallvec}; -use crate::back::write::create_informational_target_machine;+use crate::back::owned_mc_subtarget_info::OwnedMCSubtargetInfo;+use crate::back::write::{create_informational_target_machine, llvm_err}; use crate::{diagnostics, llvm};  static INIT: Once = Once::new();@@ -318,7 +320,14 @@ pub(crate) fn to_llvm_features<'a>(sess: &Session, s: &'a str) -> Option<LLVMFea /// /// We do not have to worry about RUSTC_SPECIFIC_FEATURES here, those are handled outside codegen. pub(crate) fn target_config(sess: &Session) -> TargetConfig {-    let target_machine = create_informational_target_machine(sess, true);+    require_inited();+    let target_features = global_llvm_features(sess, /* for_cfg */ true);++    let triple = SmallCStr::new(&versioned_llvm_target(sess));+    let cpu = SmallCStr::new(target_cpu(sess));+    let features = CString::new(target_features.join(",")).unwrap();+    let mc_subtarget_info = OwnedMCSubtargetInfo::new(&triple, &cpu, &features)+        .unwrap_or_else(|err| llvm_err(sess.dcx(), err));      let internal_target_features = internal_target_features(         sess,@@ -329,16 +338,17 @@ pub(crate) fn target_config(sess: &Session) -> TargetConfig {         },         |feature| {             // This closure determines whether the target CPU has the feature according to LLVM. We-            // do *not* consider the `-Ctarget-feature`s here, as that will be handled later in+            // do *not* consider the `-Ctarget-feature`s here (that's why we passed `for_cfg: true`+            // to `global_llvm_features` above) because that will be handled later in             // `internal_target_features`.             if let Some(feat) = to_llvm_features(sess, feature) {                 // All the LLVM features this expands to must be enabled.                 for llvm_feature in feat {                     let cstr = SmallCStr::new(llvm_feature);-                    // `LLVMRustHasFeature` is moderately expensive. On targets with many+                    // `has_feature` is moderately expensive. On targets with many                     // features (e.g. x86) these calls take a non-trivial fraction of runtime                     // when compiling very small programs.-                    if !unsafe { llvm::LLVMRustHasFeature(target_machine.raw(), cstr.as_ptr()) } {+                    if !mc_subtarget_info.has_feature(&cstr) {                         return false;                     }                 }@@ -479,7 +489,7 @@ fn llvm_target_features(tm: &llvm::TargetMachine) -> Vec<(&str, &str)> {  pub(crate) fn print(req: &PrintRequest, out: &mut String, sess: &Session) {     require_inited();-    let tm = create_informational_target_machine(sess, false);+    let tm = create_informational_target_machine(sess);     match req.kind {         PrintKind::TargetCPUs => print_target_cpus(sess, tm.raw(), out),         PrintKind::TargetFeatures => print_target_features(sess, tm.raw(), out),@@ -497,10 +507,11 @@ fn print_target_cpus(sess: &Session, tm: &llvm::TargetMachine, out: &mut String)         cpu_name: &'a str,         remark: String,     }-    // Compare CPU against current target to label the default.+    // Compare CPU against current target to label the default. Do not print it if+    // `need_explicit_cpu` is set, because in that case the concept of default makes less sense.     let target_cpu = handle_native(&sess.target.cpu);     let make_remark = |cpu_name| {-        if cpu_name == target_cpu {+        if cpu_name == target_cpu && !sess.target.need_explicit_cpu {             // FIXME(#132514): This prints the LLVM target string, which can be             // different from the Rust target string. Is that intended?             let target = &sess.target.llvm_target;@@ -776,7 +787,7 @@ pub(crate) fn tune_cpu(sess: &Session) -> Option<&str> {  pub(crate) fn target_has_mnemonic(sess: &Session, mnemonic: &str) -> bool {     require_inited();-    let tm = create_informational_target_machine(sess, false);+    let tm = create_informational_target_machine(sess);     let cstr = SmallCStr::new(mnemonic);     unsafe { llvm::LLVMRustTargetHasMnemonic(tm.raw(), cstr.as_ptr()) } }
compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp23 + / 7
@@ -91,15 +91,31 @@ extern "C" void LLVMRustTimeTraceProfilerFinish(const char *FileName) {   timeTraceProfilerCleanup(); } -extern "C" bool LLVMRustHasFeature(LLVMTargetMachineRef TM,-                                   const char *Feature) {-  TargetMachine *Target = unwrap(TM);-#if LLVM_VERSION_GE(23, 0)-  const MCSubtargetInfo &MCInfo = Target->getMCSubtargetInfo();+extern "C" MCSubtargetInfo *+LLVMRustCreateMCSubtargetInfo(const char *TripleStr, const char *CPU,+                              const char *Features) {+  std::string Error;+  auto Trip = Triple(Triple::normalize(TripleStr));+  const llvm::Target *TheTarget = TargetRegistry::lookupTarget(Trip, Error);+  if (TheTarget == nullptr) {+    LLVMRustSetLastError(Error.c_str());+    return nullptr;+  }++#if LLVM_VERSION_GE(22, 0)+  return TheTarget->createMCSubtargetInfo(Trip, CPU, Features); #else-  const MCSubtargetInfo &MCInfo = *Target->getMCSubtargetInfo();+  return TheTarget->createMCSubtargetInfo(Trip.str(), CPU, Features); #endif-  return MCInfo.checkFeatures(std::string("+") + Feature);+}++extern "C" bool LLVMRustMCSubtargetInfoHasFeature(MCSubtargetInfo *MCInfo,+                                                  const char *Feature) {+  return MCInfo->checkFeatures(std::string("+") + Feature);+}++extern "C" void LLVMRustDisposeMCSubtargetInfo(MCSubtargetInfo *MCInfo) {+  delete MCInfo; }  /// Check whether the target has a specific assembly mnemonic like `ret` or
compiler/rustc_target/src/spec/mod.rs5 + / 4
@@ -2260,11 +2260,12 @@ pub struct TargetOptions {     /// Extra arguments to pass to the external assembler (when used)     pub asm_args: StaticCow<[StaticCow<str>]>, -    /// Default CPU to pass to LLVM. Corresponds to `llc -mcpu=$cpu`. Defaults-    /// to "generic".+    /// Default CPU to pass to LLVM. Corresponds to `llc -mcpu=$cpu`. Must be a name the backend+    /// accepts. Defaults to "generic" (which some backends won't accept).     pub cpu: StaticCow<str>,-    /// Whether a cpu needs to be explicitly set.-    /// Set to true if there is no default cpu. Defaults to false.+    /// Whether a cpu needs to be explicitly set via `-Ctarget-cpu` for codegen to run. (Even if+    /// true, `cpu` is still consulted on non-codegen paths such as cfg/feature computation.)+    /// Defaults to false.     pub need_explicit_cpu: bool,     /// Whether `-Ctarget-cpu` is treated as a target modifier. If this is set     /// all crates that are linked together must have been compiled with the
compiler/rustc_target/src/spec/targets/avr_none.rs1 + / 0
@@ -14,6 +14,7 @@ pub(crate) fn target() -> Target {         pointer_width: 16,         options: TargetOptions {             c_int_width: 16,+            cpu: "avr2".into(),             exe_suffix: ".elf".into(),             linker: Some("avr-gcc".into()),             eh_frame_header: false,
tests/run-make/print-cfg/rmake.rs14 + / 1
@@ -14,7 +14,7 @@ use std::collections::HashSet; use std::iter::FromIterator; use std::path::PathBuf; -use run_make_support::{rfs, rustc};+use run_make_support::{llvm_components_contain, rfs, rustc};  struct PrintCfg {     target: &'static str,@@ -73,6 +73,19 @@ fn main() {         includes: &["target_has_threads"],         disallow: &[],     });+    // AVR is experimental, so don't assume it's supported.+    if llvm_components_contain("avr") {+        check(PrintCfg {+            target: "avr-none",+            args: &[],+            includes: &[+                "target_feature=\"addsubiw\"",+                "target_feature=\"ijmpcall\"",+                "target_feature=\"lpm\"",+            ],+            disallow: &[],+        });+    } }  fn check(PrintCfg { target, args, includes, disallow }: PrintCfg) {
tests/run-make/target-specs/rmake.rs6 + / 1
@@ -95,5 +95,10 @@ fn main() {         .crate_type("lib")         .arg("-Ctarget-cpu=generic")         .run();-    rustc().arg("-Zunstable-options").target("require-explicit-cpu").print("target-cpus").run();+    rustc()+        .arg("-Zunstable-options")+        .target("require-explicit-cpu")+        .print("target-cpus")+        .run()+        .assert_stdout_not_contains("default target CPU"); }
tests/ui/abi/avr-sram.rs15 + / 2
@@ -1,12 +1,25 @@-//@ revisions: has_sram no_sram disable_sram-//@ build-pass+//@ revisions: has_sram no_sram disable_sram default_cpu+//+//@[has_sram] build-pass //@[has_sram] compile-flags: --target avr-none -C target-cpu=atmega328p //@[has_sram] needs-llvm-components: avr+//+//@[no_sram] build-pass //@[no_sram] compile-flags: --target avr-none -C target-cpu=attiny11 //@[no_sram] needs-llvm-components: avr+//+//@[disable_sram] build-pass //@[disable_sram] compile-flags: --target avr-none -C target-cpu=atmega328p -C target-feature=-sram //@[disable_sram] needs-llvm-components: avr+//+// Note: this revision relies on `need_explicit_cpu` only being enforced at codegen, which is why+// it uses `check-pass` instead of `build-pass`.+//@[default_cpu] check-pass+//@[default_cpu] compile-flags: --target avr-none+//@[default_cpu] needs-llvm-components: avr+// //@ ignore-backends: gcc+// //[no_sram,disable_sram]~? WARN target feature `sram` must be enabled //[disable_sram]~? WARN target feature `sram` cannot be disabled with `-Ctarget-feature` 
tests/ui/codegen/custom-target-invalid-llvm-target.rs1 + / 1
@@ -7,4 +7,4 @@  fn main() {} -//~? ERROR failed to parse target machine config to target machine+//~? ERROR could not create LLVM MCSubtargetInfo for triple: not-a-real-target
tests/ui/codegen/custom-target-invalid-llvm-target.stderr1 + / 1
@@ -1,2 +1,2 @@-error: failed to parse target machine config to target machine: could not create LLVM TargetMachine for triple: not-a-real-target+error: could not create LLVM MCSubtargetInfo for triple: not-a-real-target: No available targets are compatible with triple "not-a-real-target"