Package all required VS2015 CRT DLLs

👁 Image	zhoubcfan@163.com Reporter
	Description • 10 years ago

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:39.0) Gecko/20100101 Firefox/39.0 Build ID: 20150802161439 Steps to reproduce: From bug 119072 appcrt140.dll and desktopcrt140.dll are removed in vs2015 ctp6 This is covered in a recent blog post: http://blogs.msdn.com/b/vcblog/archive/2015/03/03/introducing-the-universal-crt.aspx

👁 Image	zhoubcfan@163.com Reporter
	Updated • 10 years ago

👁 Image	Stefan Sitter [:ssitter]
	Comment 1 • 10 years ago

I can confirm this error when building Thunderbird, output is similar to Bug 1119082 Comment 13. My current workaround is to unset WIN32_REDIST_DIR before building. Any plans when Microsoft Visual Studio 2015 2015 RTM will be supported?

👁 Image	poiru Assignee
	Comment 2 • 10 years ago

The CRT in VS2015 has been split into numerous components and uses the universal CRT[0]. For example, in a VS2015 build, xul.dll imports symbols from: MSVCP140.dll VCRUNTIME140.dll api-ms-win-crt-math-l1-1-0.dll api-ms-win-crt-runtime-l1-1-0.dll api-ms-win-crt-stdio-l1-1-0.dll api-ms-win-crt-time-l1-1-0.dll api-ms-win-crt-convert-l1-1-0.dll api-ms-win-crt-environment-l1-1-0.dll api-ms-win-crt-string-l1-1-0.dll api-ms-win-crt-utility-l1-1-0.dll api-ms-win-crt-filesystem-l1-1-0.dll api-ms-win-crt-locale-l1-1-0.dll ucrtbase.dll (implicitly) We should either package all of the api-ms-win-*.dll binaries or perhaps write a script that packages the required subset based on the imports of xul.dll and friends. Note that there are 40 (yeah...) api-ms-win*.dll files in: C:\Program Files (x86)\Windows Kits\10\Redist\ucrt\DLLs\x86 [0]: http://blogs.msdn.com/b/vcblog/archive/2015/03/03/introducing-the-universal-crt.aspx

👁 Image	poiru Assignee
	Comment 3 • 10 years ago

👁 Image	poiru Assignee
	Updated • 10 years ago

👁 Image	poiru Assignee
	Comment 4 • 10 years ago

I checked again and it seems we need all api-ms-win-core-*.dll for ucrtbase.dll and most of api-ms-win-crt-*.dll for msvcrt140.dll/vcruntime140.dll (so we might as well package them all). Not sure who to ask so I'll default to Ehsan and Benjamin. How would you feel about littering the installation directory with 41 new DLLs? Have we considered statically linking to the CRT? If so, why did we decide not do it? I read through various old bugs, but it would be nice to get a definitive answer.

👁 Image	poiru Assignee
	Updated • 10 years ago

👁 Image	(not currently active) Ted Mielczarek
	Comment 5 • 10 years ago

Yowza! Do they get delayloaded or anything? Just shipping extra DLLs isn't that bad (assuming their on-disk size adds up to roughly the same), but loading a bunch more DLLs on startup could suck. I don't think we can statically link the CRT because of the way we implement jemalloc to replace the CRT malloc, but glandium might have a more definitive answer there.

👁 Image	(no longer active)
	Comment 6 • 10 years ago

What Ted said. I believe this is related: <https://dxr.mozilla.org/mozilla-central/source/mozglue/crt/Makefile.in#10> I think we should package up the new DLLs like we did the old ones. But can you please see what the on-disk size difference looks like?

👁 Image	(no longer active)
	Updated • 10 years ago

👁 Image	poiru Assignee
	Comment 7 • 10 years ago

