Repository navigation
Bug on array spread operator on NodeJS 6.10.0 with --optimize_for_size #11545
Description
- Version: v6.10.0 and v6.9.5
- Platform: Linux sager 4.4.0-64-generic Update README.md #85-Ubuntu SMP Mon Feb 20 11:50:30 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
The following code causes an error:
'use strict';
const assert = require('assert');
const entities = Array.apply(null, { length: 1000 }).map(() => (
{}
));
const bulkBody = entities.reduce((res, entity) => {
const action = {
foo: {
bar: 10,
baz: 20,
foobar: undefined,
},
};
// no-op
if (false) {
let a;
let b;
}
return [
...res,
action,
entity,
];
}, []);
assert(bulkBody.length == 2 * entities.length, `bulkBody.length: ${bulkBody.length}\tentities.length: ${entities.length}`);Note that on each loop of the reduce we add 2 elements to the result array. As it starts as an empty array (i.e. with 0 elements), at the end we expect that bulkBody.length == 2 * entities.length. This isn't what happens. See:
$ node --version
v6.10.0
$ node error/reproduce.js # no errors
$ node --optimize_for_size error/reproduce.js
assert.js:85
throw new assert.AssertionError({
^
AssertionError: bulkBody.length: 1999 entities.length: 1000
at Object.<anonymous> (/home/vitor/Projetos/okfn/opentrials/api/error/reproduce.js:29:1)
at Module._compile (module.js:570:32)
at Object.Module._extensions..js (module.js:579:10)
at Module.load (module.js:487:32)
at tryModuleLoad (module.js:446:12)
at Function.Module._load (module.js:438:3)
at Module.runMain (module.js:604:10)
at run (bootstrap_node.js:394:7)
at startup (bootstrap_node.js:149:9)
at bootstrap_node.js:509:3
$ nvm use 5.6.0
Now using node v5.6.0 (npm v3.6.0)
$ node --version
v5.6.0
$ node error/reproduce.js
$ node --optimize_for_size error/reproduce.jsI tested both on 6.10.0 and 6.9.5 and the error is the same. Note that the code has many no-op operations. If you remove the if (false) {} clause, for example, the error isn't triggered. If you change the entities array length from 1000 to 10000 (for example), the error isn't triggered either. Even if you change the foobar: undefined to foobar: 30, the error isn't triggered.
It seems like that Node is optimizing a very specific code and, if we change even no-op code, the bug isn't triggered.
Activity
fernandobrito commentedon Feb 24, 2017
I could reproduce it on:
v7.6.0 seems to be unaffacted.
mscdex commentedon Feb 24, 2017
/cc @nodejs/v8
[#693] Fix reindexing issues
[#693] Fix reindexing issues
bnoordhuis commentedon Feb 27, 2017
Interesting bug. I can reproduce and a quick investigation suggests it might have something to do with allocation site pretenuring in Crankshaft.
--trace_deoptlogs a rather suspiciousreason: allocation-site-tenuring-changedmessage and adding either--noallocation_site_pretenuringor--nocrankshaftmakes the test pass reliably again.It sometimes passes without the flags too so there is a timing aspect to it as well but I'm reasonably sure allocation site pretenuring is involved somehow.
targos commentedon Feb 27, 2017
I can't reproduce with d8 (5.1.281.75)
targos commentedon Feb 28, 2017
@nodejs/v8 FYI I built several
5.x-lkgrV8 branches on 64bit Linux to be able to check easily which release fixes a particular issue. Binaries are here: https://github.com/targos/d8/tree/master/binTrott commentedon Jul 27, 2017
Is this still a bug in Node.js 6.11.1? (Bug isn't tripped on my operating system, so I'm guessing it's Linux-specific.)
bnoordhuis commentedon Jul 28, 2017
I can still reproduce, on MacOS too. Try running it in a loop:
apapirovski commentedon Apr 13, 2018
Works as expected 6.14.0 so I guess this did get fixed at some point.