Skip to content

Fix race condition in clang_macro_fallback #3111

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
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
45 changes: 24 additions & 21 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,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,
}
Expand All @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -2446,3 +2445,7 @@ impl TargetInfo {
}
}
}

fn path_to_cstring(path: &Path) -> CString {
CString::new(path.to_string_lossy().as_bytes()).unwrap()
}
37 changes: 16 additions & 21 deletions bindgen/ir/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -555,7 +556,7 @@ impl BindgenContext {

clang::TranslationUnit::parse(
&index,
"",
None,
&options.clang_args,
input_unsaved_files,
parse_options,
Expand Down Expand Up @@ -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]
{
Expand All @@ -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());
Expand All @@ -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,
Expand All @@ -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() {
Expand All @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion bindgen/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<_>>();

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 @@ -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",
}
}