-
Notifications
You must be signed in to change notification settings - Fork 800
Fix race condition in clang_macro_fallback #3369
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,9 @@ | |
| use crate::ir::context::BindgenContext; | ||
| use clang_sys::*; | ||
| use std::cmp; | ||
| use std::os::unix::ffi::OsStrExt; | ||
| use std::path::{Path, PathBuf}; | ||
| use tempfile::TempDir; | ||
|
|
||
| use std::ffi::{CStr, CString}; | ||
| use std::fmt; | ||
|
|
@@ -1822,12 +1825,12 @@ impl TranslationUnit { | |
| /// Parse a source file into a translation unit. | ||
| pub(crate) fn parse( | ||
| ix: &Index, | ||
| file: &str, | ||
| file: &Path, | ||
| cmd_args: &[Box<str>], | ||
| unsaved: &[UnsavedFile], | ||
| opts: CXTranslationUnit_Flags, | ||
| ) -> Option<TranslationUnit> { | ||
| let fname = CString::new(file).unwrap(); | ||
| let fname = path_to_cstring(file); | ||
| let _c_args: Vec<CString> = cmd_args | ||
| .iter() | ||
| .map(|s| CString::new(s.as_bytes()).unwrap()) | ||
|
|
@@ -1879,10 +1882,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 +1914,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 +1930,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 | ||
|
|
@@ -1949,6 +1952,7 @@ impl FallbackTranslationUnit { | |
| CXTranslationUnit_None, | ||
| )?; | ||
| Some(FallbackTranslationUnit { | ||
| temp_dir, | ||
| file_path: file, | ||
| pch_path, | ||
| tu: f_translation_unit, | ||
|
|
@@ -1985,13 +1989,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 +2029,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(), | ||
|
|
@@ -2447,3 +2444,7 @@ impl TargetInfo { | |
| } | ||
| } | ||
| } | ||
|
|
||
| fn path_to_cstring(path: &Path) -> CString { | ||
| CString::new(path.as_os_str().as_bytes()).unwrap() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah this probably needs something else on windows right?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Btw shouldn't a windows CI be there to catch this? |
||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably needs to be
#[cfg]d to build on windows right?View changes since the review