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

Don't get memcpy, memmove, memcmp, memset, and strlen from msvcrt.dll #29130

Closed
alexchandel opened this issue Oct 18, 2015 · 35 comments
Closed
Labels
A-codegen Area: Code generation C-feature-request Category: A feature request, i.e: not implemented / a PR. O-windows Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@alexchandel
Copy link

The memcpy, memmove, memcmp, strlen, and memset symbols in msvcrt are just shims for implementations in ntdll.dll, which have existed since Windows 3.1.

If possible, we should link against these directly, rather than going through msvcrt. This would have the effect of eliminating all of our dependencies on msvcrt except for math functions and the entry point.

cc @retep998

@sfackler sfackler added the O-windows Operating system: Windows label Oct 18, 2015
@retep998
Copy link
Member

These functions are not shims as I have just disassembled and verified this. Also ntdll.dll is an explicitly unstable API that should not be relied on directly so I don't recommend linking to it directly.

@alexchandel
Copy link
Author

@retep998 At least on very old systems, msvcrt.dll did call into ntdll.dll for them. That's rather surprising that they would be called unstable, being there unchanged for 25 years. Can you find something on MSDN marking these symbols as unstable?

@retep998
Copy link
Member

@alexchandel ntdll.dll is the interface to the NT kernel. The NT kernel is free to add or remove functions with each version of NT. Just because something hasn't changed does not mean Microsoft provides any sort of promise of stability. ntdll.dll has functions like memcpy and friends because it is providing them for other system libraries. It's not intended to be directly used by user-mode applications.

@alexchandel
Copy link
Author

@retep998 I know it has all the syscalls, and that their syscalls are unstable, but I can't find any documentation at all on what symbols are in the current version. Which is uncharacteristic of MS.

But that's too bad if we can't, because unless we provide our own implementations of them (alongside an allocator like jemalloc and a math lib like openlibm), we're still tied to MS's CRT>

@retep998
Copy link
Member

@alexchandel We don't rely on the CRT for the allocator, even without jemalloc. Replacing the math memory functions with our own optimized versions should be quite doable with the new simd stuff that @huonw is working on.

@huonw
Copy link
Member

huonw commented Oct 18, 2015

(I don't think implementing our own libm-replacement is a great idea---using a preexisting one like openlibm seems better for various reasons---and, even then, SIMD isn't so useful for scalar functions like those in libm.)

@abonander
Copy link
Contributor

@huonw Can you elaborate on a few of these reasons? From what I understand, memcpy, memmove and memset have relatively trivial implementations. I realize there's probably loads of platform- and architecture-specific optimizations in the big libs but can't we still get reasonably fast with our own implementation? If it eliminates a problematic dependency I'm all for it. At the very least, it could be a viable alternative.

@alexchandel
Copy link
Author

@cybergeek94 I think he meant implementing math functions. @huonw you're working on SIMD stuff for memory?

@huonw
Copy link
Member

huonw commented Oct 18, 2015

No, I'm not. I was replying to @retep998's original comment which had a typo: it said "math" instead of "memory".

In any case, rlibc implements the memory functions Rust needs.

@alexchandel
Copy link
Author

@huonw how does one have rustc link against rlibc instead of msvcrt?

