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

#[link(kind="raw-dylib")] #2627

Merged
merged 14 commits into from Feb 24, 2019
Merged

Conversation

retep998
Copy link
Member

@retep998 retep998 commented Jan 22, 2019

Striving towards a future unburdened by the limitations of the traditional linker model.

Rendered
Tracking issue

@Centril Centril added T-lang Relevant to the language team, which will review and decide on the RFC. T-compiler Relevant to the compiler team, which will review and decide on the RFC. A-linkage Proposals relating to the linking step. A-windows Proposals relating to Windows. labels Jan 22, 2019
@Screwtapello
Copy link

...we'd no longer need to pretend the pc-windows-gnu toolchain is standalone, and we'd be able to stop bundling MinGW bits entirely in favor of the user's own MinGW installation, thereby resolving a bunch of issues.

Is this talking about rust-lang/rust#53454 ?

@retep998
Copy link
Member Author

@Screwtapello Yes.

@Screwtapello
Copy link

Huzzah! That bit me in the ass the other day, trying to cross-compile Windows binaries from Linux, so this sounds wonderful.

@GuentherVIII
Copy link

Prior art: Delphi doesn't use import libraries and instead one specifies the dll file name and optionally the function name or index:

procedure foo; external 'bar.dll'; name 'fooA';
procedure foo; external 'bar.dll'; index 1;

@retep998
Copy link
Member Author

retep998 commented Jan 23, 2019

@GuentherVIII Does Delphi compile to native binaries where those imports are resolved by the Windows PE Loader, and not Delphi doing it's own LoadLibrary/GetProcAddress stuff? An easy way to check is to produce such a binary and use depends.exe or dumpbin.exe to check if that symbol is in the imports.

@zunzster
Copy link

zunzster commented Jan 23, 2019

@retep998 Delphi supports both ways. However, originally it was just the native binaries model i.e. normal Windows PE loading. Support for delay-loaded bindings came second with the addition of the delayed keyword e.g.

function GetDesktopWindow: HWND; stdcall; external user32 name 'GetDesktopWindow' delayed;

The delayed model is useful when declaring APIs that may or may not exist on the current platform version. You can do a version check before you attempt to call them and everything works transparently and you don't get PE loader errors on startup about missing APIs.

Of course, you can do your own delayed loading by hand via LoadLibrary/GetProcAddress as I used to in earlier versions of Delphi but it's tedious boilerplate. Having it built into the compiler is quite nice since the compiler/linker can collect up all the APIs and generate nice thunks that do the minimum necessary work, reuse previous work (e.g. only one LoadLibrary per DLL not per API called) and then patch themselves out so that those APIs calls are no less efficient than directly linked ones.

Could you achieve something similar in Rust with a macro? It would probably need to be a procedural macro to have enough visibility and be 'smart'.

But does Rust support delayed or lazy APIs bindings for other dylibs?

If not, maybe a general delayed attribute would be useful for that since it's not a platform specific concept. Cleanly and easily handling versioned or missing APIs is a common systems programming annoyance/papercut.

Declaration of self-interest: I'm from a long term Delphi shop and I'd really like to shift to Rust being the preferred language for all our future development. Anything that makes Rust more capable/enjoyable for Windows development is a boon for me :-)

@retep998
Copy link
Member Author

@zunzster

Having runtime loaded variants of all external functions in winapi is something I hope to achieve in the future, but shortcomings of declarative macros make it hard to do so with minimal effort. Having a generalized approach in Rust to do lazy loading of any external function could be a future extension of kind="dll".

Having delayed loading of Rust crates does not make any sense at the moment, as Rust does not have any sort of stable ABI.

@zunzster
Copy link

zunzster commented Jan 24, 2019

@retep998 Sure, I can see that an optional #[link(delayed=true)] addition might be nice in the future and looks like a pretty clean approach syntax-wise. The compiler could then collect up the delayed APIs and generate smart run-time thunks for each API.

And you're right that delay loading is not that useful for calling Rust ABIs, it's more about convenient access to versioned platform APIs.

I'm not sure I see that having delay loaded variants of all the winapi's would be that useful, unless I misunderstand, (which is entirely possible :-))

Are you thinking these would have different names from the non-delayed win apis? e.g.

#[link(name = "kernel32.dll", kind = "dll", delayed = true)]
#[allow(non_snake_case)]
extern "system" {
    fn DelayGetStdHandle(nStdHandle: u32) -> *mut u8;
}

In my Delphi code, most Windows platform APIs I need are present in all versions. It's just those annoying few that were added later that I need to support that require declaring and calling those APIs via a delay mechanism.

If in my local crate, I have my own block like the above which has the same name as the non-delay case in some winapi crate, but I've added a delayed=true directive, would that be an error in Rust or would it shadow the declaration in the winapi crate?

I can see that copying the declaration from the winapi crate into my crate and adding the delayed=true and Delay in front of the name (for example) would be very easy.

