Implement #[link_ordinal] attribute in the context of #[link(kind = "raw-dylib")].

Author: ricobbe

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<CString> = dll_imports
+        let import_name_and_ordinal_vector: Vec<(CString, Option<u16>)> = 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::<Vec<_>>().join(", "),
         );
 
-        let ffi_exports: Vec<LLVMRustCOFFShortExport> = import_name_vector
+        let ffi_exports: Vec<LLVMRustCOFFShortExport> = 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(

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<u16>) -> LLVMRustCOFFShortExport {
+        LLVMRustCOFFShortExport {
+            name,
+            ordinal_present: ordinal.is_some(),
+            ordinal: ordinal.unwrap_or(0),
+        }
     }
 }
 

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

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,
+        }
     }
 }

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<usize>,
+    pub link_ordinal: Option<u16>,
     /// The `#[target_feature(enable = "...")]` attribute and the enabled
     /// features (only enabled features are supported right now).
     pub target_features: Vec<Symbol>,

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<usize> {
+fn check_link_ordinal(tcx: TyCtxt<'_>, attr: &ast::Attribute) -> Option<u16> {
     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
         }

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

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();
+}

src/test/run-make/raw-dylib-link-ordinal/exporter.c

@@ -0,0 +1,5 @@
+#include <stdio.h>
+
+void exported_function() {
+    printf("exported_function\n");
+}

src/test/run-make/raw-dylib-link-ordinal/exporter.def

@@ -0,0 +1,3 @@
+LIBRARY exporter
+EXPORTS
+    exported_function @13 NONAME