Swapping two arrays seems to behave asynchronous outside function

[ghost]

Version: v7.1.0
Platform: Windows 8.1; 64-bit

Issue details: Swapping two variables from a function sometimes behaves like it is asynchronous as seen from outside that function, but synchronous from inside. It only happens when variables are swapped using the following short syntax:

[a, b] = [b, a];

However, it doesn't hapen every time. This is an example

'use strict';

var a = [];
var b = [];
var c = () => a;
var d = () => [a, b] = [b, a];

d();

console.log(a === c()); // Sometimes it prints `false`, which should be impossible

However, it doesn't work always. But, after a few hours of testing, I came up with the following code, which always prints wrong output:

'use strict';

// Here if I change `const` to `var` it prints 111
const w = 100;
const h = 100;

var first = new Array(100).fill(0).map(() => new Uint8Array(100));
var second = new Array(100).fill(0).map(() => new Uint8Array(100));
var getFirst = () => first;

var x, y;
var beginning = true;
var i = 0;

var func = () => {
	if(beginning){
		beginning = false;

		// If I remove bitwise `or` operator from here, it prints 111
		for(y = 0; (y | 0) < (h | 0); y++) for(x = 0; (x | 0) < (w | 0); x++){}
	}else{
		for(y = 0; y < h; y++) for(x = 0; x < w; x++){

			// If I change the number 1 to 0 from this line, it prints 111
			if((first[x | 0][y | 0] | 0) + 1);
		}

		[first, second] = [second, first];
	}

	console.log(first === getFirst() ? 1 : 0); // It should always print 1, but third time it prints 0
};

func(); // Prints 1, that is ok
func(); // Prints 1 too, its  ok
func(); // This one always prints 0

Here is how output looks like. I cannot find any possible explanations why is zero here:

[mscdex]

This kind of question is better suited for the nodejs/help repo. FWIW though, your first example outputs 'true' every time for me. Your second example has a potential issue because of the way you're styling your loops.

Specifically, this block:

for(y = 0; y < h; y++) for(x = 0; x < w; x++){

  // If I change the number 1 to 0 from this line, it prints 111
  if((first[x | 0][y | 0] | 0) + 1);
}

[first, second] = [second, first];

is equivalent to:

for(y = 0; y < h; y++) {
  for(x = 0; x < w; x++) {
    if((first[x | 0][y | 0] | 0) + 1);
  }
}
[first, second] = [second, first];

instead of what you may be expecting:

for(y = 0; y < h; y++) {
  for(x = 0; x < w; x++) {
    if((first[x | 0][y | 0] | 0) + 1);
  }
  [first, second] = [second, first];
}

The reason is that control blocks (like for, if, etc.) only include the next statement if there are no braces surrounding the block.

Using the latter code block outputs:

1
1
1

every time. However, this is a total guess because I have no idea what your actual intentions are. I would suggest using braces and not placing multiple statements like that on a single line to avoid potential confusion like this in the future.

[ghost]

I think you misunderstood my question. You don't have to explain me how syntax works, I am programming in JavaScript for five years and I also created my own JavaScript engine in pure assembler. My loop statements are placed exactly as it is supposed to be.

The problem is strongly related to v8 engine here. Look at line where console log is called. We compare variable called first with output of function which returns first. No matter where and how we call it, it should return true always, but third time it is called it returns false. You can see it on image I posted.

[joyeecheung]

I can confirm this is a V8 bug and it still presents in the master. However it is fixed in https://github.com/v8/node/tree/vee-eight-lkgr. Not sure what commit causes this without bisecting though.

[joyeecheung]

P.S. I can narrow down the working V8 to 5.5.372.33 with the stable Chrome..

EDIT: That seems odd, the master has V8 5.5.372.40, it's probably a regression or something, or it has something to do with console.log of Node.js being async (just guessing)?

FWIW looks like it has something to do with TurboFan OSR. I've replaced the tenary expression with just the equality check so it prints true/false instead of 1/0 here.

 ../node/node --trace-opt --trace-opt-verbose 11047.js