(In reply to :Ehsan Akhgari from comment #6) > But can you please see what the on-disk size difference looks like? For Windows Try opt builds, VS2013 installer is 36231728 bytes and VS2015 installer is 36409024 so the difference is negligible. I have attached a comparison of the installer contents.

👁 Image	poiru Assignee
	Comment 8 • 10 years ago

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #5) > Yowza! Do they get delayloaded or anything? Just shipping extra DLLs isn't > that bad (assuming their on-disk size adds up to roughly the same), but > loading a bunch more DLLs on startup could suck. They are not delay-loaded. The api-ms-win-* DLLs are small stubs (~20KB) whose exported functions simply redirect to kernel32.dll (in the case of api-ms-win-core-*.dll) or ucrtbase.dll (in the case of api-ms-win-crt-*.dll). In Win7 and later, the loader treats api-ms-win-core-*.dll imports differently. It never loads the DLLs (they don't even exist on disk), but rather resolves the imports with a kernelbase.dll function. In Win10, where the Universal CRT is a OS component, the loader performs similar trickery to resolve api-ms-win-crt-*.dll imports with a ucrtbase.dll function. FWIW, the Universal CRT is being distributed as an OS update for Vista SP2 and later, but we of course still need package it to support all users. (In reply to :Ehsan Akhgari from comment #6) > What Ted said. I believe this is related: > <https://dxr.mozilla.org/mozilla-central/source/mozglue/crt/Makefile.in#10> AFAICT, that isn't needed for VS2015 (see bug 1236330). If we want to continue linking dynamically to the CRT, I think we can switch firefox.exe and plugin-container.exe back to dynamic linking as well (the CRT patch[0] is no longer needed with VS2015). I'll try it and file a separate bug if that works. [0]: http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/WindowsCrtPatch.h

👁 Image	(no longer active)
	Comment 9 • 10 years ago

IIRC we had to link firefox.exe statically because of Windows XP SP2 compatibility. See bug 1023941. Not sure how that changes with VS2015 but it's worth checking.

👁 Image	poiru Assignee
	Comment 10 • 10 years ago

(In reply to :Ehsan Akhgari from comment #9) > IIRC we had to link firefox.exe statically because of Windows XP SP2 > compatibility. See bug 1023941. Not sure how that changes with VS2015 but > it's worth checking. I filed bug 1236931 for this.

👁 Image	(not currently active) Ted Mielczarek
	Comment 11 • 10 years ago

(In reply to Birunthan Mohanathas [:poiru] from comment #8) > They are not delay-loaded. > > The api-ms-win-* DLLs are small stubs (~20KB) whose exported functions > simply redirect to kernel32.dll (in the case of api-ms-win-core-*.dll) or > ucrtbase.dll (in the case of api-ms-win-crt-*.dll). > > In Win7 and later, the loader treats api-ms-win-core-*.dll imports > differently. It never loads the DLLs (they don't even exist on disk), but > rather resolves the imports with a kernelbase.dll function. In Win10, where > the Universal CRT is a OS component, the loader performs similar trickery to > resolve api-ms-win-crt-*.dll imports with a ucrtbase.dll function. > > FWIW, the Universal CRT is being distributed as an OS update for Vista SP2 > and later, but we of course still need package it to support all users. Okay, I guess I'd want to see what the Talos numbers look like here, specifically the xperf numbers (although we only have those on Win7+, I think).

👁 Image	Benjamin Smedberg
	Comment 12 • 10 years ago

I don't have a lot to add here, except the following concerns: #1: we need to make sure that we can legally distribute these DLLs. If the license has changed from prior versions of MSVC, that requires a legal review. #2: extra DLLs normally mean perf overhead. Have we measured/are we sure that this is reasonable from a startup perf perspective?

👁 Image	Alex Kontos
	Comment 13 • 10 years ago

Just a small note here (hopefully it's relevant). I've done a vanilla build of the 44.0 release codebase using VS2015 and the build won't run on my test systems without the 2015 Redistributable installed on the system, even if the relevant DLLs are included in the same directory as xul.dll. Just for good measure I put all the DLLs from C:\Program Files (x86)\Windows Kits\10\Redist\ucrt\DLLs\x64 and C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\redist\x64\Microsoft.VC140.CRT\

👁 Image	poiru Assignee
	Comment 14 • 10 years ago

👁 Image	(no longer active)
	Comment 15 • 10 years ago

Comment on attachment 8713687 [details] [diff] [review] Package Universal CRT DLLs for VS2015 Review of attachment 8713687 [details] [diff] [review]: ----------------------------------------------------------------- r=me on the idea, but I'd like a build system peer to also look at this from a build system perspective.

👁 Image	Mike Hommey [:glandium] OoO until Jun 18
	Comment 16 • 10 years ago

Comment on attachment 8713687 [details] [diff] [review] Package Universal CRT DLLs for VS2015 Review of attachment 8713687 [details] [diff] [review]: ----------------------------------------------------------------- I'd rather use WIN32_REDIST_DIR than add a new variable for this. It's easy enough to detect what type of CRT the directory contains.

👁 Image	Masatoshi Kimura [:emk]
	Comment 17 • 10 years ago

Are you saying WIN32_REDIST_DIR should support multiple directories? The UCRT directory is different from the MSVC CRT directory.

👁 Image	Masatoshi Kimura [:emk]
	Comment 18 • 10 years ago

And MSVC 2015 requires both CRTs.

👁 Image	Mike Hommey [:glandium] OoO until Jun 18
	Comment 19 • 10 years ago

Comment on attachment 8713687 [details] [diff] [review] Package Universal CRT DLLs for VS2015 Review of attachment 8713687 [details] [diff] [review]: ----------------------------------------------------------------- Ah, I missed the fact that both are required. Sucks that we'd need to set two variables :(

👁 Image	Mike Hommey [:glandium] OoO until Jun 18
	Updated • 10 years ago

👁 Image	poiru Assignee
	Comment 20 • 10 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/74dd4af2ec4a9d593f0febbe1e13825c92cce0b4 Bug 1204202 - Remove packaging for now obsolete appcrt140.dll/desktopcrt140.dll on VS2015. r=ehsan https://hg.mozilla.org/integration/mozilla-inbound/rev/0fc068ae3289e1343df8771a79581942c2e027c9 Bug 1204202 - Package Universal CRT DLLs for VS2015. r=glandium,ehsan

👁 Image	Carsten Book [:Tomcat]
	Comment 21 • 10 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/74dd4af2ec4a https://hg.mozilla.org/mozilla-central/rev/0fc068ae3289

👁 Image	Masatoshi Kimura [:emk]
	Updated • 10 years ago

👁 Image	zhoubcfan@163.com Reporter
	Comment 22 • 10 years ago

(In reply to Alex Kontos from comment #13) > Just a small note here (hopefully it's relevant). I've done a vanilla build > of the 44.0 release codebase using VS2015 and the build won't run on my test > systems without the 2015 Redistributable installed on the system, even if > the relevant DLLs are included in the same directory as xul.dll. > > Just for good measure I put all the DLLs from C:\Program Files (x86)\Windows > Kits\10\Redist\ucrt\DLLs\x64 and C:\Program Files (x86)\Microsoft Visual > Studio 14.0\VC\redist\x64\Microsoft.VC140.CRT\ https://blogs.msdn.microsoft.com/vcblog/2015/03/03/introducing-the-universal-crt/ You may need newer binaries.

👁 Image	Frank-Rainer Grahl (:frg)
	Updated • 9 years ago

👁 Image	(Away)
	Comment 23 • 9 years ago

> In Win7 and later, the loader treats api-ms-win-core-*.dll imports > differently. It never loads the DLLs (they don't even exist on disk), but > rather resolves the imports with a kernelbase.dll function. I was hoping that the xp-eol bug 1130266 would mean that we could unship these api-ms-win-core-*.dll files, but on a plain Win7 build I still needed a handful of them to be in the installation directory. So much for that idea.

👁 Image	Masatoshi Kimura [:emk]
	Comment 24 • 9 years ago

Run vcredist_(x86|x64) on Win7 and see what is installed. The installer will only install required files. On Vista, it will install all files.

👁 Image	BMO Automation
	Updated • 8 years ago

👁 Image	Masatoshi Kimura [:emk]
	Updated • 5 years ago