Ah, now I think I see your idea for delayed versions of all the APIs. A whole crate with delayed=true already done for you for all the APIs would be quite nice provided it could easily be generated mechanically so it had everything and was kept in sync without being a maintenance burden.

Assuming your RFC goes forward, adding delayed support might make a nice first project for learning how to extend the Rust compiler by a suitably interested party. I'm thinking of a future me :-)

@aturon
Copy link
Member

aturon commented Jan 31, 2019

cc @alexcrichton

@joshtriplett
Copy link
Member

This looks great to me! Let's make import libraries a thing of the past.

I don't think this needs any bikeshedding on names; this seems quite clear.

Copy link
Contributor

@Centril Centril left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some proof-reading and some questions.

text/0000-dll-kind.md Outdated Show resolved Hide resolved
text/0000-dll-kind.md Outdated Show resolved Hide resolved
text/0000-dll-kind.md Outdated Show resolved Hide resolved
text/0000-dll-kind.md Outdated Show resolved Hide resolved
text/0000-dll-kind.md Outdated Show resolved Hide resolved
text/0000-dll-kind.md Outdated Show resolved Hide resolved
text/0000-dll-kind.md Outdated Show resolved Hide resolved
text/0000-dll-kind.md Outdated Show resolved Hide resolved
text/0000-dll-kind.md Outdated Show resolved Hide resolved

If that were to happen, we'd no longer need to pretend the pc-windows-gnu toolchain is standalone, and we'd be able to stop bundling MinGW bits entirely in favor of the user's own MinGW installation, thereby resolving a bunch of issues such as [rust-lang/rust#53454](https://github.com/rust-lang/rust/issues/53454).

A future extension of this feature would be the ability to optionally lazily load such external functions, since Rust would naturally have all the information required to do so.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you were to "speculate", what might that look like?

Copy link

@zunzster zunzster Feb 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that might be a reference to my delayed=true optional extension discussed above e.g.

#[link(name = "kernel32.dll", kind = "dll", delayed = true)]

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd personally prefer the option where the caller of the function chooses whether to do a lazy loaded call of it, and not having to choose at declaration time whether it is lazy loaded. I don't know what the syntax would look like for that.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@retep998 Can you say a bit more about that? I'm curious as to why that's useful. Going that way would seem to preclude (or at least complicate) the compiler from being able to optimize all the generated thunks so they are not doing any repeated LoadLibrary/GetProcAddress calls. It's likely you have a use case in mind that I've not come across before which warrants giving up that benefit.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler could still ensure there is only one LoadLibrary for a given dll and one GetProcAddress for each symbol. Allowing the caller to choose whether they want to lazy load it does not prevent the compiler from ensuring GetProcAddress is only called once. If no dylib crates are involved, the generation of GetProcAddress thunks can be lazily deferred all the way until binary creation time. If dylib crates are involved, then either the dylib that contains the crate with the declarations would have to generate all the thunks, or there could potentially be some duplication across dylibs. Since nobody uses dylib except rustc and people who accidentally used it when they meant to use cdylib, it doesn't seem like too big of an issue.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As for why, it depends on whether the caller is able to deal with the function not existing. If the caller has a fallback, then it can use the lazy loaded versions and fallback if it fails to load. If the caller doesn't have any fallback, then it can use the more efficient static version.

Copy link

@Arnavion Arnavion Feb 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zunzster The user can also write such thunks themselves. It would be preferable for the user to write them since they can choose how to handle the LL/GPA failure if the DLL / function doesn't exist (crash / no-op / return a custom Err()) rather than have the compiler enforce a specific choice.

Copy link

@zunzster zunzster Feb 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Arnavion Oh, sure. I know users can write the LL/GPA thunks themselves. I've done that many a time myself. It's just tedious and hence sometimes error prone since writing robust error handling around each API call is annoying. Hence, it's nice if the compiler provides a 'pit of success' offering convenient lazy loading with robust error handling as the default.

@retep998 If you're making the lazy loading transparent to the user, you can't play with the return values since they're already a concrete type defined by some random API. So, yes, having a standard 'missing_API_handler' hook which the user can potentially override is nice. You can make the hook a no-op or your own custom handler (similar to panic handlers I suppose) but the default one should probably panic with a descriptive error and back trace.

Actually, I can't see the no-op change being especially easy though with regard to specifying what the return value (if any) should be in case of a missing API.

Some Windows API return BOOL (typedefed Integers) with 0 meaning failure.
Some APIs return HANDLE with -1 (aka INVALID_HANDLE_VALUE) for failure.

So, offering the no-op case would seem to require inelegant/complicated declarative support. I think if users want that kind of no-op behavior, they probably should have to do their own LL/GPA handling since the alternative isn't well-specified enough to be safe.

Of course, this is all just my opinion. Maybe there is a better solution I'm just not seeing since I'm used to the Delphi approach. When all you have is a hammer, every problem can start to look like a nail. :-)

Centril and others added 2 commits January 31, 2019 20:15
Co-Authored-By: retep998 <retep998@gmail.com>
@joshtriplett
Copy link
Member