Combined with openlibm and an entry point, that would simplify a cross-compile to windows to LIB=$HOME/i686-pc-windows-msvc-w10/lib rustc --target i686-pc-windows-msvc test.rs, since LLD plays nicely now (#29126)

@briansmith
Copy link
Contributor

Why not just remove all uses of memcpy, memmove, memcmp, strlen, and memset from the Rust code completely, writing them all in Rust? Not only would that be better than relying on an unstable Windows API, but also that effort would help other efforts for libc-less Rust.

@huonw
Copy link
Member

huonw commented Jan 31, 2016

LLVM inserts calls to (some of) those functions during optimisation passes.

@briansmith
Copy link
Contributor

Why not just turn off those optimizations in LLVM? That would be useful for some users of #![no_std] too.

@retep998
Copy link
Member

Because then you'd be harming performance for very little gain.

@steveklabnik
Copy link
Member

Also, https://crates.io/crates/rlibc/

@briansmith
Copy link
Contributor

Also, https://crates.io/crates/rlibc/

This is my point. rlibc-based Rust should be made to be fast, and if it is made fast, then disabling the lowering to memcpy/memset/etc. wouldn't matter.

I think it would be good to measure the performance impact.

Also, Rust shouldn't be using strlen at all (it might currently be, but that should be changed if so). LLVM isn't lowering anything to strlen, is it?

@abonander
Copy link
Contributor

Also, Rust shouldn't be using strlen at all (it might currently be, but that should be changed if so). LLVM isn't lowering anything to strlen, is it?

It might lower the length-finding op of std::ffi::CStr::from_ptr() to strlen since they're essentially equivalent.

@retep998
Copy link
Member

retep998 commented Feb 2, 2016

I have never personally seen llvm lower length finding ops to strlen but we do directly call it in std. https://github.com/rust-lang/rust/blob/master/src/libstd/ffi/c_str.rs#L435

@abonander
Copy link
Contributor

I guess I should have looked at the source for that method. I could have sworn I remembered it being implemented in Rust instead of shelling out to libc.

@bluss
Copy link
Member

bluss commented Feb 4, 2016

@briansmith rlibc is intended to be able to supply the implementations that llvm refers to in its loop recognition, memcpy (and others) insertion, etc. No reason to remove that optimization if it points to the pure rust code you wanted.

[To improve ] It sounds like a cool project in the long term, but you should look into how huge the code bases for providing micro-architecture specific implementations to these functions are in some libc implementations, for a sense of the scale of it. (For example, there are 68 asm files for memcpy in glibc).

@lygstate
Copy link
Contributor

So we could use rlibc instead when memcpy, memmove, memcmp, strlen, and memset symbols in msvcrt are not exist in ntdll.dll

@alexchandel alexchandel changed the title Get memcpy, memmove, memcmp, strlen, and memset directly from ntdll.dll Don't get memcpy, memmove, memcmp, memset, and strlen from msvcrt.dll Mar 17, 2016
@alexchandel
Copy link
Author

They always exist in ntdll.dll, and probably always will.

This is slightly less of an issue once #30027 lands, since we could link against msvcrt.dlls symbols without any toolkit.

@DemiMarie
Copy link
Contributor

What about providing implementations in libcore in assembly on Windows? Windows only supports 32- and 64- bit x86 and ARM systems, so the number of platforms is reasonable.

@sfackler
Copy link
Member

sfackler commented Jul 10, 2016

@DemiMarie high performance memcpy/memmove/etc implementations are extremely complex. Here is part of glibc's memcmp implementation: https://sourceware.org/git/gitweb.cgi?p=glibc.git;a=blob;f=sysdeps/x86_64/multiarch/memcmp-sse4.S;h=786f87282c08c521b579a15d306830f6263debfb;hb=HEAD. That's only used for x86_64 processors that support SSE4. There are two more completely separate implementations for other x86_64 generations alone.

It does not seem to be a good use of resources to try to maintain our own versions when we can rely on implementations provided by the system.

@lygstate
Copy link
Contributor

why not depends on the system msvcrt under system32 dir

@DemiMarie
Copy link
Contributor

What about ntdll.dll? It is on all Windows systems, and provides such
functions.

Another option is to use one from any of the permissively-licensed libc
implementations. I did not say that we should write our own — musl libc
should have one, as do the BSDs. That is assuming of course that this is
viable — it very well might not be.
On Jul 10, 2016 01:21, "Yonggang Luo" notifications@github.com wrote:

why not depends on the system msvcrt under system32 dir


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#29130 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AGGWByxoHXdqCSSOOlxV4clmJpoSeCWKks5qUIFRgaJpZM4GQ0vd
.

@alexchandel
Copy link
Author

alexchandel commented Jul 12, 2016

Currently the options are:

  • use the memcpy/etc symbols in ntdll.dll, which are undocumented and may go away (but probably won't). C code statically linked with Rust must use a different method.
  • use the memcpy/etc symbols in msvcrt.dll, which are provided for compatibility with old Windows programs and won't go away ever. The VC6 and MinGW toolchains can compile C code against this as well. (Note: Rust seems now to depend on symbols added after VC6)
  • use the memcpy/etc symbols in msvcrXYZ.dll, which won't go away but aren't always present. This forces Rust programs to either ship the VS Redistributable or to depend on a Windows update. The VC7-VC13 toolchains can compile C code against this as well.
  • use the memcpy/etc symbols in ucrtbase.dll, which won't go away but aren't always present. This forces Rust programs to either ship the UCRT Redistributable or to depend on a Windows update. The VC14 toolchain can compile C code against this as well, which must then ship the VCRuntime redistributable.
  • take the memcpy/etc assembly from an open-source C library like MinGW (or ask MS to open-source the UCRT), and compile that into Rust programs. C code statically linked with Rust must use a different method.
  • write our own memcpy/etc code and compile that into Rust programs. C code statically linked with Rust must use a different method.

Per #30027 none of these truly requires Rust to use a toolchain. And Rust code and C code statically linked with Rust can use different methods.

@retep998
Copy link
Member

@alexchandel Don't forget that we could just statically link the CRT which avoids the dependence on redistributables. rust-lang/libc#290

@briansmith
Copy link
Contributor

Currently the options are:

I would prefer for Rust to use the same implementation that the C compiler would use, so that when I link C and Rust code together, they are using a common implementation.

@alexchandel
Copy link
Author

@retep998 When you statically link against the UCRT in MSVC 14+, does the Rust-only executable have any dependence on the new VCRUNTIME*.DLL (or the static version, libvcruntime.lib)? It's still required to link both libucrt.lib and libcmt.lib, right?

And on a parallel note, if C object files are linked into this Rust binary as well, does linking libvcruntime.lib provide the static equivalent of linking VCRUNTIME*.DLL?

@retep998
Copy link
Member

By linking to libcmt.lib you also link to both libucrt.lib and libvcruntime.lib and end up with all the CRT stuff statically linked. No vc runtime dlls, no crt dlls. Mixing static and dll versions of the ucrt and vcruntime is not supported.

@lygstate
Copy link
Contributor

I think we only need static link to libvcruntime.lib for memmove things

@retep998
Copy link
Member

@lygstate You already can statically link to the CRT. #37545 landed recently although it is still unstable.

@Mark-Simulacrum Mark-Simulacrum added the C-feature-request Category: A feature request, i.e: not implemented / a PR. label Jul 24, 2017
@sanmai-NL
Copy link

@alexchandel: reviewing your last exchange with @retep998, I would like to ask whether you still wish to keep this issue open?

@BatmanAoD BatmanAoD added A-codegen Area: Code generation T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 13, 2019
@retep998
Copy link
Member

Given that Rust provides the ability to statically link the CRT, and there are plans to provide a pure Rust Windows target in the future when #58713 is implemented, closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation C-feature-request Category: A feature request, i.e: not implemented / a PR. O-windows Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests