nodejs/node

`node-crc` failing on node 6.x only

alexgorbatchev opened this issue · 12 comments

Friends,

I'm the author of the crc module on which over 200 packages depend including express.js.

I test the module on 0.10, 0.12, 4, 5, 6 and 7. One of the tests oddly fails on 6 only. Please see the travis build. Basically, one test out of 255 fails with a consistently unexpected checksum.

I don't have experience debugging node specific issues, I'm wondering if anyone could have any pointers for me what it is that might be going on here?

The file in question is here. I'll also paste it here for the record.

import {Buffer} from 'buffer';
import defineCrc from './define_crc';

// Generated by `./pycrc.py --algorithm=table-driven --model=crc-16 --generate=c`
let TABLE = [
  0x0000, 0xc0c1, 0xc181, 0x0140, 0xc301, 0x03c0, 0x0280, 0xc241,
  0xc601, 0x06c0, 0x0780, 0xc741, 0x0500, 0xc5c1, 0xc481, 0x0440,
  0xcc01, 0x0cc0, 0x0d80, 0xcd41, 0x0f00, 0xcfc1, 0xce81, 0x0e40,
  0x0a00, 0xcac1, 0xcb81, 0x0b40, 0xc901, 0x09c0, 0x0880, 0xc841,
  0xd801, 0x18c0, 0x1980, 0xd941, 0x1b00, 0xdbc1, 0xda81, 0x1a40,
  0x1e00, 0xdec1, 0xdf81, 0x1f40, 0xdd01, 0x1dc0, 0x1c80, 0xdc41,
  0x1400, 0xd4c1, 0xd581, 0x1540, 0xd701, 0x17c0, 0x1680, 0xd641,
  0xd201, 0x12c0, 0x1380, 0xd341, 0x1100, 0xd1c1, 0xd081, 0x1040,
  0xf001, 0x30c0, 0x3180, 0xf141, 0x3300, 0xf3c1, 0xf281, 0x3240,
  0x3600, 0xf6c1, 0xf781, 0x3740, 0xf501, 0x35c0, 0x3480, 0xf441,
  0x3c00, 0xfcc1, 0xfd81, 0x3d40, 0xff01, 0x3fc0, 0x3e80, 0xfe41,
  0xfa01, 0x3ac0, 0x3b80, 0xfb41, 0x3900, 0xf9c1, 0xf881, 0x3840,
  0x2800, 0xe8c1, 0xe981, 0x2940, 0xeb01, 0x2bc0, 0x2a80, 0xea41,
  0xee01, 0x2ec0, 0x2f80, 0xef41, 0x2d00, 0xedc1, 0xec81, 0x2c40,
  0xe401, 0x24c0, 0x2580, 0xe541, 0x2700, 0xe7c1, 0xe681, 0x2640,
  0x2200, 0xe2c1, 0xe381, 0x2340, 0xe101, 0x21c0, 0x2080, 0xe041,
  0xa001, 0x60c0, 0x6180, 0xa141, 0x6300, 0xa3c1, 0xa281, 0x6240,
  0x6600, 0xa6c1, 0xa781, 0x6740, 0xa501, 0x65c0, 0x6480, 0xa441,
  0x6c00, 0xacc1, 0xad81, 0x6d40, 0xaf01, 0x6fc0, 0x6e80, 0xae41,
  0xaa01, 0x6ac0, 0x6b80, 0xab41, 0x6900, 0xa9c1, 0xa881, 0x6840,
  0x7800, 0xb8c1, 0xb981, 0x7940, 0xbb01, 0x7bc0, 0x7a80, 0xba41,
  0xbe01, 0x7ec0, 0x7f80, 0xbf41, 0x7d00, 0xbdc1, 0xbc81, 0x7c40,
  0xb401, 0x74c0, 0x7580, 0xb541, 0x7700, 0xb7c1, 0xb681, 0x7640,
  0x7200, 0xb2c1, 0xb381, 0x7340, 0xb101, 0x71c0, 0x7080, 0xb041,
  0x5000, 0x90c1, 0x9181, 0x5140, 0x9301, 0x53c0, 0x5280, 0x9241,
  0x9601, 0x56c0, 0x5780, 0x9741, 0x5500, 0x95c1, 0x9481, 0x5440,
  0x9c01, 0x5cc0, 0x5d80, 0x9d41, 0x5f00, 0x9fc1, 0x9e81, 0x5e40,
  0x5a00, 0x9ac1, 0x9b81, 0x5b40, 0x9901, 0x59c0, 0x5880, 0x9841,
  0x8801, 0x48c0, 0x4980, 0x8941, 0x4b00, 0x8bc1, 0x8a81, 0x4a40,
  0x4e00, 0x8ec1, 0x8f81, 0x4f40, 0x8d01, 0x4dc0, 0x4c80, 0x8c41,
  0x4400, 0x84c1, 0x8581, 0x4540, 0x8701, 0x47c0, 0x4680, 0x8641,
  0x8201, 0x42c0, 0x4380, 0x8341, 0x4100, 0x81c1, 0x8081, 0x4040
]

