Created attachment 430269 [details] test case The attached program triggers an assertion failure when running jsc in 32 bits and debug mode. In 32 bits, asCell() returns a pointer to the payload and using reinterpret_cast will sign extend asNumber to FFFF FFFF + <value-address>, which will fail the isImpureNaN in jsNumber. So first cast the address to uint32_t then to uint64_t to prevent the tag corruption.
Created attachment 430271 [details] Patch
Created attachment 430274 [details] Patch
Comment on attachment 430274 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=430274&action=review > Source/JavaScriptCore/jsc.cpp:1493 > +#if USE(JSVALUE32_64) > + // In 32 bits, asCell() returns a pointer to the payload and using reinterpret_cast > + // will sign extend asNumber to FFFF FFFF + <value-address>, which will fail the > + // isImpureNaN check in jsNumber. To prevent that, first reinterpret_cast the address > + // to uint32_t then cast it to uint64_t. AddressOf in 32 bits will return the same > + // address returned by describe() but in double > + uint64_t asNumber = static_cast<uint64_t>(reinterpret_cast<uint32_t>(value.asCell())); > +#else > // Need to cast to uint64_t so bitwise_cast will play along. > uint64_t asNumber = reinterpret_cast<uint64_t>(value.asCell()); > +#endif Better fix is to just replace the incorrect <uint64_t> with the correct <uintptr_t> instead of adding code and comments. No need to use static_cast to expand a 32-bit unsigned int to 64-bit.
Indeed, <uintptr_t> sounds like a better solution here, but we would still need the static_cast to 64 bits because of the bitcast<double>. In 32 bits <uintptr_t> is 32 bits long and the bitcast will fail.
(In reply to Mikhail R. Gadelha from comment #4) > Indeed, <uintptr_t> sounds like a better solution here, but we would still > need the static_cast to 64 bits because of the bitcast<double>. No static_cast needed. Write this: uint64_t asNumber = reinterpret_cast<uintptr_t>(value.asCell()); There’s no need for a static_cast on this line of code. The unsigned integer conversion, with possible promotion, can be done without any casting.
I got what you mean now! I'll send an updated patch shortly
Created attachment 430302 [details] Patch
Comment on attachment 430302 [details] Patch I suspect that other callers of reinterpret_cast<uint64_t> throughout WebKit could have the same issue on 32-bit. There are 20 of them at least, and many are likely converting from pointers to integers; the same fix would likely apply.
<rdar://problem/79015579>
Committed r278660 (238642@main): <https://commits.webkit.org/238642@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 430302 [details].