Bug 54608 – Refactor WriteBarrier and DeprecatedPtr to have less code duplication.

• Status: NEW
• Component: New Bugs

[Oliver Hunt]

Refactor WriteBarrier and DeprecatedPtr to have less code duplication.

[Oliver Hunt]

Created attachment 82833 [details]
Patch

[Geoffrey Garen]

Comment on attachment 82833 [details]
Patch

r=me

[Build Bot]

Attachment 82833 [details] did not build on win:
Build output: http://queues.webkit.org/results/7929545

[WebKit Review Bot]

http://trac.webkit.org/changeset/78856 might have broken Windows Release (Build)

[Zoltan Herczeg]

I saw this patch was rolled out, and I decided to help you a little. v8-crypto goes into an infinite loop because of the following expression:

var a = new Array()
a[0] = 1
print(a.length)

It is "0" with this patch, and "1" after the patch was rolled out. I think the append does not happen, but I need some further investigation.

[Zoltan Herczeg]

emit_op_put_by_val:
 addSlowCase(branch32(AboveOrEqual, regT2, Address(regT0, OBJECT_OFFSETOF(JSArray, m_vectorLength))));

0xb791027f:     cmp    0x30(%eax),%ecx
(gdb) x $eax+0x30
0xb78c2ff0:     0x00000003
(gdb) x $ecx
0x0:    Cannot access memory at address 0x0

eax points to the array, and its length is 3, instead of 0. Thus the append is "successful".

[Zoltan Herczeg]

Forget about my previous comment, the initialCapacity is 3, so that is ok.

However, the vector clear loop in JSArray constructor:

JSArray::JSArray(JSGlobalData& globalData, NonNullPassRefPtr<Structure> structure, const ArgList& list)

    for (; i < initialStorage; i++)
        vector[i].clear();

was set the values to EmptyValueTag, but when this patch is applied, it set to 0 by WriteBarrier.h:78 void clear() { m_value = 0; }

Zero is not even a valid JSValue32_64 tag, so this is totally wrong.

[Zoltan Herczeg]

was:
WriteBarrier.h:122 void clear() { m_value = JSValue::encode(JSValue()); }
  which is: JSValue.h:403 u.asBits.tag = EmptyValueTag;

now:
WriteBarrier.h:78 void clear() { m_value = 0; }

I'll try:
WriteBarrier.h:78 void clear() { m_value = JSValue::encode(JSValue()); }

[Adam Roben (:aroben)]

Comment on attachment 82833 [details]
Patch

Note that this patch was landed in r78856 and rolled out in r78945 (see bug 54705).

[Zoltan Herczeg]

I know :) But the patch is not necessary wrong, maybe just there is small bug in it.

Unfortunately this is not working when T is JSCell:

WriteBarrier.h:78 void clear() { m_value = JSValue::encode(JSValue()); }