if (typeof(Int32Array) !== 'undefined') TABLE = new Int32Array(TABLE);

module.exports = defineCrc('crc-16', function (buf, previous) {
  if (!Buffer.isBuffer(buf)) buf = new Buffer(buf);

  let crc = ~~previous;

  for (let index = 0; index < buf.length; index++) {
    const byte = buf[index];
    crc = ((TABLE[(crc ^ byte) & 0xff] ^ (crc >> 8)) & 0xffff);
  }

  return crc;
});

Given that you're using import, you must be using some kind of source transform. Do you still see the failure when you eliminate that? What happens when you run node --nocrankshaft test.js?

@bnoordhuis thank you for replying! I have taken your indirect suggestion and simplified everything so that there's no transpiling and no dependencies. The result is unfortunately the same.

Here's the simplified file

var Buffer = require('buffer').Buffer;

// Generated by `./pycrc.py --algorithm=table-driven --model=crc-16 --generate=c`
var TABLE = [
  0x0000, 0xc0c1, 0xc181, 0x0140, 0xc301, 0x03c0, 0x0280, 0xc241,
  0xc601, 0x06c0, 0x0780, 0xc741, 0x0500, 0xc5c1, 0xc481, 0x0440,
  0xcc01, 0x0cc0, 0x0d80, 0xcd41, 0x0f00, 0xcfc1, 0xce81, 0x0e40,
  0x0a00, 0xcac1, 0xcb81, 0x0b40, 0xc901, 0x09c0, 0x0880, 0xc841,
  0xd801, 0x18c0, 0x1980, 0xd941, 0x1b00, 0xdbc1, 0xda81, 0x1a40,
  0x1e00, 0xdec1, 0xdf81, 0x1f40, 0xdd01, 0x1dc0, 0x1c80, 0xdc41,
  0x1400, 0xd4c1, 0xd581, 0x1540, 0xd701, 0x17c0, 0x1680, 0xd641,
  0xd201, 0x12c0, 0x1380, 0xd341, 0x1100, 0xd1c1, 0xd081, 0x1040,
  0xf001, 0x30c0, 0x3180, 0xf141, 0x3300, 0xf3c1, 0xf281, 0x3240,
  0x3600, 0xf6c1, 0xf781, 0x3740, 0xf501, 0x35c0, 0x3480, 0xf441,
  0x3c00, 0xfcc1, 0xfd81, 0x3d40, 0xff01, 0x3fc0, 0x3e80, 0xfe41,
  0xfa01, 0x3ac0, 0x3b80, 0xfb41, 0x3900, 0xf9c1, 0xf881, 0x3840,
  0x2800, 0xe8c1, 0xe981, 0x2940, 0xeb01, 0x2bc0, 0x2a80, 0xea41,
  0xee01, 0x2ec0, 0x2f80, 0xef41, 0x2d00, 0xedc1, 0xec81, 0x2c40,
  0xe401, 0x24c0, 0x2580, 0xe541, 0x2700, 0xe7c1, 0xe681, 0x2640,
  0x2200, 0xe2c1, 0xe381, 0x2340, 0xe101, 0x21c0, 0x2080, 0xe041,
  0xa001, 0x60c0, 0x6180, 0xa141, 0x6300, 0xa3c1, 0xa281, 0x6240,
  0x6600, 0xa6c1, 0xa781, 0x6740, 0xa501, 0x65c0, 0x6480, 0xa441,
  0x6c00, 0xacc1, 0xad81, 0x6d40, 0xaf01, 0x6fc0, 0x6e80, 0xae41,
  0xaa01, 0x6ac0, 0x6b80, 0xab41, 0x6900, 0xa9c1, 0xa881, 0x6840,
  0x7800, 0xb8c1, 0xb981, 0x7940, 0xbb01, 0x7bc0, 0x7a80, 0xba41,
  0xbe01, 0x7ec0, 0x7f80, 0xbf41, 0x7d00, 0xbdc1, 0xbc81, 0x7c40,
  0xb401, 0x74c0, 0x7580, 0xb541, 0x7700, 0xb7c1, 0xb681, 0x7640,
  0x7200, 0xb2c1, 0xb381, 0x7340, 0xb101, 0x71c0, 0x7080, 0xb041,
  0x5000, 0x90c1, 0x9181, 0x5140, 0x9301, 0x53c0, 0x5280, 0x9241,
  0x9601, 0x56c0, 0x5780, 0x9741, 0x5500, 0x95c1, 0x9481, 0x5440,
  0x9c01, 0x5cc0, 0x5d80, 0x9d41, 0x5f00, 0x9fc1, 0x9e81, 0x5e40,
  0x5a00, 0x9ac1, 0x9b81, 0x5b40, 0x9901, 0x59c0, 0x5880, 0x9841,
  0x8801, 0x48c0, 0x4980, 0x8941, 0x4b00, 0x8bc1, 0x8a81, 0x4a40,
  0x4e00, 0x8ec1, 0x8f81, 0x4f40, 0x8d01, 0x4dc0, 0x4c80, 0x8c41,
  0x4400, 0x84c1, 0x8581, 0x4540, 0x8701, 0x47c0, 0x4680, 0x8641,
  0x8201, 0x42c0, 0x4380, 0x8341, 0x4100, 0x81c1, 0x8081, 0x4040
]

