Skip to content

Remove nondeterminism in multiple-definitions test #87092

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

Merged
merged 1 commit into from
Jul 18, 2021
Merged
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
4 changes: 1 addition & 3 deletions compiler/rustc_codegen_cranelift/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,9 +221,7 @@ impl CodegenBackend for CraneliftCodegenBackend {
sess,
&codegen_results,
outputs,
);

Ok(())
)
}
}

Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_codegen_llvm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
114 changes: 49 additions & 65 deletions compiler/rustc_codegen_ssa/src/back/link.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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};
Expand All @@ -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() {
Expand Down Expand Up @@ -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>(
Expand Down Expand Up @@ -145,6 +150,8 @@ pub fn link_binary<'a, B: ArchiveBuilder<'a>>(
}
}
});

Ok(())
}

pub fn each_linked_rlib(
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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| {
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]> {
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_middle/src/middle/cstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand All @@ -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>,
Expand All @@ -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),
Expand Down
19 changes: 19 additions & 0 deletions src/test/ui/rfc-2627-raw-dylib/multiple-declarations.rs
Original file line number Diff line number Diff line change
@@ -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); }
}
17 changes: 17 additions & 0 deletions src/test/ui/rfc-2627-raw-dylib/multiple-declarations.stderr
Original file line number Diff line number Diff line change
@@ -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