Skip to content
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

Add import_name_type parameter to #[link] #495

Closed
1 of 3 tasks
ricobbe opened this issue Mar 7, 2022 · 2 comments
Closed
1 of 3 tasks

Add import_name_type parameter to #[link] #495

ricobbe opened this issue Mar 7, 2022 · 2 comments
Labels
major-change A proposal to make a major change to rustc T-compiler Add this label so rfcbot knows to poll the compiler team

Comments

@ricobbe
Copy link

ricobbe commented Mar 7, 2022

Proposal

I propose the addition of a new parameter, import_name_type, to the #[link] attribute, to be used with #[link(kind = "raw-dylib")] when importing stdcall functions on i686-pc-windows-* targets.

When creating a Windows DLL on i686 that exports stdcall functions, it is possible to export the symbols from the DLL in one of four different ways, corresponding to the import name types from the PE-COFF spec. Each import name type encodes the symbol names in the DLL's export table in a different way, and the symbols in the import library must match in order to get a usable image. Mismatches lead either to link failures when creating the image or to "undefined symbol" errors upon attempting to load the image. For details, see the comments on the raw-dylib feature's tracking PR. To generate the correct import libraries for these DLLs, therefore, rustc has to know the import name type for each function in the extern block, and there is currently no way for users to provide this information.

I propose adding a new MetaNameValueStr key to the #[link] attribute. This key would be called import_name_type, and it would accept one of three values: "name", "noprefix", and "undecorate". (These names are taken from the PE-COFF spec.)

It is in theory possible to have a single DLL that exports some names with one type and other names with a different type, although I strongly suspect this to be quite rare in practice. Under this proposal, users could express such cases by providing multiple export blocks for the DLL, each with a different import name type.

Note: there is a fourth import name type defined in the PE-COFF spec, IMPORT_ORDINAL. This case is already implemented on master; it is handled by the #[link_ordinal] attribute. This case differs from the other three in that it requires additional information specific to each function, so moving this functionality into the #[link] attribute seems like a poor choice. I am proposing no changes to the #[link_ordinal] attribute.

Open points of discussion:

  • What is the desired behavior if import_name_type is not present on an extern "stdcall" or extern "system" block? Should the compiler signal an error, or should it default to one of the three values? If we choose to have a default, which of the three options should we choose? As one data point, the Windows API (such as kernel32.dll) requires "undecorate," but I don't have any hard data on which of the three is most common in the wild.
  • Should the compiler signal an error or warning if import_name_type is provided in situations where it is not required, or should it silently ignore the extra parameter? These cases include the following:
    • compiling for a target other than i686-pc-windows-*
    • present in a #[link] attribute where kind is not "raw-dylib"
    • present in a #[link] attribute on an extern block with a calling convention other that "stdcall" or "system"
  • One point that slightly complicates the previous two issues: if every function in an extern "stdcall" block is marked with the #[link_ordinal] attribute, then specifying import_name_type is unnecessary but harmless.

I'm currently focusing on stdcall functions, although I suspect that this would also apply to fastcall and vectorcall. However, there are currently outstanding problems with code generation for the latter two calling conventions that are unrelated to raw-dylib, so they would seem to be less pressing.

Mentors or Reviewers

@wesleywiser is willing to review this proposal.

Process

The main points of the Major Change Process are as follows:

  • File an issue describing the proposal.
  • A compiler team member or contributor who is knowledgeable in the area can second by writing @rustbot second.
    • Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a -C flag, then full team check-off is required.
    • Compiler team members can initiate a check-off via @rfcbot fcp merge on either the MCP or the PR.
  • Once an MCP is seconded, the Final Comment Period begins. If no objections are raised after 10 days, the MCP is considered approved.

You can read more about Major Change Proposals on forge.

Comments

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

@ricobbe ricobbe added major-change A proposal to make a major change to rustc T-compiler Add this label so rfcbot knows to poll the compiler team labels Mar 7, 2022
@rustbot
Copy link
Collaborator

rustbot commented Mar 7, 2022

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

cc @rust-lang/compiler @rust-lang/compiler-contributors

@rustbot rustbot added the to-announce Announce this issue on triage meeting label Mar 7, 2022
@ricobbe
Copy link
Author

ricobbe commented Mar 16, 2022

I'm withdrawing this proposal, as I no longer believe it to be sufficient to address the problem. The intent was that, when specifying import_name_type = "undecorate", rustc would simply pass the undecorated name of stdcall functions to LLVM for inclusion in the import library. However, it turns out that this doesn't work (and I'd originally thought it would because of an unrelated error involving kernel32.lib). The following example demonstrates the intent and the failure:

extern.c:

#include <stdio.h>

void __stdcall extern_function(int i) {
    printf("extern_function(%d)\n", i);
    fflush(stdout);
}

extern.def:

LIBRARY extern
EXPORTS
    extern_function

lib.rs:

#![feature(raw_dylib)]

#[link(name = "extern", kind = "raw-dylib", import_name_type = "undecorate")]
extern "stdcall" {
    fn extern_function(i: i32);
}

pub fn library_function() {
    unsafe {
        extern_function(14);
    }
}

driver.rs:

extern crate raw_dylib_import_name_type;

fn main() {
    raw_dylib_import_name_type::library_function();
}

Compile as follows, using rustc built from this branch, built for i686-pc-windows-msvc:

cl extern.c extern.def -link -dll -out: extern.dll
rustc --crate-type lib --crate-name raw_dylib_import_name_type lib.rs
rustc --crate-type bin driver.rs -L .

and the last command fails with an unresolved external symbol error for __imp__extern_function_@4. Clearly more work remains to address this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major-change A proposal to make a major change to rustc T-compiler Add this label so rfcbot knows to poll the compiler team
Projects
None yet
Development

No branches or pull requests

3 participants