-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Fix for raw-dylib when linking with MinGW BFD #88801
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
//! A helper class for dealing with static archives | ||
|
||
use std::ffi::{CStr, CString}; | ||
use std::env; | ||
use std::ffi::{CStr, CString, OsString}; | ||
use std::io; | ||
use std::mem; | ||
use std::path::{Path, PathBuf}; | ||
|
@@ -158,55 +159,102 @@ impl<'a> ArchiveBuilder<'a> for LlvmArchiveBuilder<'a> { | |
output_path.with_extension("lib") | ||
}; | ||
|
||
// we've checked for \0 characters in the library name already | ||
let dll_name_z = CString::new(lib_name).unwrap(); | ||
// BFD doesn't work properly with short imports | ||
let mingw_gnu_toolchain = self.config.sess.target.llvm_target.ends_with("pc-windows-gnu"); | ||
|
||
// All import names are Rust identifiers and therefore cannot contain \0 characters. | ||
// FIXME: when support for #[link_name] implemented, ensure that import.name values don't | ||
// have any \0 characters | ||
let import_name_vector: Vec<CString> = dll_imports | ||
.iter() | ||
.map(|import: &DllImport| { | ||
if self.config.sess.target.arch == "x86" { | ||
LlvmArchiveBuilder::i686_decorated_name(import) | ||
LlvmArchiveBuilder::i686_decorated_name(import, mingw_gnu_toolchain) | ||
} else { | ||
CString::new(import.name.to_string()).unwrap() | ||
} | ||
}) | ||
.collect(); | ||
|
||
let output_path_z = rustc_fs_util::path_to_c_string(&output_path); | ||
if mingw_gnu_toolchain { | ||
let mut def_file_path = tmpdir.as_ref().to_path_buf(); | ||
def_file_path.push(format!("{}_imports", lib_name)); | ||
def_file_path.with_extension("def"); | ||
|
||
let def_file_content = format!( | ||
"EXPORTS\n{}", | ||
import_name_vector | ||
.iter() | ||
.map(|cstring| cstring.to_str().unwrap()) | ||
.intersperse("\n") | ||
.collect::<String>() | ||
); | ||
std::fs::write(&def_file_path, def_file_content).unwrap(); | ||
|
||
let dlltool = find_dlltool(self.config.sess); | ||
let result = std::process::Command::new(dlltool) | ||
.args([ | ||
"-d", | ||
def_file_path.to_str().unwrap(), | ||
"-D", | ||
lib_name, | ||
"-l", | ||
output_path.to_str().unwrap(), | ||
]) | ||
.output(); | ||
|
||
match result { | ||
Err(e) => { | ||
self.config.sess.fatal(&format!("Error calling dlltool: {}", e.to_string())) | ||
} | ||
Ok(output) if !output.status.success() => self.config.sess.fatal(&format!( | ||
"Dlltool could not create import library: {}\n{}", | ||
String::from_utf8_lossy(&output.stdout), | ||
String::from_utf8_lossy(&output.stderr) | ||
)), | ||
_ => {} | ||
} | ||
} else { | ||
// we've checked for \0 characters in the library name already | ||
let dll_name_z = CString::new(lib_name).unwrap(); | ||
let output_path_z = rustc_fs_util::path_to_c_string(&output_path); | ||
|
||
tracing::trace!("invoking LLVMRustWriteImportLibrary"); | ||
tracing::trace!(" dll_name {:#?}", dll_name_z); | ||
tracing::trace!(" output_path {}", output_path.display()); | ||
tracing::trace!( | ||
" import names: {}", | ||
dll_imports | ||
.iter() | ||
.map(|import| import.name.to_string()) | ||
.collect::<Vec<_>>() | ||
.join(", "), | ||
); | ||
|
||
tracing::trace!("invoking LLVMRustWriteImportLibrary"); | ||
tracing::trace!(" dll_name {:#?}", dll_name_z); | ||
tracing::trace!(" output_path {}", output_path.display()); | ||
tracing::trace!( | ||
" import names: {}", | ||
dll_imports.iter().map(|import| import.name.to_string()).collect::<Vec<_>>().join(", "), | ||
); | ||
let ffi_exports: Vec<LLVMRustCOFFShortExport> = import_name_vector | ||
.iter() | ||
.map(|name_z| LLVMRustCOFFShortExport::from_name(name_z.as_ptr())) | ||
.collect(); | ||
let result = unsafe { | ||
crate::llvm::LLVMRustWriteImportLibrary( | ||
dll_name_z.as_ptr(), | ||
output_path_z.as_ptr(), | ||
ffi_exports.as_ptr(), | ||
ffi_exports.len(), | ||
llvm_machine_type(&self.config.sess.target.arch) as u16, | ||
!self.config.sess.target.is_like_msvc, | ||
) | ||
}; | ||
|
||
let ffi_exports: Vec<LLVMRustCOFFShortExport> = import_name_vector | ||
.iter() | ||
.map(|name_z| LLVMRustCOFFShortExport::from_name(name_z.as_ptr())) | ||
.collect(); | ||
let result = unsafe { | ||
crate::llvm::LLVMRustWriteImportLibrary( | ||
dll_name_z.as_ptr(), | ||
output_path_z.as_ptr(), | ||
ffi_exports.as_ptr(), | ||
ffi_exports.len(), | ||
llvm_machine_type(&self.config.sess.target.arch) as u16, | ||
!self.config.sess.target.is_like_msvc, | ||
) | ||
if result == crate::llvm::LLVMRustResult::Failure { | ||
self.config.sess.fatal(&format!( | ||
"Error creating import library for {}: {}", | ||
lib_name, | ||
llvm::last_error().unwrap_or("unknown LLVM error".to_string()) | ||
)); | ||
} | ||
}; | ||
|
||
if result == crate::llvm::LLVMRustResult::Failure { | ||
self.config.sess.fatal(&format!( | ||
"Error creating import library for {}: {}", | ||
lib_name, | ||
llvm::last_error().unwrap_or("unknown LLVM error".to_string()) | ||
)); | ||
} | ||
|
||
self.add_archive(&output_path, |_| false).unwrap_or_else(|e| { | ||
self.config.sess.fatal(&format!( | ||
"failed to add native library {}: {}", | ||
|
@@ -332,13 +380,17 @@ impl<'a> LlvmArchiveBuilder<'a> { | |
} | ||
} | ||
|
||
fn i686_decorated_name(import: &DllImport) -> CString { | ||
fn i686_decorated_name(import: &DllImport, mingw: bool) -> CString { | ||
let name = import.name; | ||
// MinGW doesn't want `_` prefix for `.def` file | ||
let prefix = if mingw { "" } else { "_" }; | ||
// We verified during construction that `name` does not contain any NULL characters, so the | ||
// conversion to CString is guaranteed to succeed. | ||
CString::new(match import.calling_convention { | ||
DllCallingConvention::C => format!("_{}", name), | ||
DllCallingConvention::Stdcall(arg_list_size) => format!("_{}@{}", name, arg_list_size), | ||
DllCallingConvention::C => format!("{}{}", prefix, name), | ||
DllCallingConvention::Stdcall(arg_list_size) => { | ||
format!("{}{}@{}", prefix, name, arg_list_size) | ||
} | ||
DllCallingConvention::Fastcall(arg_list_size) => format!("@{}@{}", name, arg_list_size), | ||
DllCallingConvention::Vectorcall(arg_list_size) => { | ||
format!("{}@@{}", name, arg_list_size) | ||
|
@@ -351,3 +403,26 @@ impl<'a> LlvmArchiveBuilder<'a> { | |
fn string_to_io_error(s: String) -> io::Error { | ||
io::Error::new(io::ErrorKind::Other, format!("bad archive: {}", s)) | ||
} | ||
|
||
fn find_dlltool(sess: &Session) -> OsString { | ||
// When cross-compiling first try binary prefixed with target triple | ||
if sess.host.llvm_target != sess.target.llvm_target { | ||
let prefixed_dlltool = if sess.target.arch == "x86" { | ||
"i686-w64-mingw32-dlltool" | ||
} else { | ||
"x86_64-w64-mingw32-dlltool" | ||
}; | ||
let prefixed_dlltool = if cfg!(windows) { | ||
[prefixed_dlltool, "exe"].concat() | ||
} else { | ||
prefixed_dlltool.to_string() | ||
}; | ||
for dir in env::split_paths(&env::var_os("PATH").unwrap_or_default()) { | ||
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. This should also either verify that the binary is executable at all in some way or provide a way to override this detection. Right now if the system is borked enough to have a non-working 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. TBH the more I think about this approach the more hacky it seems. |
||
let full_path = dir.join(&prefixed_dlltool); | ||
if full_path.is_file() { | ||
return full_path.into_os_string(); | ||
} | ||
} | ||
} | ||
OsString::from("dlltool") | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,12 @@ | ||
# Test the behavior of #[link(.., kind = "raw-dylib")] with alternative calling conventions. | ||
|
||
# only-i686-pc-windows-msvc | ||
# only-i686-pc-windows-gnu | ||
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. These tests are supposed to eventually test both architectures, right? 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, I just didn't want to make spend time on proper fix yet. |
||
|
||
-include ../../run-make-fulldeps/tools.mk | ||
|
||
all: | ||
$(call COMPILE_OBJ,"$(TMPDIR)"/extern.obj,extern.c) | ||
$(CC) "$(TMPDIR)"/extern.obj -link -dll -out:"$(TMPDIR)"/extern.dll | ||
$(CC) "$(TMPDIR)"/extern.obj -shared -o "$(TMPDIR)"/extern.dll | ||
$(RUSTC) --crate-type lib --crate-name raw_dylib_alt_calling_convention_test lib.rs | ||
$(RUSTC) --crate-type bin driver.rs -L "$(TMPDIR)" | ||
"$(TMPDIR)"/driver > "$(TMPDIR)"/output.txt | ||
|
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.
llvm_target
is probably not the best way to check for this. There are many llvm target triples that are actually equivalent in the end. (e.g.i686-pc-windows-gnu
andi686-unknown-windows-gnu
and e.g. json targets could specify either).