Bug 134237

Summary: [Win] MSVC mishandles enums in bitfields - breaks Cross-Origin Access Control
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: WebCore Misc.Assignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: allan.jensen, andersca, bfulgham, bunhere, cdumez, commit-queue, esprehn+autocc, glenn, gyuyoung.kim, japhet, kling, koivisto, kondapallykalyan, macpherson, menard, sergio, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 126972    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch msaboff: review+

Brent Fulgham
Reported 2014-06-23 21:31:18 PDT
Consider this code in CrossOriginAccessControl.cpp (passesAccessControlCheck): if (accessControlOriginString == "*" && includeCredentials == DoNotAllowStoredCredentials) Amazingly, although includeCredentials is set to 'DoNotAllowStoredCredentials', this test failed: > p includeCredentials DoNotAllowStoredCredentials | -2 (-1) > p DoNotAllowStoredCredentials DoNotAllowStoredCredentials (1) > DoNotAllowStoredCredentials == includeCredentials false This change was introduced in http://trac.webkit.org/changeset/161958/trunk/Source/WebCore/loader/ResourceLoaderOptions.h. MSVC does strange things with bit fields containing enumerations. This has been documented in a few places: http://connect.microsoft.com/VisualStudio/feedback/details/828892/vc-2013-miscompilation-with-enums-and-bit-fields http://objectmix.com/c/749570-enum-bitfield-visual-studio-bug-not-3.html Small test case: #include <iostream> enum E { A = 0, B, C, D }; struct S { E e : 2; }; int main() { S s; s.e = D; std::cout << s.e << ' ' << (s.e == D) << std::endl; } Output on MSVC: -1 0 Output on G++/clang++ (correct according to the C++ standard): 3 1 Microsoft appears to think this behavior is within specification, since their enumerated type is based on int, not unsigned. They use a sign bit, which means our one-bit enum fields only ever hold the sign bit resulting in weird behavior.
Attachments
Patch (2.01 KB, patch)
2014-06-23 21:48 PDT, Brent Fulgham
no flags
Patch (6.85 KB, patch)
2014-06-24 10:01 PDT, Brent Fulgham
no flags
Patch (38.26 KB, patch)
2014-06-24 11:17 PDT, Brent Fulgham
msaboff: review+
Radar WebKit Bug Importer
Comment 1 2014-06-23 21:40:02 PDT
Brent Fulgham
Comment 2 2014-06-23 21:48:44 PDT
Anders Carlsson
Comment 3 2014-06-24 07:11:27 PDT
Comment on attachment 233680 [details] Patch I think what we usually do in cases like this is to just use unsigned integer types, let's do that instead.
Brent Fulgham
Comment 4 2014-06-24 10:01:05 PDT
Brent Fulgham
Comment 5 2014-06-24 10:01:43 PDT
I found a few other cases where this was happening and have expanded the bug to cover them.
Brent Fulgham
Comment 6 2014-06-24 11:17:41 PDT
Brent Fulgham
Comment 7 2014-06-24 11:20:24 PDT
See also Bug 134252.
Michael Saboff
Comment 8 2014-06-24 11:42:43 PDT
Comment on attachment 233720 [details] Patch r=me. Fix Mac build.
Brent Fulgham
Comment 9 2014-06-24 12:49:53 PDT
Note You need to log in before you can comment on or make changes to this bug.