From 6927cf50c32ca2078d2dc008e13445a7699f66c1 Mon Sep 17 00:00:00 2001 From: hkalbasi Date: Mon, 10 Feb 2025 20:36:27 +0330 Subject: [PATCH] Fix race condition in clang_macro_fallback --- Cargo.lock | 1 + bindgen/Cargo.toml | 1 + bindgen/clang.rs | 45 ++++++++++++++++++++++-------------------- bindgen/ir/context.rs | 37 +++++++++++++++------------------- bindgen/lib.rs | 2 +- bindgen/options/cli.rs | 5 ----- bindgen/options/mod.rs | 18 ----------------- 7 files changed, 43 insertions(+), 66 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4e88c224f5..4827e15da6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -52,6 +52,7 @@ dependencies = [ "rustc-hash", "shlex", "syn 2.0.90", + "tempfile", ] [[package]] diff --git a/bindgen/Cargo.toml b/bindgen/Cargo.toml index c01f8f0c44..7cb5f0a506 100644 --- a/bindgen/Cargo.toml +++ b/bindgen/Cargo.toml @@ -41,6 +41,7 @@ regex = { workspace = true, features = ["std", "unicode-perl"] } rustc-hash.workspace = true shlex.workspace = true syn = { workspace = true, features = ["full", "extra-traits", "visit-mut"] } +tempfile.workspace = true [features] default = ["logging", "prettyplease", "runtime"] diff --git a/bindgen/clang.rs b/bindgen/clang.rs index 04fe3e1538..2f3a145aa0 100644 --- a/bindgen/clang.rs +++ b/bindgen/clang.rs @@ -7,6 +7,8 @@ use crate::ir::context::BindgenContext; use clang_sys::*; use std::cmp; +use std::path::{Path, PathBuf}; +use tempfile::TempDir; use std::ffi::{CStr, CString}; use std::fmt; @@ -1822,12 +1824,15 @@ impl TranslationUnit { /// Parse a source file into a translation unit. pub(crate) fn parse( ix: &Index, - file: &str, + file: Option<&Path>, cmd_args: &[Box], unsaved: &[UnsavedFile], opts: CXTranslationUnit_Flags, ) -> Option { - let fname = CString::new(file).unwrap(); + let fname = match file { + Some(file) => path_to_cstring(file), + None => CString::new(vec![]).unwrap(), + }; let _c_args: Vec = cmd_args .iter() .map(|s| CString::new(s.as_bytes()).unwrap()) @@ -1879,10 +1884,8 @@ impl TranslationUnit { } /// Save a translation unit to the given file. - pub(crate) fn save(&mut self, file: &str) -> Result<(), CXSaveError> { - let Ok(file) = CString::new(file) else { - return Err(CXSaveError_Unknown); - }; + pub(crate) fn save(&mut self, file: &Path) -> Result<(), CXSaveError> { + let file = path_to_cstring(file); let ret = unsafe { clang_saveTranslationUnit( self.x, @@ -1913,8 +1916,9 @@ impl Drop for TranslationUnit { /// Translation unit used for macro fallback parsing pub(crate) struct FallbackTranslationUnit { - file_path: String, - pch_path: String, + temp_dir: TempDir, + file_path: PathBuf, + pch_path: PathBuf, idx: Box, tu: TranslationUnit, } @@ -1928,8 +1932,9 @@ impl fmt::Debug for FallbackTranslationUnit { impl FallbackTranslationUnit { /// Create a new fallback translation unit pub(crate) fn new( - file: String, - pch_path: String, + temp_dir: TempDir, + file: PathBuf, + pch_path: PathBuf, c_args: &[Box], ) -> Option { // Create empty file @@ -1943,12 +1948,13 @@ impl FallbackTranslationUnit { let f_index = Box::new(Index::new(true, false)); let f_translation_unit = TranslationUnit::parse( &f_index, - &file, + Some(&file), c_args, &[], CXTranslationUnit_None, )?; Some(FallbackTranslationUnit { + temp_dir, file_path: file, pch_path, tu: f_translation_unit, @@ -1985,13 +1991,6 @@ impl FallbackTranslationUnit { } } -impl Drop for FallbackTranslationUnit { - fn drop(&mut self) { - let _ = std::fs::remove_file(&self.file_path); - let _ = std::fs::remove_file(&self.pch_path); - } -} - /// A diagnostic message generated while parsing a translation unit. pub(crate) struct Diagnostic { x: CXDiagnostic, @@ -2032,9 +2031,9 @@ pub(crate) struct UnsavedFile { } impl UnsavedFile { - /// Construct a new unsaved file with the given `name` and `contents`. - pub(crate) fn new(name: &str, contents: &str) -> UnsavedFile { - let name = CString::new(name.as_bytes()).unwrap(); + /// Construct a new unsaved file with the given `path` and `contents`. + pub(crate) fn new(path: &Path, contents: &str) -> UnsavedFile { + let name = path_to_cstring(path); let contents = CString::new(contents.as_bytes()).unwrap(); let x = CXUnsavedFile { Filename: name.as_ptr(), @@ -2446,3 +2445,7 @@ impl TargetInfo { } } } + +fn path_to_cstring(path: &Path) -> CString { + CString::new(path.to_string_lossy().as_bytes()).unwrap() +} diff --git a/bindgen/ir/context.rs b/bindgen/ir/context.rs index 78790d61c4..3ee7b10d69 100644 --- a/bindgen/ir/context.rs +++ b/bindgen/ir/context.rs @@ -31,6 +31,7 @@ use std::cell::{Cell, RefCell}; use std::collections::{BTreeSet, HashMap as StdHashMap}; use std::mem; use std::path::Path; +use tempfile::TempDir; /// An identifier for some kind of IR item. #[derive(Debug, Copy, Clone, Eq, PartialOrd, Ord, Hash)] @@ -555,7 +556,7 @@ impl BindgenContext { clang::TranslationUnit::parse( &index, - "", + None, &options.clang_args, input_unsaved_files, parse_options, @@ -2040,20 +2041,18 @@ If you encounter an error missing from this list, please file an issue or a PR!" &mut self, ) -> Option<&mut clang::FallbackTranslationUnit> { if self.fallback_tu.is_none() { - let file = format!( - "{}/.macro_eval.c", - match self.options().clang_macro_fallback_build_dir { - Some(ref path) => path.as_os_str().to_str()?, - None => ".", - } - ); + let temp_dir = TempDir::new().unwrap(); + + let file = temp_dir.path().join(".macro_eval.c"); let index = clang::Index::new(false, false); let mut header_names_to_compile = Vec::new(); let mut header_paths = Vec::new(); let mut header_includes = Vec::new(); - let single_header = self.options().input_headers.last().cloned()?; + let single_header = + Path::new(&self.options().input_headers.last()?.as_ref()) + .to_owned(); for input_header in &self.options.input_headers [..self.options.input_headers.len() - 1] { @@ -2072,14 +2071,9 @@ If you encounter an error missing from this list, please file an issue or a PR!" header_names_to_compile .push(header_name.split(".h").next()?.to_string()); } - let pch = format!( - "{}/{}", - match self.options().clang_macro_fallback_build_dir { - Some(ref path) => path.as_os_str().to_str()?, - None => ".", - }, - header_names_to_compile.join("-") + "-precompile.h.pch" - ); + let pch = temp_dir + .path() + .join(header_names_to_compile.join("-") + "-precompile.h.pch"); let mut c_args = self.options.fallback_clang_args.clone(); c_args.push("-x".to_string().into_boxed_str()); @@ -2093,7 +2087,7 @@ If you encounter an error missing from this list, please file an issue or a PR!" } let mut tu = clang::TranslationUnit::parse( &index, - &single_header, + Some(&single_header), &c_args, &[], clang_sys::CXTranslationUnit_ForSerialization, @@ -2102,7 +2096,7 @@ If you encounter an error missing from this list, please file an issue or a PR!" let mut c_args = vec![ "-include-pch".to_string().into_boxed_str(), - pch.clone().into_boxed_str(), + pch.to_string_lossy().into_owned().into_boxed_str(), ]; let mut skip_next = false; for arg in self.options.fallback_clang_args.iter() { @@ -2114,8 +2108,9 @@ If you encounter an error missing from this list, please file an issue or a PR!" c_args.push(arg.clone()) } } - self.fallback_tu = - Some(clang::FallbackTranslationUnit::new(file, pch, &c_args)?); + self.fallback_tu = Some(clang::FallbackTranslationUnit::new( + temp_dir, file, pch, &c_args, + )?); } self.fallback_tu.as_mut() diff --git a/bindgen/lib.rs b/bindgen/lib.rs index 1a15d51d67..dfff8004f1 100644 --- a/bindgen/lib.rs +++ b/bindgen/lib.rs @@ -371,7 +371,7 @@ impl Builder { std::mem::take(&mut self.options.input_header_contents) .into_iter() .map(|(name, contents)| { - clang::UnsavedFile::new(name.as_ref(), contents.as_ref()) + clang::UnsavedFile::new((*name).as_ref(), contents.as_ref()) }) .collect::>(); diff --git a/bindgen/options/cli.rs b/bindgen/options/cli.rs index 8c4c05bc84..91bfd3cd6b 100644 --- a/bindgen/options/cli.rs +++ b/bindgen/options/cli.rs @@ -459,9 +459,6 @@ struct BindgenCommand { /// Enable fallback for clang macro parsing. #[arg(long)] clang_macro_fallback: bool, - /// Set path for temporary files generated by fallback for clang macro parsing. - #[arg(long)] - clang_macro_fallback_build_dir: Option, /// Use DSTs to represent structures with flexible array members. #[arg(long)] flexarray_dst: bool, @@ -635,7 +632,6 @@ where override_abi, wrap_unsafe_ops, clang_macro_fallback, - clang_macro_fallback_build_dir, flexarray_dst, with_derive_custom, with_derive_custom_struct, @@ -932,7 +928,6 @@ where override_abi => |b, (abi, regex)| b.override_abi(abi, regex), wrap_unsafe_ops, clang_macro_fallback => |b, _| b.clang_macro_fallback(), - clang_macro_fallback_build_dir, flexarray_dst, wrap_static_fns, wrap_static_fns_path, diff --git a/bindgen/options/mod.rs b/bindgen/options/mod.rs index 9d1d195980..9e455ec21f 100644 --- a/bindgen/options/mod.rs +++ b/bindgen/options/mod.rs @@ -2153,22 +2153,4 @@ options! { }, as_args: "--clang-macro-fallback", } - /// Path to use for temporary files created by clang macro fallback code like precompiled - /// headers. - clang_macro_fallback_build_dir: Option { - methods: { - /// Set a path to a directory to which `.c` and `.h.pch` files should be written for the - /// purpose of using clang to evaluate macros that can't be easily parsed. - /// - /// The default location for `.h.pch` files is the directory that the corresponding - /// `.h` file is located in. The default for the temporary `.c` file used for clang - /// parsing is the current working directory. Both of these defaults are overridden - /// by this option. - pub fn clang_macro_fallback_build_dir>(mut self, path: P) -> Self { - self.options.clang_macro_fallback_build_dir = Some(path.as_ref().to_owned()); - self - } - }, - as_args: "--clang-macro-fallback-build-dir", - } }