if (typeof(Int32Array) !== 'undefined') TABLE = new Int32Array(TABLE);

function crc16(buf, previous) {
  if (!Buffer.isBuffer(buf)) buf = new Buffer(buf);

  var crc = ~~previous;

  for (var index = 0; index < buf.length; index++) {
    var byte = buf[index];
    crc = ((TABLE[(crc ^ byte) & 0xff] ^ (crc >> 8)) & 0xffff);
  }

  return crc;
}

function testBufferSplitValue(value, initial) {
  var middle = value.length / 2;
  var chunk1 = value.slice(0, middle);
  var chunk2 = value.slice(middle);
  var v1 = crc16(chunk1, initial);
  var v2 = crc16(chunk2, v1);

  return v2.toString(16);
}

var input = new Buffer('AR0AAAGP2KJc/vg/AAAAErgGAK8dAAgLAQAAPpo=', 'base64').slice(0, 27);

console.log('Expected f57c')
console.log('Actual  ', testBufferSplitValue(input));

And here are my results:

Node 5:

➜  node-crc git:(node-6-issue) ✗ nvm use 5
Now using node v5.9.1
➜  node-crc git:(node-6-issue) ✗ node --nocrankshaft src/crc16-node-6-simplified.js
Expected f57c
Actual   f57c

Node 6:

➜  node-crc git:(node-6-issue) ✗ nvm use 6
Now using node v6.9.1
➜  node-crc git:(node-6-issue) ✗ node --nocrankshaft src/crc16-node-6-simplified.js
Expected f57c
Actual   7d4c

Node 7:

➜  node-crc git:(node-6-issue) nvm use 7
Now using node v7.0.0
➜  node-crc git:(node-6-issue) node --nocrankshaft src/crc16-node-6-simplified.js
Expected f57c
Actual   f57c

If this helps, here's my uname

➜  node-crc git:(node-6-issue) uname -a
Darwin agorbatchev.local 15.6.0 Darwin Kernel Version 15.6.0: Thu Jun 23 18:25:34 PDT 2016; root:xnu-3248.60.10~1/RELEASE_X86_64 x86_64

It was something between node v6.2.2 and v6.3.0 that caused the change. The first suspect might be #7349?

Works as expected in 6.2.2

➜  node-crc git:(node-6-issue) nvm use 6.2.2
Now using node v6.2.2
➜  node-crc git:(node-6-issue) node --nocrankshaft src/crc16-node-6-simplified.js
Expected f57c
Actual   f57c

Yeah it looks like it has to do with the changes to Buffer slicing offset calculations.

Ref: #9341 ?

I've confirmed that #9341 fixes this.

@mscdex Thank you so much for checking in on this. What will happen next?

@alexgorbatchev Wait for that PR to land and it will get backported and be available in the next release.

Now that 6.9.2 is out with the referenced #9341 included, and:

marek@pesumasin-2(2016-12-12 15:13:29):~$ node --version
v6.9.2
marek@pesumasin-2(2016-12-12 15:13:33):~$ node crc16-node-6-simplified.js
Expected f57c
Actual   f57c
marek@pesumasin-2(2016-12-12 15:13:35):~$ node --nocrankshaft crc16-node-6-simplified.js
Expected f57c
Actual   f57c

Can this be declared to be fixed?

Yes, this should be fixed now. I'll go ahead and close the issue, thanks.

Thanks everyone, I really appreciate all the hard work that went into this!