Skip to content

Commit

Permalink
Fix race condition in clang_macro_fallback
Browse files Browse the repository at this point in the history
  • Loading branch information
HKalbasi committed Feb 2, 2025
1 parent 03d49b6 commit b002407
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 67 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions bindgen/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
50 changes: 26 additions & 24 deletions bindgen/clang.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -1913,9 +1916,10 @@ impl Drop for TranslationUnit {

/// Translation unit used for macro fallback parsing
pub(crate) struct FallbackTranslationUnit {
file_path: String,
header_path: String,
pch_path: String,
temp_dir: TempDir,
file_path: PathBuf,
header_path: PathBuf,
pch_path: PathBuf,
idx: Box<Index>,
tu: TranslationUnit,
}
Expand All @@ -1929,9 +1933,10 @@ impl fmt::Debug for FallbackTranslationUnit {
impl FallbackTranslationUnit {
/// Create a new fallback translation unit
pub(crate) fn new(
file: String,
header_path: String,
pch_path: String,
temp_dir: TempDir,
file: PathBuf,
header_path: PathBuf,
pch_path: PathBuf,
c_args: &[Box<str>],
) -> Option<Self> {
// Create empty file
Expand All @@ -1945,12 +1950,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,
header_path,
pch_path,
Expand Down Expand Up @@ -1988,14 +1994,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.header_path);
let _ = std::fs::remove_file(&self.pch_path);
}
}

/// A diagnostic message generated while parsing a translation unit.
pub(crate) struct Diagnostic {
x: CXDiagnostic,
Expand Down Expand Up @@ -2036,9 +2034,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(),
Expand Down Expand Up @@ -2450,3 +2448,7 @@ impl TargetInfo {
}
}
}

fn path_to_cstring(path: &Path) -> CString {
CString::new(path.to_string_lossy().as_bytes()).unwrap()
}
33 changes: 14 additions & 19 deletions bindgen/ir/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use std::fs::OpenOptions;
use std::io::Write;
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)]
Expand Down Expand Up @@ -557,7 +558,7 @@ impl BindgenContext {

clang::TranslationUnit::parse(
&index,
"",
None,
&options.clang_args,
input_unsaved_files,
parse_options,
Expand Down Expand Up @@ -2042,13 +2043,9 @@ 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);

Expand All @@ -2072,15 +2069,12 @@ If you encounter an error missing from this list, please file an issue or a PR!"
header_contents +=
format!("\n#include <{header_name}>").as_str();
}
let header_to_precompile = 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"
);
let pch = header_to_precompile.clone() + ".pch";
let header_to_precompile = temp_dir
.path()
.join(header_names_to_compile.join("-") + "-precompile.h");
let pch = temp_dir
.path()
.join(header_names_to_compile.join("-") + "-precompile.h.pch");

let mut header_to_precompile_file = OpenOptions::new()
.create(true)
Expand Down Expand Up @@ -2110,7 +2104,7 @@ If you encounter an error missing from this list, please file an issue or a PR!"
);
let mut tu = clang::TranslationUnit::parse(
&index,
&header_to_precompile,
Some(&header_to_precompile),
&c_args,
&[],
clang_sys::CXTranslationUnit_ForSerialization,
Expand All @@ -2119,7 +2113,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(),
];
c_args.extend(
self.options
Expand All @@ -2133,6 +2127,7 @@ If you encounter an error missing from this list, please file an issue or a PR!"
.cloned(),
);
self.fallback_tu = Some(clang::FallbackTranslationUnit::new(
temp_dir,
file,
header_to_precompile,
pch,
Expand Down
2 changes: 1 addition & 1 deletion bindgen/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,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<_>>();

Expand Down
5 changes: 0 additions & 5 deletions bindgen/options/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
18 changes: 0 additions & 18 deletions bindgen/options/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2148,22 +2148,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",
}
}

0 comments on commit b002407

Please sign in to comment.