Fix race condition in clang_macro_fallback

Author: HKalbasi

Cargo.lock

@@ -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"]

bindgen/Cargo.toml

@@ -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<str>],
         unsaved: &[UnsavedFile],
         opts: CXTranslationUnit_Flags,
     ) -> Option<TranslationUnit> {
-        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<CString> = 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<Index>,
     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<str>],
     ) -> Option<Self> {
         // 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()
+}

bindgen/clang.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()

bindgen/ir/context.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::<Vec<_>>();
 

bindgen/lib.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<PathBuf>,
     /// 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,

bindgen/options/cli.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<PathBuf> {
-        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<P: AsRef<Path>>(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",
-    }
 }