Skip to content
This repository

Object comparison fails in v0.8 #3867

Closed
mattijs opened this Issue August 14, 2012 · 9 comments

6 participants

Mattijs Hoitink Ben Noordhuis Stéphan Kochen Mariusz Nowak Douglas Christopher Wilson Bert Belder
Mattijs Hoitink

In some cases Object comparison fails in node v0.8. While this first occurred with a certain order of require statements using the Backbone, Underscore and Require modules (see jashkenas/backbone#1543), this does not seem to be the problem.

After some testing it seemed to occur when requiring the http module and with more prying when requiring the punycode module. The bug can be recreated with a small test script so punycode does not seem to be the problem directly.

The test script can be found at https://gist.github.com/3351832

The test script has two identical methods for comparing objects (similar to Underscore) but with two triggers fails to recognize that a String is not an Object. Removing any of the two triggers from the script results in normal behavior. After triggering on comparison method (isObjectA) the other method (isObjectB) does return the correct result while the triggered method (isObjectA) does not.

I tested the script with v0.8.6 where it fails and with v0.6.20 where it behaves normal.

Ben Noordhuis

I can't reproduce it but it could be a bug in one of V8's code generators. What do node -pe process.versions and file $(which node) print?

Do you see the same error when you build V8 standalone? Here is how you do that:

$ cd deps/v8
$ ln -s ../../../tools/gyp build/gyp
$ make -j 8 native
$ out/native/d8 script.js # <- your test case

Replace the console.log statements with print statements.

Mattijs Hoitink

Test done with node v0.8.6 on OSX 10.7

$ node --version
  v0.8.6

$ node -pe process.version
  v0.8.6

$ file $(which node)
  /usr/local/bin/node: Mach-O 64-bit executable x86_64

$ node test.js
  isObjectA(str): true
  isObjectB(str): false
  inline check:   false

I built the v8 dep from the master branch and from the v0.8 branch with the instruction above. Test outcome:

$ node.git/deps/v8/out/native/d8 test.v8.js
  isObjectA(str): true
  isObjectB(str): false
  inline check:   false

I had some colleagues run the script too but they are also all on a Mac. The script fails there too.

Stéphan Kochen

On EC2, 64-bit:

$ ./node -pe process.version
v0.8.6

$ file ./node
./node: ELF 64-bit LSB executable, x86-64, version 1 (GNU/Linux), dynamically linked (uses shared libs), for GNU/Linux 2.6.24, BuildID[sha1]=0x9890cd5fd4ca2011cd8586f80607f122252b3b93, not stripped

$ ./node test.js 
isObjectA(str): true
isObjectB(str): false
inline check:   false

And on EC2, 32-bit:

$ ./node -pe process.version
v0.8.6

$ file ./node
./node: ELF 32-bit LSB executable, Intel 80386, version 1 (GNU/Linux), dynamically linked (uses shared libs), for GNU/Linux 2.6.24, BuildID[sha1]=0xbe04750a6fb16b3c29612c41d2e490d837be65ee, not stripped

$ ./node test.js
isObjectA(str): true
isObjectB(str): false
inline check:   false
Mariusz Nowak

Indeed, very strange and scary bug, I reproduced that on node v0.8.6 and OSX 10.8

It's definitely V8 bug, I've checked and unfortunately it still exists in trunk. I've simplified a test case and posted issue to V8 project: http://code.google.com/p/v8/issues/detail?id=2291

Ben Noordhuis

For the record, I'm able to reproduce it and the bug still exists in V8's HEAD. Here is a minimal test script:

function a(o) {
    return o === Object(o);
}
function b(o) {
    return o === Object(o);
}

var o = {};
o == o; // EQ_STRICT does *not* trigger the bug
b(o);

if (a(0)) throw Error('a(s) === true'); // okay
if (b(0)) throw Error('b(s) === true'); // error
1;

It's interesting that V8 compiles a and b to exactly the same code. It's the non-strict comparison before the call to b that triggers it.

Ben Noordhuis bnoordhuis closed this August 15, 2012
Ben Noordhuis

Okay, looks like it'll be fixed soon: https://chromiumcodereview.appspot.com/10830334

Douglas Christopher Wilson

I know this issue is closed because it has been patched in some release of V8, but I was wondering when this patch would make it downstream and into node a v0.8. The current v0.8 branch HEAD still shows this bug is active (v8 3.11.10.18). Upstream there is now a v8 3.11.10.19, which includes the patch for this.

Bert Belder
Collaborator

Ya. V8 was upgraded in 2d92393 so this will be fixed in the upcoming v0.8.8 release.

Douglas Christopher Wilson

Awesome, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.