Bug 141498

Summary: [Win] [64-bit] Work around MSVC2013 Runtime Bug
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: WebKit Misc.Assignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, andersca, benjamin, bfulgham, cmarcelo, commit-queue, dbates, roger_fong, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Bug Depends on:    
Bug Blocks: 141499    
Attachments:
Description Flags
Patch
none
Patch andersca: review+

Description Brent Fulgham 2015-02-11 20:26:58 PST
Microsoft Visual Studio 2013 has a bug in their 64-bit runtime libraries that causes a crash with error "0xC000001D: Illegal Instruction" if the CPU does not support the AVX2 instruction set. Apparently, the runtime library does not properly recognize when the AVX2 set is not present (or if it has been disabled in the firmware) and attempts to use them regardless of CPU support. Microsoft apparently does not plan to address this in any VS2013 update. The VS2015 environment does have a correction for this.

Until Microsoft releases VS2015 (and this project migrates to it), our only recourse is to disable the use of FMA3 in the math library.

See <https://connect.microsoft.com/VisualStudio/feedback/details/811093/visual-studio-2013-rtm-c-x64-code-generation-bug-for-avx2-instructions> for some comments from Microsoft on this topic.
Comment 1 Brent Fulgham 2015-02-11 20:27:29 PST
<rdar://problem/19803642>
Comment 2 Brent Fulgham 2015-02-11 20:32:12 PST
Microsoft: "“Thank you for reporting this issue. This is indeed a bug in the instruction set availability detection in the math library of the Visual C++ 2013 C Runtime (CRT). The bug occurs when [1] the CPU supports FMA3 and [2] either the operating system does not support AVX or support for AVX is disabled in the operating system."

Note that in addition to the hardware restraints I mentioned initially, users can actually create this issue by disabling the AVX instruction sets (even if the CPU does support them):

Disable AVX: bcdedit /set xsavedisable 1

You can re-enable it by:
    bcdedit /set xsavedisable 0

-or-

    bcdedit /deletevalue xsavedisable
Comment 3 Brent Fulgham 2015-02-11 22:05:57 PST
Created attachment 246424 [details]
Patch
Comment 4 Brent Fulgham 2015-02-11 22:10:01 PST
Created attachment 246425 [details]
Patch
Comment 5 Anders Carlsson 2015-02-12 07:33:06 PST
Comment on attachment 246425 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=246425&action=review

> Source/JavaScriptCore/jsc.cpp:1177
> +    // TODO(141449): Remove this workaround when we switch to VS2015+.

Please use FIXME: here (and a proper bug URL!) Here and all other places.
Comment 6 Alex Christensen 2015-02-12 07:50:40 PST
I think we should put this into WebKitCreateInstance instead of each executable that uses it.  That way it will fix 3rd party applications, too.
Comment 7 Brent Fulgham 2015-02-12 09:09:19 PST
(In reply to comment #6)
> I think we should put this into WebKitCreateInstance instead of each
> executable that uses it.  That way it will fix 3rd party applications, too.

The problem is that we have some clients that only use JavaScriptCore.dll, and test programs that only link against WTF, etc., for test purposes.
Comment 8 Brent Fulgham 2015-02-12 09:14:11 PST
(In reply to comment #7)
> (In reply to comment #6)
> > I think we should put this into WebKitCreateInstance instead of each
> > executable that uses it.  That way it will fix 3rd party applications, too.
> 
> The problem is that we have some clients that only use JavaScriptCore.dll,
> and test programs that only link against WTF, etc., for test purposes.

Also, by putting it in the DllMain we do fix third party applications. We have to pepper all of our code with this setting because we are building with static MSVC runtime linking.

We should be able to go back to dynamic linking soon, which would reduce the need to set this flag all over the place.
Comment 9 Brent Fulgham 2015-02-12 09:20:12 PST
Committed r179993: <http://trac.webkit.org/changeset/179993>