[marking 0x15b90280b079 <JS Function Uint8ArrayConstructByLength (SharedFunctionInfo 0x9c5d7c18c49)> for optimized recompilation, reason: small function, ICs with typeinfo: 5/4 (125%), generic ICs: 0/4 (0%)]
[compiling method 0x15b90280b079 <JS Function Uint8ArrayConstructByLength (SharedFunctionInfo 0x9c5d7c18c49)> using Crankshaft]
[optimizing 0x15b90280b079 <JS Function Uint8ArrayConstructByLength (SharedFunctionInfo 0x9c5d7c18c49)> - took 0.299, 0.610, 0.256 ms]
[completed optimizing 0x15b90280b079 <JS Function Uint8ArrayConstructByLength (SharedFunctionInfo 0x9c5d7c18c49)>]
true
[marking 0x2d3813104d61 <JS Function func (SharedFunctionInfo 0x364a935e0d49)> for optimized recompilation, reason: hot and stable, ICs with typeinfo: 23/45 (51%), generic ICs: 1/45 (2%)]
true
[compiling method 0x2d3813104d61 <JS Function func (SharedFunctionInfo 0x364a935e0d49)> using TurboFan]
[compiling method 0x2d3813104d61 <JS Function func (SharedFunctionInfo 0x364a935e0d49)> using TurboFan OSR]
[optimizing 0x2d3813104d61 <JS Function func (SharedFunctionInfo 0x364a935e0d49)> - took 3.026, 4.122, 0.493 ms]
[optimizing 0x2d3813104d61 <JS Function func (SharedFunctionInfo 0x364a935e0d49)> - took 3.700, 5.486, 0.439 ms]
[completed optimizing 0x2d3813104d61 <JS Function func (SharedFunctionInfo 0x364a935e0d49)>]
false

 ../v8-node/node --trace-opt --trace-opt-verbose 11047.js
[marking 0x2f78f9123ea9 <JS Function Uint8ArrayConstructByLength (SharedFunctionInfo 0x155859394e81)> for optimized recompilation, reason: small function, ICs with typeinfo: 5/7 (71%), generic ICs: 0/7 (0%)]
[compiling method 0x2f78f9123ea9 <JS Function Uint8ArrayConstructByLength (SharedFunctionInfo 0x155859394e81)> using Crankshaft]
[optimizing 0x2f78f9123ea9 <JS Function Uint8ArrayConstructByLength (SharedFunctionInfo 0x155859394e81)> - took 0.466, 1.584, 1.494 ms]
[completed optimizing 0x2f78f9123ea9 <JS Function Uint8ArrayConstructByLength (SharedFunctionInfo 0x155859394e81)>]
true
[marking 0x35259e304cb9 <JS Function func (SharedFunctionInfo 0x23db7f3d92c9)> for optimized recompilation, reason: hot and stable, ICs with typeinfo: 20/39 (51%), generic ICs: 2/39 (5%)]
true
[compiling method 0x35259e304cb9 <JS Function func (SharedFunctionInfo 0x23db7f3d92c9)> using TurboFan]
true

[ghost]

I think I found what possibly may cause the problem. Look at this code. TurboFan caches the result of comparison, but in my code variables first and second are swapped. According to optimization techniques used in TurboFan, first are cached, but its value is actually second, when compared to real first variable it results false.

[joyeecheung]

This should probably be fixed by #10992. cc @nodejs/v8

[fhinkel]

cc @nodejs/v8 (@joyeecheung 's mention didn't work).

[ofrobots]

/cc @bmeurer

[ChALkeR]

Upd: this comment had a mistype, amended.

Can't reproduce on Linux and Node.js 7.5.0.
Removed «question» label, though, as it seems that this is an actual issue that has been confirmed above.

[joyeecheung]

Forgot to mention I'm on Darwin 15.6.0.

Just tried out #10992, the bug is fixed in that brach :D. Master (58dc229) still has it though.

[bmeurer]

It's a bug in V8, here's a minimal repro that fails in d8 (ran with --allow-natives-syntax in 5.4.500.43):

(function() {
  'use strict';
  let first = 1;
  let second = 2;
  let getFirst = () => first;

  let func = () => {
    [first, second] = [second, first];
    return first === getFirst();
  };

  print(func());
  print(func());
  %OptimizeFunctionOnNextCall(func);
  print(func());
})();

Doesn't fail on ToT tho, so it's already fixed. It's clearly a bug in TurboFan generated code.

[bmeurer]

Even simpler repro:

(function() {
  'use strict';
  let first = 1;
  let getFirst = () => first;

  let func = (x) => {
    [first] = [x];
    return first === getFirst();
  };

  print(func(2));
  print(func(3));
  %OptimizeFunctionOnNextCall(func);
  print(func(4));
})();

The bug is that the Parser tells TurboFan that first is never re-assigned, and thus TurboFan constant-folds first when inlining getFirst into func. The fix is in crrev.com/2562443003.

[joyeecheung]

@bmeurer Thanks for the explanation!

I think this can be left open until #10992 is merged?

[ofrobots]

It would be quite straightforward to backport the fix until #10992 is merged, if someone is motivated to put it together (here's a guide to backporting.) If #10992 is merged, this will no longer be necessary.

[ghost]

@ofrobots Maybe it would be worth to add "confirmed-bug" label on this issue.

[bnoordhuis]

#10992 was merged last week so I'll go ahead and close this out.