Fix MSVC signed enum bitfield in JSClosureVar

[dff096]

Fix crash issue (as soon as campaign or tutorial loads) when building 3rdparty/quickjs-wz/ and Warzone with MSVC.

The official Warzone releases are not built with MSVC and consider enum bitfields as unsigned. But when built with MSVC (Visual Studio), closure_type's 3-bit enum bitfield is interpreted as signed, causing values 4-7 to be read as -4 to -1.

The enum has values 0-7 (for example JS_CLOSURE_GLOBAL_DECL = 4). A 3-bit field can store 0-7.

MSVC treats enum bitfields as signed by default. A signed 3-bit field has range -4 to +3. So when the code uses value 4, MSVC read it back as -4 (same bit pattern 100, different interpretation).

The fix changes the type to uint8_t which is explicitly unsigned.

The only minor downside is losing the enum type annotation - you lose compile-time checking that only valid JSClosureTypeEnum values are assigned.

[past-due]

Thanks for the catch!

Out of curiosity, which version of MSVC did you encounter this on?

(Part of the reason I ask is apparently Visual Studio 2022 version 17.4+ added support for the /Zc:enumTypes compiler option, which also sounds like it might fix this. Would you be able to test if it does? - I'd feel more comfortable using that so we don't miss another instance of this in future QuickJS updates.)

[dff096]

• Compiler: Microsoft C/C++ 19.38.33145 for x64
• Toolset: 14.38.33130
• IDE: Visual Studio 2022 Professional

The 19.38 compiler version corresponds to Visual Studio 2022 version 17.8.

Agreed that a compiler option fix would be preferred. I'll trial that.

[dff096]

I reverted the change and added
xcheck_add_c_compiler_flag(/Zc:enumTypes) to CMakeLists.txt for quickjs-wz
Crashes as before in the same place (js_closure2) line 16986 because cv->closure_type is -4.
I wonder if it's maybe because it's compiled as C and this is a C++ flag? In C, the standard says enum bitfield signedness is implementation-defined whether a plain int bitfield (or enum bitfield, which MSVC treats as int) is signed or unsigned.
I'll dig into it a bit more.

[past-due]

I reverted the change and added xcheck_add_c_compiler_flag(/Zc:enumTypes) to CMakeLists.txt for quickjs-wz Crashes as before in the same place (js_closure2) line 16986 because cv->closure_type is -4. I wonder if it's maybe because it's compiled as C and this is a C++ flag? In C, the standard says enum bitfield signedness is implementation-defined whether a plain int bitfield (or enum bitfield, which MSVC treats as int) is signed or unsigned. I'll dig into it a bit more.

I was wondering if that might be the case. (Did it actually add the compiler flag?)

[dff096]

Yeah. I see %(AdditionalOptions) ... /utf-8 /Zc:enumTypes in the vcxproj output and then CL.exe invocation with verbose output shows...

• /std:c11 - compiling as C11 (not C++)
• /TC - treat files as C source (not C++)

So the compiler is definitely is doing its thing in C, and /Zc:enumTypes is a C++ conformance flag that has presumably has no effect on C compilation.

[past-due]

Welp, looks like this is the best fix we have then. (Hopefully this isn't too frequent of a pattern for upstream.)

Many thanks for all of your testing and investigation!

EDIT: It looks like this would be worth contributing upstream to bellard/quickjs as well. This code appears to be relying on implementation-defined behavior: there is no guarantee of the underlying type, nor an ability to explicitly specify the type of an enum prior to C23 - and I believe the code is written to a much earlier standard. A compiler may choose an unsigned type, which apparently GCC/Clang often(?) do, but it's entirely implementation-defined.