[RFC] Add `#[export_ordinal(n)]` attribute #3641

MeguminSama · 2024-05-20T20:54:35Z

This RFC adds an attribute that allows specifying the ordinal of an exported function when creating cdylibs on windows.

We already have a #[link_ordinal(n)] attribute which allows importing functions from DLLs by its ordinal, but we don't have any way of doing the opposite - specifying the ordinal of a function when we're making our own DLLs.

Currently, you need to create a lib.def file and specify the ordinals, and then link it with cargo:rustc-cdylib-link-arg=/DEF.

The biggest downside of the current method is that once you specify a .def file, you will have to specify an ordinal for every function that you want to export from the DLL, or else it won't be present in the generated .lib file. This can become very overwhelming if you have a lot of exported functions.

This RFC would allow you to specify exported function ordinals like so:

// Export this function at Ordinal 1
#[no_mangle]
#[export_ordinal(1)]
pub extern "C" fn hello() {
	println!("Hello, World!");
}

Rendered

programmerjake · 2024-05-20T21:08:58Z

you should probably specify that the attribute is ignored everywhere other than Windows (or similar) targets.

MeguminSama · 2024-05-20T21:12:21Z

@programmerjake Thanks, I wasn't sure if ordinals were a thing on other targets. I'll update it accordingly.

Ordinals are a specific feature in Windows DLLs, and are not present on other targets.

The attribute should not cause an error on non-windows targets. Instead, it should do nothing.

…als.md

joshtriplett · 2024-06-18T17:00:52Z

@rfcbot merge

rfcbot · 2024-06-18T17:00:54Z

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

tmandry · 2024-06-20T21:39:54Z

Can we reuse #[link_ordinal(n)] for this purpose, the same way extern "C" can be used both for "importing" and "exporting" functions? I don't think there's any ambiguity based on context.

Should this attribute be unsafe once unsafe attributes are implemented? I'm unsure if we can guarantee there will be no collisions with anything else linked into the program (including native libraries).

MeguminSama · 2024-06-20T21:53:50Z

Can we reuse #[link_ordinal(n)] for this purpose, the same way extern "C" can be used both for "importing" and "exporting" functions?

I think this could be a good idea if there is no ambiguity. It could be confusing at a glance if a project has multiple #[link_ordinal(n)], one for importing, and one for exporting, but it's probably a non-issue.

Should this attribute be unsafe once unsafe attributes are implemented?

I'm not sure about how export_ordinal would be implemented, but if there's the chance of collisions, then I agree it should be marked as unsafe.

tmandry

Does the attribute itself modify symbol attributes such that the symbol is exported, or does it only modify the ordinal field? I'm having a hard time finding the docs for the rules determining when a function is exported are not. I think it has to do with whether they are public, which seems like it would have some drawbacks. I'm surprised by the example in the RFC:

#[link(name = "external_library")]
extern "C" {
    #[export_ordinal(2)]
    pub fn external_function();
}

In this case is the externally-declared function somehow being re-exported? Then it seems like we might need both #[link_ordinal(n)] and #[export_ordinal(n)] on the same function.

I'm not sure about how export_ordinal would be implemented, but if there's the chance of collisions, then I agree it should be marked as unsafe.

Can you add this as an unresolved question to the RFC?

text/3641-export-function-ordinals.md

- Removed a statement that should be an unresolved question instead. - Add unresolved question for whether export_ordinal should be marked unsafe when RFC3325 (Unsafe Attributes) is implemented.

MeguminSama · 2024-06-21T12:54:03Z

In this case is the externally-declared function somehow being re-exported?

@tmandry I took into consideration that some libraries could possibly be statically linked, and the possibility that someone may want to re-export a function. If this isn't a valid concern I'm happy to remove it - I just tried to think of every possibility.

RalfJung · 2024-06-21T13:03:44Z

Are reexports even possible?
Existing attributes like link_name use the same name for imports and exports so it would seem odd to use a different name here.

tmandry · 2024-06-21T17:58:15Z

Existing attributes like link_name use the same name for imports and exports so it would seem odd to use a different name here.

I don't think that's true; if I try to use it on an export I get a warning:

#[link_name = "actual_symbol_name"]
pub extern "C" fn name_in_rust() {}

