Bug 213448 - On 64bit build, JSValue::operator bool maybe wrong.
Summary: On 64bit build, JSValue::operator bool maybe wrong.
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-06-21 19:03 PDT by xc.o.c.1180@gmail.com
Modified: 2020-06-23 13:51 PDT (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description xc.o.c.1180@gmail.com 2020-06-21 19:03:21 PDT
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.
Comment 1 Alexey Proskuryakov 2020-06-23 12:15:33 PDT
We don't have this code in WebKit trunk. Are you perhaps looking at a fork, or an old branch?
Comment 2 xc.o.c.1180@gmail.com 2020-06-23 13:13:12 PDT
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.
Comment 3 Alexey Proskuryakov 2020-06-23 13:40:07 PDT
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.
Comment 4 Yusuke Suzuki 2020-06-23 13:51:25 PDT
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.