| Summary: | On 64bit build, JSValue::operator bool maybe wrong. | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | xc.o.c.1180 <xc.o.c.1180> |
| Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED INVALID | ||
| Severity: | Normal | CC: | ap, fpizlo, mark.lam, saam, ysuzuki |
| Priority: | P2 | ||
| Version: | WebKit Nightly Build | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
We don't have this code in WebKit trunk. Are you perhaps looking at a fork, or an old branch? Sorry for the confusion, the code snippet in previous description was modified version.
The original one as below,
// 0x0 can never occur naturally because it has a tag of 00, indicating a pointer value, but a payload of 0x0, which is in the (invalid) zero page.
inline JSValue::JSValue()
{
u.asInt64 = ValueEmpty;
}
// 0x4 can never occur naturally because it has a tag of 00, indicating a pointer value, but a payload of 0x4, which is in the (invalid) zero page.
inline JSValue::JSValue(HashTableDeletedValueTag)
{
u.asInt64 = ValueDeleted;
}
inline JSValue::JSValue(JSCell* ptr)
{
u.asInt64 = reinterpret_cast<uintptr_t>(ptr);
}
inline JSValue::JSValue(const JSCell* ptr)
{
u.asInt64 = reinterpret_cast<uintptr_t>(const_cast<JSCell*>(ptr));
}
inline JSValue::operator bool() const
{
return u.asInt64;
}
Please check again.
Thanks.
Thank you for following up. Seems unlikely that this is actively harmful or even wrong, because this code is constantly tested on multiple 32-bit and 64-bit platforms, but CC'ing experts for comment. Thanks for your report! JSC has different representations for JSValue in 32bit and 64bit. And, in JSC in 64bit, nullptr is the empty value representation. So, this code is correct. |
On 32bit build, JSValue::operator bool checks not empty value. On 64bit build, JSValue::operator bool checks not null pointer. Also on 32bit build, when passing null pointer JSCell, an empty value is created. Should 64bit build do the same thing? ALWAYS_INLINE JSValue::JSValue(JSCell* ptr) { if (LIKELY(ptr != 0)) u.asInt64 = reinterpret_cast<uintptr_t>(ptr); else u.asInt64 = ValueEmpty; } ALWAYS_INLINE JSValue::JSValue(const JSCell* ptr) { if (LIKELY(ptr != 0)) u.asInt64 = reinterpret_cast<uintptr_t>(const_cast<JSCell*>(ptr)); else u.asInt64 = ValueEmpty; } inline JSValue::operator bool() const { return u.asInt64 != ValueEmpty; } Please take a look. Thanks.