r/javascript • u/ziriax • Jun 20 '16
Javascript developers, be warned about this crazy JIT bug in V8!
Last week I've spent a full day to fix very strange behavior in a single page application for my customer that only happens on Chrome 51 (maybe 52, but not in Canary 53).
After a while our SPA started behaving incorrectly...
It turned out to be a nasty bug in the V8 JIT engine.
Try the following in Chrome's developer console:
function foo() {
return typeof null === "undefined";
}
for(var i=0; i<10000; ++i) console.log(foo())This should of course always print false, but in Chrome 51, it prints true after about about 100 iterations (it varies...)
I want to warn other web-developers for this bug, to avoid many $$$ being wasted (or becoming insane ;-))
EDIT: The following - more realistic case that we had in third-party library code - will also fail, but after about 10 thousand iterations:
function isUndefined(thing){
return typeof thing === "undefined";
}
for(var i=0; i<100000; i++) console.log(isUndefined(null));
EDIT: The bug also manifests itself with the more common pattern
typeof x !== "undefined"UPDATE: I just tried this with the latest stable Chrome (52.0.2743.82), and it now works correctly :)
2
u/ziriax Jun 21 '16
Just to clarify, this bug obviously manifested itself in an indirect way, in thirdparty libraries we were using, that had code like:
export function isUndefined(obj: any): boolean {
return (typeof obj) === 'undefined';
}1
u/soddi Jun 21 '16
But that does not trigger the bug for me. Only if there is
typeof nulldirectly and not null passed into via variable indirectly.
45
u/fforw Jun 20 '16
Happens in Chrome 51, 64-bit Linux, too.
And it is clearly not a case of undefined behavior.
5
u/daytodave Jun 20 '16
Got it in Chrome 51, but it took 12292 iterations.
1
u/Browsing_From_Work Jun 21 '16
Odd. When I do the comparison as a function it takes fewer iterations.
function f() { return typeof null !== "undefined"; } var i; for (i = 0; f(); i++); console.log(i); // Outputs 12291, then subsequently starts outputting zeroesVersus:
var i; for (i = 0; typeof null !== "undefined"; i++); console.log(i); // Reliably outputs 18436
21
Jun 20 '16
I get about 20% falses on the first time executing, then 100% true on consecutive executions on Windows x64 Chrome 51.
2
u/MCFRESH01 Jun 20 '16
Seems like it always has failures if you copy and paste the function and the loop at the same time. Works as expected if you past the function and then run the loop
0
Jun 20 '16
[deleted]
1
u/repeatedly_once Jun 22 '16
I think it's because you're in the same thread when executing the JS. The first time it's executed is the first time the compiler has seen it after that is is associated null with undefined and does so ever more.
14
u/compteNumero9 Jun 20 '16
Doesn't seem to happen in node 5.10 but happens in node 6.0 and 6.2.2
1
u/bliow Jun 21 '16
Ditto 6.2.1, unsurprisingly.
I posted without seeing yours but I will leave mine up because it has my shell session.
1
Jun 21 '16
Can also confirm getting this on the 6.2.2 Linux x64 build.
I guess this is a good reason to go with stable instead of latest Node.
14
u/Pyrise Jun 20 '16
Anyone have any insight into why this happens?(Newbie here)
28
u/RainHappens Jun 20 '16
I found the bugfix.
Suffice to say (and I am probably wrong here, having next-to-no knowledge of the codebase) that the optimizer keeps track of "special" strings (booleans, "undefined", some integers, etc). It also does some type propagation (as in what types a variable etc can be). There's a special "undetectable" type for when a variable is undefined or null. In this case the optimizer goes "null is undetectable, so it can be replaced with the "undefined" string for the comparison. Oh look, both strings are the same!"
If you notice, this:
for(let i=0; i<10000; ++i) console.log(typeof null === 'object')exhibits the same incorrect behavior, albeit reversed. Same thing. The initial interpreter thinks 'typeof null' is 'object' for comparison purposes, but the optimizer thinks 'typeof null' is 'undefined' for comparison purposes.
7
Jun 21 '16
So scoping out the string should fix this right?
var uu = "undefined"; for(let i=0; i<10000; ++i) console.log(typeof null === uu);Works for me.
1
4
u/gsnedders Jun 20 '16
The initial interpreter
FWIW, V8 currently doesn't have an interpreter, they always run JIT'd code. (Ignition, which will finally add an interpreter, is still under development.)
2
1
u/RedditWithBoners Jun 20 '16
4
u/gsnedders Jun 20 '16
I presume the paragraph you mean is:
Note that the role that the interpreter plays in the case of a mixed-mode execution engine, namely providing fast startup, and also potentially collecting information and providing fallback capability may alternatively also be played by a second JIT compiler. This is how V8 works, for example. V8 never interprets, it always compiles. The first compiler is a very fast, very slim compiler that starts up very quick. The code it produces isn't very fast, though. This compiler also injects profiling code into the code it generates. The other compiler is slower and uses more memory, but produces much faster code, and it can use the profiling information collected by running the code compiled by the first compiler.
Calling it a second JIT is actually slightly confusing to me, I'm used to tiers being ordered by hotness (so the baseline JIT is the first tier, not the second). Regardless…
Essentially, while it is a very fast compiler, for code that runs only once or twice the overhead of compilation will almost certainly exceed any performance you gain by eliminating the dispatch overhead of an interpreter (which is the biggest gain of the baseline JIT). You also save a lot of memory by not having to allocate memory for the native code you're not generating, which on embedded devices can be significant.
These slides from BlinkOn 6 are a decent overview about motivating factors behind Ignition.
1
2
2
u/jeokrang Jun 21 '16
I have got questions. Why did v8 determine that null is a undetectable in this code? Are there any performance benefits, when null is replaced with "undefined"?
3
u/RainHappens Jun 21 '16
"undetectable" in this case means "I do not yet know what type this variable is".
v8 can do a lot of optimizations when variables are known to be of a specific type. But null and undef don't actually tell you anything about what type(s) a variable can be - they can effectively be any type. So as long as you only assign null or undef to a variable, v8 just goes "I don't know what type this variable is yet" and leaves it at that.
23
u/tweq Jun 20 '16 edited Jul 03 '23
2
1
u/MailDeadDrop Jun 22 '16
Interesting. I recall a security bug some years ago caused by the compiler realizing that the buffer we'd just memset to zero was never referenced again, so it optimized-away the memset operation, causing security info to be leaked via the no-longer-obliterated buffer (whoops!).
6
u/Cosmologicon Jun 20 '16
Good find. In case anyone is wondering, according to the bug report, checking for equality to undefined directly is not affected:
if (x === undefined) ...So those of us who do it this way don't have to worry about this particular bug.
5
Jun 20 '16
You do, however, have to worry about whether or not
xis defined, asx === undefinedwill throw an error wheretypeof x === "undefined"won't.3
u/qwzxercv Jun 21 '16
Can you clarify? I had the same thought to just compare against the true undefined... what errors would I expect if x was defined? I've never had issues with this before.
1
Jun 21 '16
unlike
x === undefined,typeof x === "undefined"doesn't throw a ReferenceError if the variable doesn't existit feels kind of hacky though and i consider it a bad part(tm) of the language as i can't think of a scenario where it couldn't be replaced with either
"varname" in globalorvarname !== undefined1
u/qwzxercv Jun 21 '16
You should not be attempting to use variables that do not exist. That's an error in any language. I don't think this is reason enough to avoid
x === undefined.2
Jun 21 '16
Have you written JavaScript for the web before? How can you tell whether or not your dependencies load? For example, let's say you have a
<script>tag that loadsfoo.js, which exports thefooobject. How can you tell whether or not the HTTP request failed, and whether or not you havefooin scope?The answer:
if (typeof foo !== "undefined") { // we have foo } else { // something went wrong }You could also solve this specific problem with jumping to the global scope and checking a property with a hacky one-liner (below), but it's intrinsically janky, non-portable, and it isn't The Right Way™.
(function () { return this; })().hasOwnProperty('foo')4
u/wyqydsyq Jun 22 '16
Dependency issues are mostly resolved in modern web dev thanks to build tools like webpack, browserify etc. (typeof x !== 'undefined') is mostly useful these days for checking existence of properties on dynamically generated or user-input objects, or for polyfills to test whether the browser implements a feature.
2
Jun 22 '16
Absolutely, my comment was probably over-simplified, but I was trying not to confuse them further. Thanks!
2
u/qwzxercv Jun 22 '16 edited Jun 22 '16
With modern JS development for the web, you definitely should not just throw global variables around on the page and hope that they exist. You should be using a module system to help mitigate this issue, which renders this discussion moot.
However, if you really need to rely on global variables, they do exist as properties on window, so if you need to check for its existence, you should check for
if (window.foo !== undefined) { ... }instead.I see no valid reason to be concerned about the existence of global variables when you shouldn't even be using global variables anyways.
1
Jun 22 '16
you definitely should not just throw global variables around on the page and hope that they exist
if (window.foo !== undefined) { ... }Two problems:
windowisn't guaranteed to be the global object in all JavaScript engines, so you'll want to replace that with(function () { return this; })()like I wrote above. For example, it returnswindowin Chrome andglobalin Node.
undefinedis a global variable, which, like you've expressed, shouldn't be trusted. What happens when someone setsundefined = 42? The solution is to usetypeof window.foo !== "undefined", at which point you may as well just be writingfoo !== "undefined".QED.
1
u/qwzxercv Jun 22 '16
First of all, you cannot overwrite the global undefined in modern JS engines anymore; that was fixed a while back. Secondly, I used
windowas an example for the global scope. Whether or not you havewindoworglobalmeans naught--all of points are still entirely valid. Thirdly, your function to retrieve the global context does not work in strict mode, which is what all modern JS moving forward is/should be written in.If you're concerned about the global
undefined, you are able to use thevoidkeyword to retrieve the true engine's undefined, but using the global undefined is safe on its own.0
Jun 22 '16
You're being intentionally dense, I'm done with this "discussion". Don't reply.
→ More replies (0)1
u/Cosmologicon Jun 20 '16 edited Jun 20 '16
True, I'm not recommending you change your code. But if you're already using this construct in places where it works (eg checking whether a function argument was specified), you don't need to worry about the bug.
2
Jun 20 '16
Correct! I probably should've been more clear, I was just throwing some information out for others who didn't know.
3
u/Nomikos Jun 20 '16
After it switches from false to true (around iteration 130), I also can't seem to delete foo anymore.
I can set it to other values (123, undefined, null) but 'delete foo' keeps returning false and it retains the last-set value.
Edit: ok, that is /also/ seemingly random..
2
2
1
u/guywithalamename Jun 20 '16
Wow how has this been open for so long?
6
u/guorbatschow Jun 20 '16
What are you talking about? The bug has been filed on Apr 15th and fixed on Apr 21th. Not even a full week.
8
u/guywithalamename Jun 20 '16
Sorry should have worded it better. I was talking about the fact that it's still not released apparently
18
1
3
Jun 20 '16
I've run into JIT bugs in other browsers too. What a pain in the ass. An easy fix is to disable the JIT by wrapping the effected code in a try-catch.
2
2
u/GentleMareFucker Jun 20 '16
Also happens in the latest node.js 6.2.2
I simply used
for (let i=0;i<10000;i++) {typeof null === 'undefined'}That loop only prints the final result when run on the console.
The threshold value for "false" turning into "true" was around 4610 for me. Using that value (plus minus a few numbers) I got "false" the first two times I ran that loop and from then on "true".
2
u/AlGoreBestGore Jun 20 '16
While this is trippy, when would you need to check if null's type is undefined?
0
Jun 20 '16
[deleted]
-4
u/hkrdrm Jun 20 '16
still wouldn't it be more proper to check
if(some_variable === null)
13
u/StoneCypher Jun 20 '16
no.
nullis a placeholder here.what he's saying is "if you compare things to typeof undefined, every hundred or so iterations it'll fail. look, it even happens with
null."3
u/bits_and_bytes Jun 20 '16 edited Jun 20 '16
I've tried a few different permutations and I can only get it to reliably happen with null (or a var set to null). So I think /u/hkrdrm and /u/AlGoreBestGore are right to be asking their questions. typeof null is "object" which is why you get the false returns for the first 100+ tries when comparing it to "undefined". But I've tried comparing other "object"s with "undefined" and I don't have the same problem. For example:
var thing = {}; function foo() { return typeof thing === "undefined"; } for(var i=0; i<10000; ++i) console.log(foo())This works just fine. But if thing = null, then it all blows up.
EDIT: I've found that the bug doesn't show up if null is in a var either:
var thing = null; function foo() { return typeof thing === "undefined"; } for(var i=0; i<10000; ++i) console.log(foo())This works correctly as well. It only seems to happen if you directly reference 'null' in the typeof comparison, and only if it's in a declared function. After the function starts returning true, it will always return true until it's re-initialized. A very strange bug indeed.
2
u/iamallamaa Jun 20 '16
I found that if the variable is defined within the function as null that it does still has the error.
function foo(){ var thing=null; return typeof thing === "undefined"; } for(var i=0; i<10000; i++) console.log(foo());265 false 9735 true
And when null is passed as an argument, it doesn't.
function foo(thing){ return typeof thing === "undefined"; } for(var i=0; i<10000; i++) console.log(foo(null));10000 false
1
u/ziriax Jun 21 '16
Try 100 thousand times... It will still fail at some point
function foo(thing){ return typeof thing === "undefined"; } for(var i=0; i<100000; i++) console.log(foo(null));12291 false 87709 true
Also, once this function is JIT optimized, it can keep failing if doesn't get de-optimized
2
-8
u/hkrdrm Jun 20 '16
I get that but what I'm saying if you have some function and i want to know if the argument is null wouldn't it make more sense to check it like this
function foo(my_arg) { if(my_arg === null){ console.log("arg is null here insert logic for null case here."); } }I'm not getting why anyone would need to check type equal to undefined.
14
u/StoneCypher Jun 20 '16
Because he's not checking if it's
null, and you can't actually strict-equals compare toundefinedin this language in a bunch of common circumstances.It is very common to have to check if something is
undefined. By example, if you retain a JSON object from an API, and you want to check whether the server actually set some or another value, before you attempt to use it.He's talking about the
undefinedcomparison.I'd like you to stop focussing on
null, please. It's beside the point. Thank you1
u/RReverser Jun 21 '16
and you can't actually strict-equals compare to undefined in this language in a bunch of common circumstances.
Of course you can. The only case where you can't is if you're checking existance of a global variable for feature detection, in all the other cases === undefined works like a charm.
3
u/agmcleod @agmcleod Jun 20 '16
Properties on objects are not initialized to null by default. No doubt having your objects well defined with sensible defaults goes a long way, but this is still a nasty bug that shouldn't happen.
0
Jun 20 '16
[deleted]
11
u/compteNumero9 Jun 20 '16
Look at the bug report, it's not only for this case. But you might have an unaffected version of Chrome/node
0
Jun 20 '16
[deleted]
2
u/compteNumero9 Jun 20 '16
Hard to tell how the code is optimized in your case. It's possible for example that the
foo(baz)function is just optimized away and replaced with the result.3
Jun 20 '16
but when you move variable to that function scope...
function foo() { var bar = null; return typeof bar === "undefined"; } for(var i=0; i<10000; ++i) console.log(foo())1
u/Buckwheat469 Jun 20 '16 edited Jun 20 '16
I've created 4 different examples, after moving some code around I can no longer get it to reproduce the problem on the last 3 examples. The first example still returns true on Chrome 51 Linux x64. I want to point out that it did reproduce the error more often at first but I noticed some bad code and moved some functions around. This seemed to fix the problem.
https://jsfiddle.net/sssn9fvo/2/
Edit: I just noticed that the link above has an extra foo() function. As soon as I deleted this function it started working. Perhaps the interpreter is jumping between 2 different hoisted functions?
1
1
u/55704841711534631249 Jun 20 '16
That fix stops working if you set the loop to 100,000 cycles (at around the 12k mark)
function foo(bar) { return typeof bar === "undefined"; } var baz = null; for(var i=0; i<100000; ++i) console.log(foo(baz))
12291 false VM1493:5 87709 true VM1493:5 undefinedhttps://postimg.org/image/sdwp5ttcn/ https://s31.postimg.org/t3fhi6tw9/failcase1.png
A better fix would be to add true to the expression inside the function:
function foo() { return typeof(null) === "undefined" + true; } for(var i=0; i<100000; i++) console.log(foo())
100000 false VM1494:5 undefinedhttps://postimg.org/image/gctc5qm9z/ https://s31.postimg.org/c3om3kj0p/wincase1.png
This is on OS X 10.11.5, Chrome 51. I haven't tested it on Windows, so YMMV.
6
u/leesusmusic Jun 20 '16 edited Jun 21 '16
Am I missing something? "undefined" + true will evaluate to "undefinedtrue", no?
3
u/barrtender Jun 20 '16
The "===" operator is after the "+" operator, so yes.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Operator_Precedence
1
1
Jun 20 '16
Confirmed not a problem in 53, doesn't happen for me at all, it always prints false.
Windows 7 Chrome Version 53.0.2756.0 dev-m (64-bit)
1
Jun 20 '16
So if you run it with the console.log in the loop all of them evaluate false
for(var i=0; i<10000; ++i) console.log(typeof null === 'undefined')
// 10000 falseBut if you change the var to a let and run the console.log right in the loop it's almost 50/50 split
for(let i=0; i<10000; ++i) console.log(typeof null === 'undefined')
//4917 false
//5083 true1
u/RReverser Jun 21 '16
Perhaps you were just running in the DevTools console and not as a script - that would result in a different behavior as in the first case you're introducing a global variable (aka window.i).
1
Jun 21 '16 edited Jun 21 '16
Shouldn't matter the variable isn't even used, regardless if you wrap each of those in an IIFE the result is the same.
1
u/RReverser Jun 30 '16
It does matter because in one case it's just local variable which can be optimized away, while in another it's a property access on a global object which can't be - very different operations, with different handling and performance.
1
Jun 30 '16
As I said I've put this in an IIFE, effectively making a local variable, and the behaviour was the same.
1
1
1
u/raptorok Jun 20 '16
Version 51.0.2704.103 (64-bit) prints 'false' 130-150 times, then rest is 'true'
1
u/bliow Jun 21 '16
Repros in Node 6.2.1
$ cat v8_bug.js
function foo() {
return typeof null === "undefined";
}
for(var i=0; i<10000; ++i) console.log(foo())
$ node v8_bug.js > v8_bug.out
$ uniq -c ~/tmp/v8_bug.out
132 false
9868 true
$ node --version
v6.2.11
u/vooydzig Jun 21 '16
Semi-newbie question: why not this: return typeof null === typeof undefined; ? It always returns false since object !== undefined.
1
u/afternoonshore Jun 22 '16
Semi-newbie? Seems like you know
(typeof null); // "object" (typeof undefined); // "undefined"// anyway, the bug is that ...
for (let i = 0; i < 10000; i++){ console.log(typeof null === 'undefined'); }// somehow returns 50% false and 50% true in Chrome yet
for (let i = 0; i < 10000; i++){ console.log(typeof null === typeof undefined); }// returns 100% false, so you're right in your suggestion ... of course traditional use of var ...
for (var i = 0; i < 10000; i++){ console.log(typeof null === 'undefined'); }// returns 100% false ... interestingly enough, declaring let before the loop ...
let i; for (i = 0; i < 10000; i++){ console.log(typeof null === 'undefined'); }// returns 100% false ................ what a crazy weird error
1
u/vooydzig Jun 23 '16
Semi-newbie as in "I've been programming for some time now, not necessarily in JS" ;) it's just that I don't why you'd want to check types by string comparison not types comparison.
1
u/izuriel Jun 23 '16
Now it's obvious you haven't messed with JS, there aren't really any types to compare other than strings. The
typeof"operator" returns a string value (see here) so that's why there is string comparison. Of course I've never seen anisUndefinedthat only checked againstundefined. For all intents and purposesnullis usually considered "undefined." But that doesn't change the fact this is a weird bug.2
u/vooydzig Jun 24 '16
Yeah, i get all that. What I mean is: let say I want to check if variable type is number. I'd rather do
typeof var_name === typeof 0thantypeof var_name === "Number". My reasoning is that on both sides of comparison I'd use operation to get type(as string) instead of hardcoding the type, because if someone changes specs for JS(let say ES9?) and rename number type to "whatever" first code would still work. As I've said I really don't write much JS so maybe there are some downsides to what I suggest here.2
u/izuriel Jun 24 '16
Not a bad point. I never questioned why strings are typically used. This is just the way I've always seen it.
1
u/TotesMessenger Jun 21 '16
1
1
u/kqvrp Jun 22 '16
I get this in Chromium Version 50.0.2661.102 (64-bit) on Linux as well. I thought this was a Chrome 51 issue?
1
1
u/brunolemos Jun 25 '16
I created a gif running this code on Chrome and on Safari: http://www.giphy.com/gifs/3o72Fb14R1TlauqbhC
1
u/medleyj Jun 27 '16
FYI. This has been reported to Chrome here: https://bugs.chromium.org/p/chromium/issues/detail?id=621887
0
0
u/strixvarius Jun 20 '16
Instead of guessing a loop size, just:
let x=true; while (x) { console.log(x=typeof null==='object') }1
Jun 21 '16
You're going to want a limit counter on that, otherwise if it works as intended it's going to hang
-4
u/Jafit Jun 20 '16 edited Jun 20 '16
I want to warn other web-developers for this bug, to avoid many $$$ being wasted (or becoming insane ;-))
Consider using a highly opinionated linter.
JSLint would flag return typeof null === "undefined"; as a weird statement.
You don't have to use JSLint, since Crockford is very opinionated and very draconian. At my job we use ESLint with a ton of rules turned on both to catch errors and enforce a consistent coding style. For a language like Javascript I'd say its a necessity because there's so many different ways to do things.
Also I'm not sure why you're actually using this statement in your code.
In any case I hope you filed a bug report. I'd be curious to see if this affects node.js since node uses V8 as well.
8
Jun 21 '16
He probably isn't but iterating over a list of entires with a lot of nulls and getting false positives. Use some common sense, bud.
-1
u/Jafit Jun 21 '16
So you'd use
typeof variable === "undefined"to look for null... whentypeof null === "object"?That's not a great way to look for null values, bud.
1
Jun 21 '16
Super cool, but again you are assuming what his production code looks like. Either it doesn't look like that at all -or- he is doing something "not a great way". It doesn't matter.
Either way, my statement still stands.
1
u/ziriax Jun 22 '16
I edited my post, to reflect the real code. The code was part of thirdparty libraries that checked if a value was undefined. Many popular libraries have code likes that.
-6
0
Jun 20 '16
[deleted]
9
2
u/LukaLightBringer Jun 21 '16
Why not just use the JS CPU profiling tool in the chrome dev tools?
3
u/compteNumero9 Jun 21 '16
I guess it was a joke.
I hope so, at least.
1
0
37
u/theofREDDIT Jun 20 '16
Good job digging that deep in one day !!