In a number of places, addresses are reinterpreted as uint64, which can lead to wrong addresses in 32 bits arch. For example, in https://bugs.webkit.org/show_bug.cgi?id=226503, asCell() returns a pointer to the payload and using reinterpret_cast will sign extend asNumber to FFFF FFFF + , which will fail the isImpureNaN check in jsNumber.
Created attachment 431777 [details] Patch
I've also added a bitwise_cast there, which is not memory-related but it makes the code slightly more readable.
Created attachment 431781 [details] Patch
Comment on attachment 431781 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=431781&action=review r=me with nits > Source/JavaScriptCore/disassembler/ARM64/A64DOpcode.h:190 > + bufferPrintf("0x%" PRIxPTR, reinterpret_cast<uintptr_t>(pc + immediate)); Using bitwise_cast would be better since it checks whether the start() is the same size to uintptr_t. > Source/JavaScriptCore/runtime/JSCell.cpp:295 > + ptrdiff_t cellOffset = cellAddress - reinterpret_cast<uintptr_t>(foundBlockHandle->start()); Using bitwise_cast would be better since it checks whether the start() is the same size to uintptr_t. > Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp:2799 > + append(Move, Arg::bigImm(reinterpret_cast<uintptr_t>(&m_tierUp->m_counter)), countdownPtr); Ditto. > Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp:2851 > + append(Move, Arg::bigImm(reinterpret_cast<uintptr_t>(&m_tierUp->m_counter)), countdownPtr); Ditto. > Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:2033 > + Value* countDownLocation = constant(pointerType(), reinterpret_cast<uintptr_t>(&m_tierUp->m_counter), Origin()); Ditto. > Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:2083 > + Value* countDownLocation = constant(pointerType(), reinterpret_cast<uintptr_t>(&m_tierUp->m_counter), origin); Ditto. > Source/WTF/wtf/LoggerHelper.h:79 > + return reinterpret_cast<const void*>((reinterpret_cast<uintptr_t>(parentIdentifier) & parentMask) | (childIdentifier & maskLowerWord)); Ditto.
Comment on attachment 431781 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=431781&action=review >> Source/WTF/wtf/LoggerHelper.h:79 >> + return reinterpret_cast<const void*>((reinterpret_cast<uintptr_t>(parentIdentifier) & parentMask) | (childIdentifier & maskLowerWord)); > > Ditto. Is this correct? We are using `static constexpr uint64_t`
Created attachment 431863 [details] Patch
Comment on attachment 431781 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=431781&action=review >>> Source/WTF/wtf/LoggerHelper.h:79 >>> + return reinterpret_cast<const void*>((reinterpret_cast<uintptr_t>(parentIdentifier) & parentMask) | (childIdentifier & maskLowerWord)); >> >> Ditto. > > Is this correct? We are using `static constexpr uint64_t` The compiler will introduce an implicit cast from uintptr_t to uint64_t in 32 bits mode... Here's the clang ast dump (I actually just copied the function to a separate file but it should generate the same code during compilation): | `-BinaryOperator 0x5655a48 <col:47, col:91> 'unsigned long long' '&' HERE->| |-ImplicitCastExpr 0x5655a30 <col:47, col:87> 'unsigned long long' <IntegralCast> | | `-CallExpr 0x5655998 <col:47, col:87> 'unsigned int':'unsigned int' | | |-ImplicitCastExpr 0x5655980 <col:47, col:69> 'unsigned int (*)(const void *)' <FunctionToPointerDecay> | | | `-DeclRefExpr 0x56558b8 <col:47, col:69> 'unsigned int (const void *)' lvalue Function 0x56557a0 'bitwise_cast' 'unsigned int (const void *)' (FunctionTemplate 0x5653770 'bitwise_cast') | | `-ImplicitCastExpr 0x56559c0 <col:71> 'const void *' <LValueToRValue> | | `-DeclRefExpr 0x56554e0 <col:71> 'const void *' lvalue ParmVar 0x5654f98 'parentIdentifier' 'const void *' | `-ImplicitCastExpr 0x5655a18 <col:91> 'uint64_t':'unsigned long long' <LValueToRValue> | `-DeclRefExpr 0x56559f8 <col:91> 'const uint64_t':'const unsigned long long' lvalue Var 0x5655218 'parentMask' 'const uint64_t':'const unsigned long long' non_odr_use_constant No implicit cast is added in 64 bits mode.
Created attachment 432265 [details] Patch
<rdar://problem/79779047>
Committed r279341 (239209@main): <https://commits.webkit.org/239209@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 432265 [details].