Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

node-crc failing on node 6.x only #9342

Closed
alexgorbatchev opened this issue Oct 28, 2016 · 12 comments
Closed

node-crc failing on node 6.x only #9342

alexgorbatchev opened this issue Oct 28, 2016 · 12 comments
Labels
buffer Issues and PRs related to the buffer subsystem. confirmed-bug Issues with confirmed bugs.

Comments

@alexgorbatchev
Copy link

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;
});
@bnoordhuis
Copy link
Member

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?

@mscdex mscdex added question Issues that look for answers. v6.x labels Oct 28, 2016
@alexgorbatchev
Copy link
Author

alexgorbatchev commented Oct 28, 2016

@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

@alexgorbatchev
Copy link
Author

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

@mscdex
Copy link
Contributor

mscdex commented Oct 28, 2016

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

@alexgorbatchev
Copy link
Author

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

@mscdex
Copy link
Contributor

mscdex commented Oct 28, 2016

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

Ref: #9341 ?

@mscdex mscdex added confirmed-bug Issues with confirmed bugs. and removed question Issues that look for answers. labels Oct 28, 2016
@mscdex
Copy link
Contributor

mscdex commented Oct 28, 2016

I've confirmed that #9341 fixes this.

@alexgorbatchev
Copy link
Author

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

@mscdex
Copy link
Contributor

mscdex commented Oct 28, 2016

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

@manjabes
Copy link

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?

@bnoordhuis
Copy link
Member

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

@mscdex mscdex added the buffer Issues and PRs related to the buffer subsystem. label Dec 12, 2016
@alexgorbatchev
Copy link
Author

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

abread added a commit to cansat-icarus/capture-wrapper-desktop that referenced this issue May 1, 2017
Electron 1.4 uses Node 6.5.0, where crc may not work as expected:
nodejs/node#9342 (present until Node 6.9.2).
Electron 1.6.5 uses Node 7.4, where the issue does not exist.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buffer Issues and PRs related to the buffer subsystem. confirmed-bug Issues with confirmed bugs.
Projects
None yet
Development

No branches or pull requests

4 participants