Consider all fields when comparing DllImports, to remove nondetermininsm in multiple-definitions test

This commit is contained in:
Richard Cobbe 2021-07-12 12:46:27 -07:00
parent b5a2ccee81
commit ce59f1aac5
7 changed files with 91 additions and 75 deletions

View File

@ -221,9 +221,7 @@ impl CodegenBackend for CraneliftCodegenBackend {
sess,
&codegen_results,
outputs,
);
Ok(())
)
}
}

View File

@ -292,9 +292,7 @@ impl CodegenBackend for LlvmCodegenBackend {
// Run the linker on any artifacts that resulted from the LLVM run.
// This should produce either a finished executable or library.
link_binary::<LlvmArchiveBuilder<'_>>(sess, &codegen_results, outputs);
Ok(())
link_binary::<LlvmArchiveBuilder<'_>>(sess, &codegen_results, outputs)
}
}

View File

@ -1,9 +1,9 @@
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
use rustc_data_structures::temp_dir::MaybeTempDir;
use rustc_errors::Handler;
use rustc_errors::{ErrorReported, Handler};
use rustc_fs_util::fix_windows_verbatim_for_gcc;
use rustc_hir::def_id::CrateNum;
use rustc_middle::middle::cstore::{DllCallingConvention, DllImport};
use rustc_middle::middle::cstore::DllImport;
use rustc_middle::middle::dependency_format::Linkage;
use rustc_session::config::{self, CFGuard, CrateType, DebugInfo, LdImpl, Strip};
use rustc_session::config::{OutputFilenames, OutputType, PrintRequest};
@ -35,7 +35,6 @@ use object::{Architecture, BinaryFormat, Endianness, FileFlags, SectionFlags, Se
use tempfile::Builder as TempFileBuilder;
use std::ffi::OsString;
use std::iter::FromIterator;
use std::path::{Path, PathBuf};
use std::process::{ExitStatus, Output, Stdio};
use std::{ascii, char, env, fmt, fs, io, mem, str};
@ -54,7 +53,7 @@ pub fn link_binary<'a, B: ArchiveBuilder<'a>>(
sess: &'a Session,
codegen_results: &CodegenResults,
outputs: &OutputFilenames,
) {
) -> Result<(), ErrorReported> {
let _timer = sess.timer("link_binary");
let output_metadata = sess.opts.output_types.contains_key(&OutputType::Metadata);
for &crate_type in sess.crate_types().iter() {
@ -95,11 +94,17 @@ pub fn link_binary<'a, B: ArchiveBuilder<'a>>(
match crate_type {
CrateType::Rlib => {
let _timer = sess.timer("link_rlib");
link_rlib::<B>(sess, codegen_results, RlibFlavor::Normal, &out_filename, &path)
.build();
link_rlib::<B>(
sess,
codegen_results,
RlibFlavor::Normal,
&out_filename,
&path,
)?
.build();
}
CrateType::Staticlib => {
link_staticlib::<B>(sess, codegen_results, &out_filename, &path);
link_staticlib::<B>(sess, codegen_results, &out_filename, &path)?;
}
_ => {
link_natively::<B>(
@ -145,6 +150,8 @@ pub fn link_binary<'a, B: ArchiveBuilder<'a>>(
}
}
});
Ok(())
}
pub fn each_linked_rlib(
@ -220,7 +227,7 @@ fn link_rlib<'a, B: ArchiveBuilder<'a>>(
flavor: RlibFlavor,
out_filename: &Path,
tmpdir: &MaybeTempDir,
) -> B {
) -> Result<B, ErrorReported> {
info!("preparing rlib to {:?}", out_filename);
let mut ab = <B as ArchiveBuilder>::new(sess, out_filename, None);
@ -259,7 +266,7 @@ fn link_rlib<'a, B: ArchiveBuilder<'a>>(
}
for (raw_dylib_name, raw_dylib_imports) in
collate_raw_dylibs(sess, &codegen_results.crate_info.used_libraries)
collate_raw_dylibs(sess, &codegen_results.crate_info.used_libraries)?
{
ab.inject_dll_import_lib(&raw_dylib_name, &raw_dylib_imports, tmpdir);
}
@ -312,7 +319,7 @@ fn link_rlib<'a, B: ArchiveBuilder<'a>>(
}
}
}
return ab;
return Ok(ab);
// For rlibs we "pack" rustc metadata into a dummy object file. When rustc
// creates a dylib crate type it will pass `--whole-archive` (or the
@ -454,65 +461,40 @@ fn link_rlib<'a, B: ArchiveBuilder<'a>>(
fn collate_raw_dylibs(
sess: &Session,
used_libraries: &[NativeLib],
) -> Vec<(String, Vec<DllImport>)> {
let mut dylib_table: FxHashMap<String, FxHashSet<DllImport>> = FxHashMap::default();
) -> Result<Vec<(String, Vec<DllImport>)>, ErrorReported> {
// Use index maps to preserve original order of imports and libraries.
let mut dylib_table = FxIndexMap::<String, FxIndexMap<Symbol, &DllImport>>::default();
for lib in used_libraries {
if lib.kind == NativeLibKind::RawDylib {
let name = lib.name.unwrap_or_else(||
bug!("`link` attribute with kind = \"raw-dylib\" and no name should have caused error earlier")
);
let name = if matches!(lib.verbatim, Some(true)) {
name.to_string()
} else {
format!("{}.dll", name)
};
dylib_table.entry(name).or_default().extend(lib.dll_imports.iter().cloned());
}
}
// Rustc already signals an error if we have two imports with the same name but different
// calling conventions (or function signatures), so we don't have pay attention to those
// when ordering.
// FIXME: when we add support for ordinals, figure out if we need to do anything if we
// have two DllImport values with the same name but different ordinals.
let mut result: Vec<(String, Vec<DllImport>)> = dylib_table
.into_iter()
.map(|(lib_name, import_table)| {
let mut imports = Vec::from_iter(import_table.into_iter());
imports.sort_unstable_by_key(|x: &DllImport| x.name.as_str());
(lib_name, imports)
})
.collect::<Vec<_>>();
result.sort_unstable_by(|a: &(String, Vec<DllImport>), b: &(String, Vec<DllImport>)| {
a.0.cmp(&b.0)
});
let result = result;
// Check for multiple imports with the same name but different calling conventions or
// (when relevant) argument list sizes. Rustc only signals an error for this if the
// declarations are at the same scope level; if one shadows the other, we only get a lint
// warning.
for (library, imports) in &result {
let mut import_table: FxHashMap<Symbol, DllCallingConvention> = FxHashMap::default();
for import in imports {
if let Some(old_convention) =
import_table.insert(import.name, import.calling_convention)
{
if import.calling_convention != old_convention {
sess.span_fatal(
import.span,
&format!(
"multiple definitions of external function `{}` from library `{}` have different calling conventions",
import.name,
library,
));
let ext = if matches!(lib.verbatim, Some(true)) { "" } else { ".dll" };
let name = format!("{}{}", lib.name.expect("unnamed raw-dylib library"), ext);
let imports = dylib_table.entry(name.clone()).or_default();
for import in &lib.dll_imports {
if let Some(old_import) = imports.insert(import.name, import) {
// FIXME: when we add support for ordinals, figure out if we need to do anything
// if we have two DllImport values with the same name but different ordinals.
if import.calling_convention != old_import.calling_convention {
sess.span_err(
import.span,
&format!(
"multiple declarations of external function `{}` from \
library `{}` have different calling conventions",
import.name, name,
),
);
}
}
}
}
}
result
sess.compile_status()?;
Ok(dylib_table
.into_iter()
.map(|(name, imports)| {
(name, imports.into_iter().map(|(_, import)| import.clone()).collect())
})
.collect())
}
/// Create a static archive.
@ -531,9 +513,9 @@ fn link_staticlib<'a, B: ArchiveBuilder<'a>>(
codegen_results: &CodegenResults,
out_filename: &Path,
tempdir: &MaybeTempDir,
) {
) -> Result<(), ErrorReported> {
let mut ab =
link_rlib::<B>(sess, codegen_results, RlibFlavor::StaticlibBase, out_filename, tempdir);
link_rlib::<B>(sess, codegen_results, RlibFlavor::StaticlibBase, out_filename, tempdir)?;
let mut all_native_libs = vec![];
let res = each_linked_rlib(&codegen_results.crate_info, &mut |cnum, path| {
@ -581,6 +563,8 @@ fn link_staticlib<'a, B: ArchiveBuilder<'a>>(
print_native_static_libs(sess, &all_native_libs);
}
}
Ok(())
}
fn escape_stdout_stderr_string(s: &[u8]) -> String {

View File

@ -1569,7 +1569,7 @@ impl EncodeContext<'a, 'tcx> {
fn encode_native_libraries(&mut self) -> Lazy<[NativeLib]> {
empty_proc_macro!(self);
let used_libraries = self.tcx.native_libraries(LOCAL_CRATE);
self.lazy(used_libraries.iter().cloned())
self.lazy(used_libraries.iter())
}
fn encode_foreign_modules(&mut self) -> Lazy<[ForeignModule]> {

View File

@ -66,7 +66,7 @@ pub enum LinkagePreference {
RequireStatic,
}
#[derive(Clone, Debug, Encodable, Decodable, HashStable)]
#[derive(Debug, Encodable, Decodable, HashStable)]
pub struct NativeLib {
pub kind: NativeLibKind,
pub name: Option<Symbol>,
@ -77,7 +77,7 @@ pub struct NativeLib {
pub dll_imports: Vec<DllImport>,
}
#[derive(Clone, Debug, PartialEq, Eq, Encodable, Decodable, Hash, HashStable)]
#[derive(Clone, Debug, Encodable, Decodable, HashStable)]
pub struct DllImport {
pub name: Symbol,
pub ordinal: Option<u16>,
@ -94,7 +94,7 @@ pub struct DllImport {
///
/// The usize value, where present, indicates the size of the function's argument list
/// in bytes.
#[derive(Copy, Clone, Debug, PartialEq, Eq, Encodable, Decodable, Hash, HashStable)]
#[derive(Clone, PartialEq, Debug, Encodable, Decodable, HashStable)]
pub enum DllCallingConvention {
C,
Stdcall(usize),

View File

@ -0,0 +1,19 @@
// only-i686-pc-windows-msvc
// compile-flags: --crate-type lib --emit link
#![allow(clashing_extern_declarations)]
#![feature(raw_dylib)]
//~^ WARN the feature `raw_dylib` is incomplete
#[link(name = "foo", kind = "raw-dylib")]
extern "C" {
fn f(x: i32);
}
pub fn lib_main() {
#[link(name = "foo", kind = "raw-dylib")]
extern "stdcall" {
fn f(x: i32);
//~^ ERROR multiple declarations of external function `f` from library `foo.dll` have different calling conventions
}
unsafe { f(42); }
}

View File

@ -0,0 +1,17 @@
warning: the feature `raw_dylib` is incomplete and may not be safe to use and/or cause compiler crashes
--> $DIR/multiple-declarations.rs:4:12
|
LL | #![feature(raw_dylib)]
| ^^^^^^^^^
|
= note: `#[warn(incomplete_features)]` on by default
= note: see issue #58713 <https://github.com/rust-lang/rust/issues/58713> for more information
error: multiple declarations of external function `f` from library `foo.dll` have different calling conventions
--> $DIR/multiple-declarations.rs:14:9
|
LL | fn f(x: i32);
| ^^^^^^^^^^^^^
error: aborting due to previous error; 1 warning emitted