From 142f6c0b078ceef1dc817c418f628d350551f6e4 Mon Sep 17 00:00:00 2001 From: Richard Cobbe Date: Fri, 10 Sep 2021 17:34:09 -0700 Subject: [PATCH] Implement #[link_ordinal] attribute in the context of #[link(kind = "raw-dylib")]. --- .../rustc_codegen_llvm/src/back/archive.rs | 10 +++--- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 11 ++++-- .../rustc_llvm/llvm-wrapper/RustWrapper.cpp | 11 +++--- compiler/rustc_metadata/src/native_libs.rs | 8 ++++- .../src/middle/codegen_fn_attrs.rs | 2 +- compiler/rustc_typeck/src/collect.rs | 35 ++++++++++++++++--- .../run-make/raw-dylib-link-ordinal/Makefile | 18 ++++++++++ .../run-make/raw-dylib-link-ordinal/driver.rs | 5 +++ .../raw-dylib-link-ordinal/exporter.c | 5 +++ .../raw-dylib-link-ordinal/exporter.def | 3 ++ .../run-make/raw-dylib-link-ordinal/lib.rs | 13 +++++++ .../raw-dylib-link-ordinal/output.txt | 1 + .../link-ordinal-missing-argument.rs | 11 ++++++ .../link-ordinal-missing-argument.stderr | 19 ++++++++++ .../link-ordinal-multiple.rs | 13 +++++++ .../link-ordinal-multiple.stderr | 17 +++++++++ .../link-ordinal-too-large.rs | 4 +-- .../link-ordinal-too-large.stderr | 8 ++--- .../link-ordinal-too-many-arguments.rs | 11 ++++++ .../link-ordinal-too-many-arguments.stderr | 19 ++++++++++ 20 files changed, 201 insertions(+), 23 deletions(-) create mode 100644 src/test/run-make/raw-dylib-link-ordinal/Makefile create mode 100644 src/test/run-make/raw-dylib-link-ordinal/driver.rs create mode 100644 src/test/run-make/raw-dylib-link-ordinal/exporter.c create mode 100644 src/test/run-make/raw-dylib-link-ordinal/exporter.def create mode 100644 src/test/run-make/raw-dylib-link-ordinal/lib.rs create mode 100644 src/test/run-make/raw-dylib-link-ordinal/output.txt create mode 100644 src/test/ui/rfc-2627-raw-dylib/link-ordinal-missing-argument.rs create mode 100644 src/test/ui/rfc-2627-raw-dylib/link-ordinal-missing-argument.stderr create mode 100644 src/test/ui/rfc-2627-raw-dylib/link-ordinal-multiple.rs create mode 100644 src/test/ui/rfc-2627-raw-dylib/link-ordinal-multiple.stderr create mode 100644 src/test/ui/rfc-2627-raw-dylib/link-ordinal-too-many-arguments.rs create mode 100644 src/test/ui/rfc-2627-raw-dylib/link-ordinal-too-many-arguments.stderr diff --git a/compiler/rustc_codegen_llvm/src/back/archive.rs b/compiler/rustc_codegen_llvm/src/back/archive.rs index 4e86946219fb1..58a76b30578d2 100644 --- a/compiler/rustc_codegen_llvm/src/back/archive.rs +++ b/compiler/rustc_codegen_llvm/src/back/archive.rs @@ -163,13 +163,13 @@ impl<'a> ArchiveBuilder<'a> for LlvmArchiveBuilder<'a> { // 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 = dll_imports + let import_name_and_ordinal_vector: Vec<(CString, Option)> = dll_imports .iter() .map(|import: &DllImport| { if self.config.sess.target.arch == "x86" { - LlvmArchiveBuilder::i686_decorated_name(import) + (LlvmArchiveBuilder::i686_decorated_name(import), import.ordinal) } else { - CString::new(import.name.to_string()).unwrap() + (CString::new(import.name.to_string()).unwrap(), import.ordinal) } }) .collect(); @@ -184,9 +184,9 @@ impl<'a> ArchiveBuilder<'a> for LlvmArchiveBuilder<'a> { dll_imports.iter().map(|import| import.name.to_string()).collect::>().join(", "), ); - let ffi_exports: Vec = import_name_vector + let ffi_exports: Vec = import_name_and_ordinal_vector .iter() - .map(|name_z| LLVMRustCOFFShortExport::from_name(name_z.as_ptr())) + .map(|(name_z, ordinal)| LLVMRustCOFFShortExport::new(name_z.as_ptr(), *ordinal)) .collect(); let result = unsafe { crate::llvm::LLVMRustWriteImportLibrary( diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 3f2ed02d90df3..f69716e4e1e9b 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -34,11 +34,18 @@ pub enum LLVMRustResult { #[repr(C)] pub struct LLVMRustCOFFShortExport { pub name: *const c_char, + pub ordinal_present: bool, + // value of `ordinal` only important when `ordinal_present` is true + pub ordinal: u16, } impl LLVMRustCOFFShortExport { - pub fn from_name(name: *const c_char) -> LLVMRustCOFFShortExport { - LLVMRustCOFFShortExport { name } + pub fn new(name: *const c_char, ordinal: Option) -> LLVMRustCOFFShortExport { + LLVMRustCOFFShortExport { + name, + ordinal_present: ordinal.is_some(), + ordinal: ordinal.unwrap_or(0), + } } } diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index 9850f395a0f65..b0ebae5214af3 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -1749,10 +1749,11 @@ LLVMRustBuildMaxNum(LLVMBuilderRef B, LLVMValueRef LHS, LLVMValueRef RHS) { } // This struct contains all necessary info about a symbol exported from a DLL. -// At the moment, it's just the symbol's name, but we use a separate struct to -// make it easier to add other information like ordinal later. struct LLVMRustCOFFShortExport { const char* name; + bool ordinal_present; + // The value of `ordinal` is only meaningful if `ordinal_present` is true. + uint16_t ordinal; }; // Machine must be a COFF machine type, as defined in PE specs. @@ -1768,13 +1769,15 @@ extern "C" LLVMRustResult LLVMRustWriteImportLibrary( ConvertedExports.reserve(NumExports); for (size_t i = 0; i < NumExports; ++i) { + bool ordinal_present = Exports[i].ordinal_present; + uint16_t ordinal = ordinal_present ? Exports[i].ordinal : 0; ConvertedExports.push_back(llvm::object::COFFShortExport{ Exports[i].name, // Name std::string{}, // ExtName std::string{}, // SymbolName std::string{}, // AliasTarget - 0, // Ordinal - false, // Noname + ordinal, // Ordinal + ordinal_present, // Noname false, // Data false, // Private false // Constant diff --git a/compiler/rustc_metadata/src/native_libs.rs b/compiler/rustc_metadata/src/native_libs.rs index 5f0d8c46f20dc..45050d0618a59 100644 --- a/compiler/rustc_metadata/src/native_libs.rs +++ b/compiler/rustc_metadata/src/native_libs.rs @@ -433,6 +433,12 @@ impl Collector<'tcx> { } } }; - DllImport { name: item.ident.name, ordinal: None, calling_convention, span: item.span } + + DllImport { + name: item.ident.name, + ordinal: self.tcx.codegen_fn_attrs(item.id.def_id).link_ordinal, + calling_convention, + span: item.span, + } } } diff --git a/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs b/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs index b2705c7693914..b054d21adaa13 100644 --- a/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs +++ b/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs @@ -22,7 +22,7 @@ pub struct CodegenFnAttrs { /// imported function has in the dynamic library. Note that this must not /// be set when `link_name` is set. This is for foreign items with the /// "raw-dylib" kind. - pub link_ordinal: Option, + pub link_ordinal: Option, /// The `#[target_feature(enable = "...")]` attribute and the enabled /// features (only enabled features are supported right now). pub target_features: Vec, diff --git a/compiler/rustc_typeck/src/collect.rs b/compiler/rustc_typeck/src/collect.rs index 1bc7bc3e063d4..02857c7886fd0 100644 --- a/compiler/rustc_typeck/src/collect.rs +++ b/compiler/rustc_typeck/src/collect.rs @@ -2858,6 +2858,14 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs { } else if attr.has_name(sym::link_name) { codegen_fn_attrs.link_name = attr.value_str(); } else if attr.has_name(sym::link_ordinal) { + if link_ordinal_span.is_some() { + tcx.sess + .struct_span_err( + attr.span, + "multiple `link_ordinal` attributes on a single definition", + ) + .emit(); + } link_ordinal_span = Some(attr.span); if let ordinal @ Some(_) = check_link_ordinal(tcx, attr) { codegen_fn_attrs.link_ordinal = ordinal; @@ -3153,22 +3161,41 @@ fn should_inherit_track_caller(tcx: TyCtxt<'_>, def_id: DefId) -> bool { false } -fn check_link_ordinal(tcx: TyCtxt<'_>, attr: &ast::Attribute) -> Option { +fn check_link_ordinal(tcx: TyCtxt<'_>, attr: &ast::Attribute) -> Option { use rustc_ast::{Lit, LitIntType, LitKind}; let meta_item_list = attr.meta_item_list(); let meta_item_list: Option<&[ast::NestedMetaItem]> = meta_item_list.as_ref().map(Vec::as_ref); let sole_meta_list = match meta_item_list { Some([item]) => item.literal(), + Some(_) => { + tcx.sess + .struct_span_err(attr.span, "incorrect number of arguments to `#[link_ordinal]`") + .note("the attribute requires exactly one argument") + .emit(); + return None; + } _ => None, }; if let Some(Lit { kind: LitKind::Int(ordinal, LitIntType::Unsuffixed), .. }) = sole_meta_list { - if *ordinal <= usize::MAX as u128 { - Some(*ordinal as usize) + // According to the table at https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#import-header, + // the ordinal must fit into 16 bits. Similarly, the Ordinal field in COFFShortExport (defined + // in llvm/include/llvm/Object/COFFImportFile.h), which we use to communicate import information + // to LLVM for `#[link(kind = "raw-dylib"_])`, is also defined to be uint16_t. + // + // FIXME: should we allow an ordinal of 0? The MSVC toolchain has inconsistent support for this: + // both LINK.EXE and LIB.EXE signal errors and abort when given a .DEF file that specifies + // a zero ordinal. However, llvm-dlltool is perfectly happy to generate an import library + // for such a .DEF file, and MSVC's LINK.EXE is also perfectly happy to consume an import + // library produced by LLVM with an ordinal of 0, and it generates an .EXE. (I don't know yet + // if the resulting EXE runs, as I haven't yet built the necessary DLL -- see earlier comment + // about LINK.EXE failing.) + if *ordinal <= u16::MAX as u128 { + Some(*ordinal as u16) } else { let msg = format!("ordinal value in `link_ordinal` is too large: `{}`", &ordinal); tcx.sess .struct_span_err(attr.span, &msg) - .note("the value may not exceed `usize::MAX`") + .note("the value may not exceed `u16::MAX`") .emit(); None } diff --git a/src/test/run-make/raw-dylib-link-ordinal/Makefile b/src/test/run-make/raw-dylib-link-ordinal/Makefile new file mode 100644 index 0000000000000..04b257d063204 --- /dev/null +++ b/src/test/run-make/raw-dylib-link-ordinal/Makefile @@ -0,0 +1,18 @@ +# Test the behavior of #[link(.., kind = "raw-dylib")] and #[link_ordinal] on windows-msvc + +# only-windows-msvc + +-include ../../run-make-fulldeps/tools.mk + +all: + $(call COMPILE_OBJ,"$(TMPDIR)"/exporter.obj,exporter.c) + $(CC) "$(TMPDIR)"/exporter.obj exporter.def -link -dll -out:"$(TMPDIR)"/exporter.dll + $(RUSTC) --crate-type lib --crate-name raw_dylib_test lib.rs + $(RUSTC) --crate-type bin driver.rs -L "$(TMPDIR)" + "$(TMPDIR)"/driver > "$(TMPDIR)"/output.txt + +ifdef RUSTC_BLESS_TEST + cp "$(TMPDIR)"/output.txt output.txt +else + $(DIFF) output.txt "$(TMPDIR)"/output.txt +endif diff --git a/src/test/run-make/raw-dylib-link-ordinal/driver.rs b/src/test/run-make/raw-dylib-link-ordinal/driver.rs new file mode 100644 index 0000000000000..4059ede11fc96 --- /dev/null +++ b/src/test/run-make/raw-dylib-link-ordinal/driver.rs @@ -0,0 +1,5 @@ +extern crate raw_dylib_test; + +fn main() { + raw_dylib_test::library_function(); +} diff --git a/src/test/run-make/raw-dylib-link-ordinal/exporter.c b/src/test/run-make/raw-dylib-link-ordinal/exporter.c new file mode 100644 index 0000000000000..a9dd6da6616f9 --- /dev/null +++ b/src/test/run-make/raw-dylib-link-ordinal/exporter.c @@ -0,0 +1,5 @@ +#include + +void exported_function() { + printf("exported_function\n"); +} diff --git a/src/test/run-make/raw-dylib-link-ordinal/exporter.def b/src/test/run-make/raw-dylib-link-ordinal/exporter.def new file mode 100644 index 0000000000000..1a4b4c941b65d --- /dev/null +++ b/src/test/run-make/raw-dylib-link-ordinal/exporter.def @@ -0,0 +1,3 @@ +LIBRARY exporter +EXPORTS + exported_function @13 NONAME diff --git a/src/test/run-make/raw-dylib-link-ordinal/lib.rs b/src/test/run-make/raw-dylib-link-ordinal/lib.rs new file mode 100644 index 0000000000000..20609caa5be21 --- /dev/null +++ b/src/test/run-make/raw-dylib-link-ordinal/lib.rs @@ -0,0 +1,13 @@ +#![feature(raw_dylib)] + +#[link(name = "exporter", kind = "raw-dylib")] +extern { + #[link_ordinal(13)] + fn imported_function(); +} + +pub fn library_function() { + unsafe { + imported_function(); + } +} diff --git a/src/test/run-make/raw-dylib-link-ordinal/output.txt b/src/test/run-make/raw-dylib-link-ordinal/output.txt new file mode 100644 index 0000000000000..2d0ed60f21667 --- /dev/null +++ b/src/test/run-make/raw-dylib-link-ordinal/output.txt @@ -0,0 +1 @@ +exported_function diff --git a/src/test/ui/rfc-2627-raw-dylib/link-ordinal-missing-argument.rs b/src/test/ui/rfc-2627-raw-dylib/link-ordinal-missing-argument.rs new file mode 100644 index 0000000000000..c391ccd1c8227 --- /dev/null +++ b/src/test/ui/rfc-2627-raw-dylib/link-ordinal-missing-argument.rs @@ -0,0 +1,11 @@ +#![feature(raw_dylib)] +//~^ WARN the feature `raw_dylib` is incomplete + +#[link(name = "foo")] +extern "C" { + #[link_ordinal()] + //~^ ERROR incorrect number of arguments to `#[link_ordinal]` + fn foo(); +} + +fn main() {} diff --git a/src/test/ui/rfc-2627-raw-dylib/link-ordinal-missing-argument.stderr b/src/test/ui/rfc-2627-raw-dylib/link-ordinal-missing-argument.stderr new file mode 100644 index 0000000000000..8e9edfb9d20ac --- /dev/null +++ b/src/test/ui/rfc-2627-raw-dylib/link-ordinal-missing-argument.stderr @@ -0,0 +1,19 @@ +warning: the feature `raw_dylib` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/link-ordinal-missing-argument.rs:1:12 + | +LL | #![feature(raw_dylib)] + | ^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #58713 for more information + +error: incorrect number of arguments to `#[link_ordinal]` + --> $DIR/link-ordinal-missing-argument.rs:6:5 + | +LL | #[link_ordinal()] + | ^^^^^^^^^^^^^^^^^ + | + = note: the attribute requires exactly one argument + +error: aborting due to previous error; 1 warning emitted + diff --git a/src/test/ui/rfc-2627-raw-dylib/link-ordinal-multiple.rs b/src/test/ui/rfc-2627-raw-dylib/link-ordinal-multiple.rs new file mode 100644 index 0000000000000..9874121267717 --- /dev/null +++ b/src/test/ui/rfc-2627-raw-dylib/link-ordinal-multiple.rs @@ -0,0 +1,13 @@ +// only-windows-msvc +#![feature(raw_dylib)] +//~^ WARN the feature `raw_dylib` is incomplete + +#[link(name = "foo", kind = "raw-dylib")] +extern "C" { + #[link_ordinal(1)] + #[link_ordinal(2)] + //~^ ERROR multiple `link_ordinal` attributes on a single definition + fn foo(); +} + +fn main() {} diff --git a/src/test/ui/rfc-2627-raw-dylib/link-ordinal-multiple.stderr b/src/test/ui/rfc-2627-raw-dylib/link-ordinal-multiple.stderr new file mode 100644 index 0000000000000..a79fb2de94402 --- /dev/null +++ b/src/test/ui/rfc-2627-raw-dylib/link-ordinal-multiple.stderr @@ -0,0 +1,17 @@ +warning: the feature `raw_dylib` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/link-ordinal-multiple.rs:2:12 + | +LL | #![feature(raw_dylib)] + | ^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #58713 for more information + +error: multiple `link_ordinal` attributes on a single definition + --> $DIR/link-ordinal-multiple.rs:8:5 + | +LL | #[link_ordinal(2)] + | ^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error; 1 warning emitted + diff --git a/src/test/ui/rfc-2627-raw-dylib/link-ordinal-too-large.rs b/src/test/ui/rfc-2627-raw-dylib/link-ordinal-too-large.rs index 10db497297027..b6089d27e7ab7 100644 --- a/src/test/ui/rfc-2627-raw-dylib/link-ordinal-too-large.rs +++ b/src/test/ui/rfc-2627-raw-dylib/link-ordinal-too-large.rs @@ -3,8 +3,8 @@ #[link(name = "foo")] extern "C" { - #[link_ordinal(18446744073709551616)] - //~^ ERROR ordinal value in `link_ordinal` is too large: `18446744073709551616` + #[link_ordinal(72436)] + //~^ ERROR ordinal value in `link_ordinal` is too large: `72436` fn foo(); } diff --git a/src/test/ui/rfc-2627-raw-dylib/link-ordinal-too-large.stderr b/src/test/ui/rfc-2627-raw-dylib/link-ordinal-too-large.stderr index 35f9b53fdf720..bbe985fa10ada 100644 --- a/src/test/ui/rfc-2627-raw-dylib/link-ordinal-too-large.stderr +++ b/src/test/ui/rfc-2627-raw-dylib/link-ordinal-too-large.stderr @@ -7,13 +7,13 @@ LL | #![feature(raw_dylib)] = note: `#[warn(incomplete_features)]` on by default = note: see issue #58713 for more information -error: ordinal value in `link_ordinal` is too large: `18446744073709551616` +error: ordinal value in `link_ordinal` is too large: `72436` --> $DIR/link-ordinal-too-large.rs:6:5 | -LL | #[link_ordinal(18446744073709551616)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | #[link_ordinal(72436)] + | ^^^^^^^^^^^^^^^^^^^^^^ | - = note: the value may not exceed `usize::MAX` + = note: the value may not exceed `u16::MAX` error: aborting due to previous error; 1 warning emitted diff --git a/src/test/ui/rfc-2627-raw-dylib/link-ordinal-too-many-arguments.rs b/src/test/ui/rfc-2627-raw-dylib/link-ordinal-too-many-arguments.rs new file mode 100644 index 0000000000000..93286c616c5ac --- /dev/null +++ b/src/test/ui/rfc-2627-raw-dylib/link-ordinal-too-many-arguments.rs @@ -0,0 +1,11 @@ +#![feature(raw_dylib)] +//~^ WARN the feature `raw_dylib` is incomplete + +#[link(name = "foo")] +extern "C" { + #[link_ordinal(3, 4)] + //~^ ERROR incorrect number of arguments to `#[link_ordinal]` + fn foo(); +} + +fn main() {} diff --git a/src/test/ui/rfc-2627-raw-dylib/link-ordinal-too-many-arguments.stderr b/src/test/ui/rfc-2627-raw-dylib/link-ordinal-too-many-arguments.stderr new file mode 100644 index 0000000000000..484c85a0f422a --- /dev/null +++ b/src/test/ui/rfc-2627-raw-dylib/link-ordinal-too-many-arguments.stderr @@ -0,0 +1,19 @@ +warning: the feature `raw_dylib` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/link-ordinal-too-many-arguments.rs:1:12 + | +LL | #![feature(raw_dylib)] + | ^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #58713 for more information + +error: incorrect number of arguments to `#[link_ordinal]` + --> $DIR/link-ordinal-too-many-arguments.rs:6:5 + | +LL | #[link_ordinal(3, 4)] + | ^^^^^^^^^^^^^^^^^^^^^ + | + = note: the attribute requires exactly one argument + +error: aborting due to previous error; 1 warning emitted +