| 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: |
|
||||||||||
Created attachment 233680 [details]
Patch
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.
Created attachment 233714 [details]
Patch
I found a few other cases where this was happening and have expanded the bug to cover them. Created attachment 233720 [details]
Patch
See also Bug 134252. Comment on attachment 233720 [details]
Patch
r=me. Fix Mac build.
Committed r170381: <http://trac.webkit.org/changeset/170381> |
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.