-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Make backtraces work on Windows GNU targets again. #39234
Merged
bors
merged 4 commits into
rust-lang:master
from
segevfiner:fix-backtraces-on-windows-gnu
Jan 28, 2017
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
4186037
Make backtraces work on Windows GNU targets again.
segevfiner 450554e
Attempt at fixing dead code lints
segevfiner 1b4a6c8
Use libc::c_char instead of i8 due to platforms with unsigned char
segevfiner ab21314
Disable backtrace tests on i686-pc-windows-gnu since it's broken by FPO
segevfiner File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT | ||
// file at the top-level directory of this distribution and at | ||
// http://rust-lang.org/COPYRIGHT. | ||
// | ||
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or | ||
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license | ||
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your | ||
// option. This file may not be copied, modified, or distributed | ||
// except according to those terms. | ||
|
||
use io; | ||
use sys::c; | ||
use libc::c_char; | ||
use path::PathBuf; | ||
use fs::{OpenOptions, File}; | ||
use sys::ext::fs::OpenOptionsExt; | ||
use sys::handle::Handle; | ||
use super::super::{fill_utf16_buf, os2path, to_u16s, wide_char_to_multi_byte}; | ||
|
||
fn query_full_process_image_name() -> io::Result<PathBuf> { | ||
unsafe { | ||
let process_handle = Handle::new(c::OpenProcess(c::PROCESS_QUERY_INFORMATION, | ||
c::FALSE, | ||
c::GetCurrentProcessId())); | ||
fill_utf16_buf(|buf, mut sz| { | ||
if c::QueryFullProcessImageNameW(process_handle.raw(), 0, buf, &mut sz) == 0 { | ||
0 | ||
} else { | ||
sz | ||
} | ||
}, os2path) | ||
} | ||
} | ||
|
||
fn lock_and_get_executable_filename() -> io::Result<(PathBuf, File)> { | ||
// We query the current image name, open the file without FILE_SHARE_DELETE so it | ||
// can't be moved and then get the current image name again. If the names are the | ||
// same than we have successfully locked the file | ||
let image_name1 = query_full_process_image_name()?; | ||
let file = OpenOptions::new() | ||
.read(true) | ||
.share_mode(c::FILE_SHARE_READ | c::FILE_SHARE_WRITE) | ||
.open(&image_name1)?; | ||
let image_name2 = query_full_process_image_name()?; | ||
|
||
if image_name1 != image_name2 { | ||
return Err(io::Error::new(io::ErrorKind::Other, | ||
"executable moved while trying to lock it")); | ||
} | ||
|
||
Ok((image_name1, file)) | ||
} | ||
|
||
// Get the executable filename for libbacktrace | ||
// This returns the path in the ANSI code page and a File which should remain open | ||
// for as long as the path should remain valid | ||
pub fn get_executable_filename() -> io::Result<(Vec<c_char>, File)> { | ||
let (executable, file) = lock_and_get_executable_filename()?; | ||
let u16_executable = to_u16s(executable.into_os_string())?; | ||
Ok((wide_char_to_multi_byte(c::CP_ACP, c::WC_NO_BEST_FIT_CHARS, | ||
&u16_executable, true)?, file)) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
You need to validate that the reverse conversion with
MultiByteToWideChar
gives the originalu16
string, otherwise the conversion was lossy and libbacktrace will try to open some other file.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.
I used the
lpUsedDefaultChar
parameter ofWideCharToMultiByte
, it can save the need to convert twice assuming that it's not broken in some way.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.
Actually there's still ways in which data loss can occur other than default char replacement. You'll want to pass
WC_NO_BEST_FIT_CHARS
.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.
Ideally the set of flags should mirror whatever is used inside of (most probably)
CreateFileA
which is used inside posixopen
in CRT which is used in libbacktrace.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.
The function which is actually used to convert strings inside
A
functions isRtlAnsiStringToUnicodeString
which in turn callsRtlMultiByteToUnicodeN
, notMultiByteToWideChar
.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.
Also another point is that we're going in the opposite direction here, so we can't match the flags anyway.
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.
Ok, if we do want to "fix" this, then this will work 😄
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.
MSDN documents that
WC_NO_BEST_FIT_CHARS
should actually make the conversion reversible, which is what you wanted (assuming no surprises). So I added it as suggested.