warning: attribute should be applied to a foreign function or static
 --> src/lib.rs:1:1
  |
1 | #[link_name = "actual_symbol_name"]
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2 | #[no_mangle]
3 | pub extern "C" fn name_in_rust() {}
  | ----------------------------------- not a foreign function or static
  |
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
  = note: `#[warn(unused_attributes)]` on by default

Regardless, agreed that we should be consistent: Either link_name and link_ordinal work for both imports and exports, or we would have export_name and export_ordinal. I still lean toward the former unless it's possible in some executable format to have a re-exported function under a different name.

RalfJung · 2024-06-21T21:19:04Z

Seems like I misremembered... I thought there was a way to set the link name independent of the item name. Maybe there isn't.

ChrisDenton · 2024-06-22T02:29:13Z

Were you thinking of export_name? E.g.

#[export_name = "exported_symbol_name"]
pub extern "C" fn name_in_rust() {}

MeguminSama · 2024-06-25T23:52:10Z

If we already use export_name, I think it'd be best to stick with export_ordinal?

RalfJung · 2024-06-26T05:38:34Z

Yeah, that makes sense.

tmandry · 2024-06-28T22:24:14Z

@rfcbot reviewed

Agreed that we should stick with export_ordinal to be consistent with export_name.

MeguminSama · 2024-07-30T12:46:57Z

I noticed that the edition guide for unsafe attributes got merged.

It says that export_name must be marked as unsafe. I think that if this is the case, export_ordinal should definitely be unsafe.

Should I update the RFC to remove it from the unanswered questions, and clarify that in 2024 edition it should be marked as unsafe?

Edit: I don't entirely understand how editions would work - if export_ordinal got added to rust, would it only be available in 2024 edition? Or would it be available in prior editions, and just marked as unsafe in the 2024 edition?

RalfJung · 2024-07-30T12:52:23Z

If it has the same risks as export_name, then yeah it should be unsafe.

MeguminSama · 2024-07-30T12:54:52Z

It's entirely possible that a dependency could mark something as a duplicate ordinal, so I believe the same risks apply.

I'm not sure whether to word it as "in 2024 edition" or not though.

RalfJung · 2024-07-30T12:57:47Z

I don't see a reason to make this unsafe only in some editions. We do that for old attributes solely due to backwards compatibility. New things should be properly unsafe from the start.

MeguminSama · 2024-07-30T12:58:59Z

Thanks for the clarification. I'll work on the changes in a bit.

MeguminSama · 2024-08-02T14:36:44Z

I have updated the RFC to mark export_ordinal as unsafe.

I have also removed the section on re-exporting bindings, as after testing, I've confirmed that this is not valid code.

Initial draft for rfc: export_function_ordinals

a0c43e4

MeguminSama added 4 commits May 20, 2024 22:19

Specify that this attribute is only for windows targets

b728fa9

Ordinals are a specific feature in Windows DLLs, and are not present on other targets.

Better wording for windows-only effects

5f44f70

The attribute should not cause an error on non-windows targets. Instead, it should do nothing.

Rename 0000-export-function-ordinals.md to 3641-export-function-ordin…

d0b70b9

…als.md

Add RFC PR link to the RFC Document

e5078bd

ehuss added the T-lang Relevant to the language team, which will review and decide on the RFC. label May 21, 2024

rfcbot added proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. labels Jun 18, 2024

tmandry reviewed Jun 20, 2024

View reviewed changes

text/3641-export-function-ordinals.md Outdated Show resolved Hide resolved

Resolve Feedback: Unresolved Questions

60b62ff

- Removed a statement that should be an unresolved question instead. - Add unresolved question for whether export_ordinal should be marked unsafe when RFC3325 (Unsafe Attributes) is implemented.

Mark export_ordinal as unsafe. Remove re-export section.

73ce649

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFC] Add `#[export_ordinal(n)]` attribute #3641

[RFC] Add `#[export_ordinal(n)]` attribute #3641

[RFC] Add #[export_ordinal(n)] attribute #3641

Are you sure you want to change the base?

[RFC] Add #[export_ordinal(n)] attribute #3641

Conversation

Choose a reason for hiding this comment

[RFC] Add `#[export_ordinal(n)]` attribute #3641

[RFC] Add `#[export_ordinal(n)]` attribute #3641