Bug 134237 – [Win] MSVC mishandles enums in bitfields - breaks Cross-Origin Access Control

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Brent Fulgham]

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.

[Radar WebKit Bug Importer]

<rdar://problem/17430588>

[Brent Fulgham]

Created attachment 233680 [details]
Patch

[Anders Carlsson]

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]

Created attachment 233714 [details]
Patch

[Brent Fulgham]

I found a few other cases where this was happening and have expanded the bug to cover them.

[Brent Fulgham]

Created attachment 233720 [details]
Patch

[Brent Fulgham]

See also Bug 134252.

[Michael Saboff]

Comment on attachment 233720 [details]
Patch

r=me.  Fix Mac build.

[Brent Fulgham]

Committed r170381: <http://trac.webkit.org/changeset/170381>