Summary: | Prevent sign-extended casts for 32 bits arch | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mikhail R. Gadelha <mikhail> | ||||||||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | angelos, benjamin, cdumez, cmarcelo, ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer, ysuzuki | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Mikhail R. Gadelha
2021-06-18 08:17:32 PDT
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
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]. |