Bug 170934 – Computed Properties with increment sometimes produces incorrect results

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Joseph Pecoraro]

Summary:
Computed Properties with increment sometimes produces incorrect results

Test:

    let c = 0;
    let o1 = { [++c]: ++c };
    // Should be 1 => 2 and is.

    {
        let x = 0;
        let o2 = { [++x]: ++x };
        // Should be 1 => 2, but is not. Instead it is 2 => 2?!
    }

Notes:

> <global>#EZzSqs:[0x10f3740a0->0x10f37c520, NoneGlobal, 122]: 122 m_instructions; 976 bytes; 1 parameter(s); 12 callee register(s); 5 variable(s); scope at loc3
> [   0] enter             
> [   1] get_scope         loc3
> [   3] mov               loc4, loc3
> [   6] check_traps       
> [   7] mov               loc5, Undefined(const0)
> [  10] resolve_scope     loc6, loc3, c(@id0), <GlobalLexicalVar>, 0, 0x10f3c40a0
> [  17] put_to_scope      loc6, c(@id0), Int32: 0(const1), 1048578<DoNotThrowIfNotFound|GlobalLexicalVar|Initialization>, <structure>, 242782464
> [  24] resolve_scope     loc6, loc3, o1(@id1), <GlobalLexicalVar>, 0, 0x10f3c40a0
> [  31] new_object        loc7, 0
> [  35] resolve_scope     loc8, loc3, c(@id0), <GlobalLexicalVar>, 0, 0x10f3c40a0
> [  42] get_from_scope    loc9, loc8, c(@id0), 2050<ThrowIfNotFound|GlobalLexicalVar|NotInitialization>, 242782464    predicting None
> [  50] inc               loc9
> [  52] put_to_scope      loc8, c(@id0), loc9, 2050<ThrowIfNotFound|GlobalLexicalVar|NotInitialization>, <structure>, 242782464
> [  59] resolve_scope     loc10, loc3, c(@id0), <GlobalLexicalVar>, 0, 0x10f3c40a0
> [  66] get_from_scope    loc11, loc10, c(@id0), 2050<ThrowIfNotFound|GlobalLexicalVar|NotInitialization>, 242782464    predicting None
> [  74] inc               loc11
> [  76] put_to_scope      loc10, c(@id0), loc11, 2050<ThrowIfNotFound|GlobalLexicalVar|NotInitialization>, <structure>, 242782464
> [  83] put_by_val_direct loc7, loc9, loc11    Original
> [  88] put_to_scope      loc6, o1(@id1), loc7, 1048578<DoNotThrowIfNotFound|GlobalLexicalVar|Initialization>, <structure>, 242782472
> [  95] mov               loc6, <JSValue()>(const2)
> [  98] mov               loc7, <JSValue()>(const2)
> [ 101] mov               loc6, Int32: 0(const1)
> [ 104] new_object        loc8, 0
> [ 108] inc               loc6
> [ 110] inc               loc6
> [ 112] put_by_val_direct loc8, loc6, loc6    Original
> [ 117] mov               loc7, loc8
> [ 120] end               loc5

See Object 1's construction [31-88] compared to Object 2's [104-112].

[Saam Barati]

Created attachment 307408 [details]
patch-ish

We weren't assigning the ++x to a new register, that was the bug.

[Joseph Pecoraro]

Excellent, I'll include this when I also fix the order (computed property should happen first before the value).

[Ross Kirsling]

Created attachment 391209 [details]
Patch

[Yusuke Suzuki]

Comment on attachment 391209 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=391209&action=review

r=me with one comment.

> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:742
>      if (const auto* identifier = node.name()) {

Let's insert `ASSERT(!propertyName);`.

[Ross Kirsling]

Created attachment 391240 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 391240 [details]
Patch for landing

Clearing flags on attachment: 391240

Committed r257034: <https://trac.webkit.org/changeset/257034>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/59615634>

[Caitlin Potter (:caitp)]

It's nice to see some other uses of the new opcode unrelated to class fields :)
👍

[Ross Kirsling]

(In reply to Caitlin Potter (:caitp) from comment #9)
> It's nice to see some other uses of the new opcode unrelated to class fields
> :)
> 👍

Yeah! :D I realized in the previous bug (bug 207297) that it was exactly the thing we needed.