joshtriplett commented Feb 1, 2019 via email

text/0000-dll-kind.md Outdated Show resolved Hide resolved
text/0000-dll-kind.md Outdated Show resolved Hide resolved
@zunzster
Copy link

zunzster commented Feb 1, 2019

@joshtriplett Might kind = "dylib" make sense as a platform-neutral term? It neatly parallels the crate-types specifiers that Rust uses i.e. dylib (and cdylib) produce a .dll, .so or .dylib respectively and this linkage spec allows such entities to be used.

@retep998
Copy link
Member Author

retep998 commented Feb 8, 2019

Oh, I just realized another benefit of this. If two dlls both provide the symbol foo, with this feature you'd be able to link to both of them at the same time!

@joshtriplett
Copy link
Member

(I didn't realize until poking rfcbot here that this was tagged with multiple teams. I don't think it needs to be a multi-team RFC.)

@rfcbot cancel

@rfcbot
Copy link
Collaborator

rfcbot commented Feb 8, 2019

@joshtriplett proposal cancelled.

@rfcbot rfcbot removed 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 Feb 8, 2019
@joshtriplett joshtriplett removed the T-compiler Relevant to the compiler team, which will review and decide on the RFC. label Feb 8, 2019
@joshtriplett
Copy link
Member

@rfcbot merge

@rfcbot
Copy link
Collaborator

rfcbot commented Feb 8, 2019

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 none object), 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!

See this document for info about what commands tagged team members can give me.

@rfcbot 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 Feb 8, 2019
@estebank

This comment has been minimized.

@Zoxc
Copy link

Zoxc commented Feb 8, 2019

I don't like how to this uses the normal form of the link attribute, which effectively apply to multiple crates, while this only applies to the extern section. Also raw-dylibs are exactly the same as dylibs, they are just linked differently. You'd also want to be able to use for static and framework kinds (so you can link to things with the same symbol from different libraries). I think we'd want a new attribute or a modifier on the link attribute instead of introducing new kinds.

@retep998
Copy link
Member Author

retep998 commented Feb 9, 2019

Frankly, I find it kind of weird that most of the time #[link] technically has nothing to do with the extern block it is attached to (the only exception being whether dllimport is applied on Windows).

Doing the equivalent of raw-dylib but for static libraries is a completely different challenge because it's just not possible with current linkers. Unlike with dlls, you need the static library around to link to it and nothing can ever change that. As for linking to the same symbol from multiple static libraries, that would require Rust to use LLD with some fairly major changes to allow Rust to dictate that sort of thing to LLD.

I am not opposed to having a new property for #[link] that would modify the dylib kind to function as described in the RFC. I'll leave it up to the lang team to decide what they think is better.

@joshtriplett
Copy link
Member

@Zoxc

Also raw-dylibs are exactly the same as dylibs, they are just linked differently.

Yes, that's the idea, and where the name raw-dylib came from.

You'd also want to be able to use for static and framework kinds (so you can link to things with the same symbol from different libraries).

That would be a completely different proposal, and one that, as @retep998 mentions, would be quite different in implementation to the extent it's possible. That shouldn't be part of this proposal, which is focused on a specific problem that's causing practical issues on Windows platforms today.

@liigo
Copy link
Contributor

liigo commented Feb 11, 2019

The new (and fixed) rendered link: https://github.com/retep998/rfcs/blob/kindly-idata-my-dlls/text/0000-raw-dylib-kind.md

By the way, how are you think about #[link(kind="cdylib")]? Rust already use crate-type = cdylib to build a dynamic library. @retep998 @joshtriplett

@retep998
Copy link
Member Author

@liigo That would imply dylib is for linking to Rust dylibs while cdylib is for linking to normal dynamic libraries, which would be excessively confusing. It's already confusing enough as is to get people to use the cdylib crate type, let's not overload that terminology here.

@aturon
Copy link
Member

aturon commented Feb 14, 2019

@rfcbot reviewed

Marking as "reviewed" to let this go forward, but I'm abstaining on this one.

@rfcbot rfcbot added final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. and removed proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. labels Feb 14, 2019
@rfcbot
Copy link
Collaborator

rfcbot commented Feb 14, 2019

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added the finished-final-comment-period The final comment period is finished for this RFC. label Feb 24, 2019
@rfcbot
Copy link
Collaborator

rfcbot commented Feb 24, 2019

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process,I would like to thank @joshtriplettfor their work and everyone else who contributed.

The RFC will be merged soon.

@rfcbot rfcbot removed the final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. label Feb 24, 2019
@retep998
Copy link
Member Author

Who is @joshtriplettfor?

@joshtriplett joshtriplett merged commit 528e0ae into rust-lang:master Feb 24, 2019
@joshtriplett
Copy link
Member

🎉 RFC 2627 is now #[link(kind="raw-dylib")]! 🎉

Tracking issue: rust-lang/rust#58713

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linkage Proposals relating to the linking step. A-windows Proposals relating to Windows. disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this RFC. T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet