1236931 - Switch firefox.exe and friends back to dynamic CRT linking for VS2015

Status: RESOLVED DUPLICATE of bug 1035125

(Firefox Build System :: General, defect)

Description

[Biru [:poiru]]

In VS2013, msvcr120.dll imports kernel32!GetLogicalProcessorInformation, which is not available on Windows XP SP2. To make the VS2013 CRT work on XP SP2, bug 1023941 introduced WindowsCrtPatch[0].

In VS2015, the GetLogicalProcessorInformation requirement has moved into concrt140.dll (concurrency runtime). Firefox doesn't use the concurrency runtime at all so our binaries don't directly depend on concrt140.dll. There is an indirect dependency through msvcp140.dll, which implements the guts of std::condition_variable and std::mutex using the concurrency runtime. Thankfully, msvcp140.dll delay-loads concrt140.dll. Because Gecko code doesn't use std::mutex or std::condition_variable at all, we will never trigger the delay-load.

This means we can effectively revert the changes introduced by bug 1023941 when we switch to VS2015.

When we do this, we should add `MOZ_ASSERT(!GetModuleHandle(L"concrt140.dll"))` to both firefox.exe and xul.dll shutdown (and, optionally, startup) in order to make sure that we notice if some new code happens to trigger the concrt140.dll delay-load. Note that concrt140.dll won't be packaged into the installer, so triggering the delay-load will terminate Firefox (unless concrt140.dll happens to be elsewhere in the PATH).

[0]: http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/WindowsCrtPatch.h

Comment 1

[Masatoshi Kimura [:emk]]

(In reply to Birunthan Mohanathas [:poiru] from comment #0)
> When we do this, we should add
> `MOZ_ASSERT(!GetModuleHandle(L"concrt140.dll"))` to both firefox.exe and
> xul.dll shutdown (and, optionally, startup) in order to make sure that we
> notice if some new code happens to trigger the concrt140.dll delay-load.
> Note that concrt140.dll won't be packaged into the installer, so triggering
> the delay-load will terminate Firefox (unless concrt140.dll happens to be
> elsewhere in the PATH).

Where to add? The code may delay-load the concrt140.dll after our check.

Comment 2

[Masatoshi Kimura [:emk]]

Ah, I must have missed "shutdown" somehow.
How about using the notification hook to catch the error earlier?
https://msdn.microsoft.com/en-us/library/z9h1h6ty.aspx

Comment 3

[Biru [:poiru]]

(In reply to Masatoshi Kimura [:emk] from comment #2)
> Ah, I must have missed "shutdown" somehow.
> How about using the notification hook to catch the error earlier?
> https://msdn.microsoft.com/en-us/library/z9h1h6ty.aspx

AFAIK the notification hooks are supposed to be used from within the binary that does the delay-loading. I think we would need to find where msvcp140.dll's __pfnDliNotifyHook2 is in memory and change it. It would probably be easier and safer to just create our own version of concrt140.dll that exports the relevant functions with stubs that print out an error and terminate the process.

But I don't think any of this is really necessary. Assuming that concrt140.dll will not be present on the test machines (it won't unless something installs it to system32), Firefox will just crash.

Comment 4

[Yuhong Bao]

I was wanting to kill XP SP2 support completely when we move to VS2015.

Comment 5

[Mike Hommey [:glandium]]

Depending how delay-loading works internally, putting the lib in our dll blacklist might work.

Comment 6

[(no longer active)]

(In reply to Mike Hommey (VAC: 31 Dec - 11 Jan) [:glandium] from comment #5)
> Depending how delay-loading works internally, putting the lib in our dll
> blacklist might work.

I'd expect that no matter what it does it ends up calling LdrLoadDll, but I agree with poiru that this trouble is not really worth it.

Comment 7

[(Away)]

This was done by parts 1 through 5 of bug 1035125.