Uglifyjs: ufuzz failure

[alexlamsl]

// original code
// (beautified)
var _calls_ = 10, a = 100, b = 10, c = 0;

function f0(Infinity, NaN, bar_1) {
    {
        var brake1 = 5;
        while ((1 === 1 ? a : b) && --brake1 > 0) {}
    }
    {
        var expr3 = [ (c = c + 1) + /[abc4]/g.exec(([ bar_1 += bar_1 ^= (c = c + 1) + (typeof f2 == "function" && --_calls_ >= 0 && f2((c = 1 + c, 
        -3 >> "number" > -4 <= ("a" >= [ , 0 ][1]) >>> (22 >>> 24..toString())), (c = 1 + c, 
        (-4 / 4 != false - undefined) / ((bar_1 && (bar_1[c = 1 + c, ("" || Infinity) > ("object", 
        -2) ^ (25 & [ , 0 ][1], -3 & [ , 0 ].length === 2)] /= NaN % 22)) > ([ , 0 ][1] !== 24..toString()))), (c = 1 + c, 
        bar_1 && (bar_1.Infinity = [] != 2) || bar_1 && (bar_1[c = 1 + c, (22 / "object" & 1 > "function") - ((true >>> NaN) + (23..toString() < null))] = Infinity + false), 
        ([ , 0 ][1] >= 23..toString()) / (c = c + 1, "a")))), a++ + {
            Infinity: /[abc4]/g.exec(((c = 1 + c, bar_1 = (null >> 25) - [] / /[a2][^e]+$/, 
            "" !== 25 && NaN > Infinity) || b || 5).toString()),
            in: (c = c + 1) + (b + 1 - .1 - .1 - .1),
            0: (c = c + 1) + +(((NaN, [ , 0 ].length === 2) | ("undefined" | [ , 0 ][1])) >> ("a" - 24..toString() < "function" >>> "a")),
            c: (c = c + 1) + +(c = c + 1, 38..toString() === "undefined" ^ "object" < -1)
        }, (c = c + 1) + function a_2() {
            {
                var brake4 = 5;
                while ((c = 1 + c, ((24..toString() === -2) > (22, "a")) << (23..toString() < 25 & null <= "foo")) && --brake4 > 0) {
                    c = 1 + c, (c = c + 1, "number" << 25) * +(c = c + 1, true);
                }
            }
            try {
                c = 1 + c, (-3 >> {}, 38..toString() !== true) | (NaN | "foo") > ("foo" == 0);
            } finally {
            }
        }, typeof f0 == "function" && --_calls_ >= 0 && f0("a", {
            c: (c = 1 + c, ((-1 || "undefined") === 25 > {}) >>> (25 >> -5 >>> (true || 38..toString())))
        }), --b + (b + 1 - .1 - .1 - .1) ] || b || 5).toString()), b++, a++ + void function() {
            c = c + 1;
            c = c + 1;
            {
                var b_2 = function f1(a_2, bar) {
                    {
                        var brake11 = 5;
                        while ((c = 1 + c, (a_2 && (a_2[(c = c + 1) + [].undefined] = (a_2 && (a_2[c = 1 + c, 
                        (-0 === 23..toString()) <= (a_2 && (a_2[c = 1 + c, (false < "c") - (bar_1 %= -0 + -4) << (-4 - "object" | true < "function")] = Infinity != 38..toString())), 
                        25 - "c" >= (2 !== "number")] += "" >>> null)) * (a_2 && (a_2[c = 1 + c, (a_2 = 1 >> {}) + (3 || -1) < ("a" !== ([ , 0 ].length === 2)) % ("foo" === false)] = ([ , 0 ].length === 2, 
                        undefined))))) != ("" * -1 || "" <= "c")) && --brake11 > 0) {
                            c = 1 + c, 4 * null >> (bar_1 && (bar_1.c += {} === false)) >> delete ("number", 
                            true);
                        }
                    }
                    switch (c = 1 + c, (a_2 &= -3 % "function" < ("undefined" ^ "c")) << (22 & -3 && "" ^ "a")) {
                      default:
                        ;

                      case c = 1 + c, ((22 ^ -2) & (bar_1 += -5 < "bar")) + (("" ^ "b") !== (false === "object")):
                        ;
                        break;

                      case c = 1 + c, 4 % "a" && 3 > "number" && 22 >>> true === (24..toString() && "function"):
                        ;
                        break;

                      case c = 1 + c, ({} == "a") * (-0 === 4) * ((undefined ^ [ , 0 ][1]) <= ("number" <= true)):
                        ;
                        break;
                    }
                }(2, 24..toString(), (c = c + 1) + (typeof bar_1 == "function" && --_calls_ >= 0 && bar_1(-1, "foo")));
            }
            c = c + 1;
        }(), --b + b-- ];
        for (var key3 in expr3) {
            for (var brake15 = 5; a++ + (1 === 1 ? a : b) && brake15 > 0; --brake15) {
                var brake16 = 5;
                L14093: do {
                } while (a++ + (b /= a) && --brake16 > 0);
            }
        }
    }
}

var foo_1 = f0(1, (c = c + 1) + (b -= a), --b + ([ , 0 ].length === 2));

console.log(null, a, b, c, Infinity, NaN, undefined);

// uglified code
// (beautified)
var g = 10, c = 100, d = 10, u = 0;

(function S(i, n, o) {
    for (var t = 5; c && 0 < --t; ) {}
    var e = [ (u += 1) + /[abc4]/g.exec([ o += o ^ (u += 1) + ("function" == typeof f2 && 0 <= --g && f2((u = 1 + u, 
    !0 <= !1 >>> (22 >>> 24..toString())), (u = 1 + u, (-1 != !1 - undefined) / ((o && (o[u = 1 + u, 
    -2 < i ^ -3 & 2 === [ , 0 ].length] /= n % 22)) > (0 !== 24..toString()))), (u = 1 + u, 
    o && (o.Infinity = 2 != []) || o && (o[u = 1 + u, 0 - ((!0 >>> n) + (23..toString() < null))] = i + !1), 
    (23..toString() <= 0) / (u += 1, "a")))), c++ + {
        Infinity: /[abc4]/g.exec((u = 1 + u, o = 0 - [] / /[a2][^e]+$/, (i < n || d || 5).toString())),
        in: (u += 1) + (d + 1 - .1 - .1 - .1),
        0: (u += 1) + +((2 === [ , 0 ].length | 0) >> ("a" - 24..toString() < 0)),
        c: (u += 1) + (u += 1, +("undefined" === 38..toString() ^ !1))
    }, (u += 1) + function() {
        for (var n = 5; u = 1 + u, ("a" < (-2 === 24..toString())) << (23..toString() < 25 & !1) && 0 < --n; ) {
            u = 1 + u, u += 1, u += 1;
        }
        u = 1 + u, 38..toString();
    }, 0 <= --g && S("a", {
        c: (u = 1 + u, (-1 === {} < 25) >>> 0)
    }), --d + (d + 1 - .1 - .1 - .1) ].toString()), d++, c++ + (u += 1, u += 1, function(n) {
        for (var t = 5; u = 1 + u, 1 != (n && (n[(u += 1) + [].undefined] = (n && (n[u = 1 + u, 
        23..toString(), n && (n[u = 1 + u, !1 - (o %= -4) << 0] = i != 38..toString()), 
        !1] += 0)) * (n && (n[u = 1 + u, 3 + (n = 1 >> {}) < ("a" !== (2 === [ , 0 ].length)) % !1] = undefined)))) && 0 < --t; ) {
            u = 1 + u, o && (o.c += !1 === {});
        }
        switch ((n &= !1) << 0) {
          default:
          case u = 1 + (1 + u), (-24 & (o += !1)) + !0:
          case u = 1 + u, NaN:
          case u = 1 + u, !1 * ("a" == {}) * ((0 ^ undefined) <= !1):
        }
    }(2, (24..toString(), u += 1, "function" == typeof o && 0 <= --g && o(-1, "foo"))), 
    void (u += 1)), --d + d-- ];
    for (var r in e) {
        for (var f = 5; c++ + c && 0 < f; --f) {
            var a = 5;
            do {} while (c++ + (d /= c) && 0 < --a);
        }
    }
})(1, (u += 1) + (d -= c), --d + (2 === [ , 0 ].length));

console.log(null, c, d, u, Infinity, NaN, undefined);



md5-a760f5ef1c5765694e08c4c2434d81b9



original result:
null 1486 -0 506 Infinity NaN undefined

uglified result:
null 1486 -8.88053083e-316 506 Infinity NaN undefined



md5-a760f5ef1c5765694e08c4c2434d81b9



// reduced test case (output will differ)

// Can't reproduce test failure
// minify options: {
//   "ie8": true,
//   "toplevel": true
// }



md5-a760f5ef1c5765694e08c4c2434d81b9



minify(options):
{
  "ie8": true,
  "toplevel": true
}

[alexlamsl]

@kzc another one like #3914 − ran on GitHub Actions using Windows Server 2019.

~Seems more repeatable, as the original & uglified code both get beautified.~ Only two sets of options failed − the one above and:

{
  "compress": {
    "passes": 1000000,
    "unsafe": true
  },
  "toplevel": true
}

... which likely means only toplevel sandbox.run_code() on the original test case gave the wrong results.

[kzc]

Odd coincidence about toplevel since it just makes an IIFE wrapper in the sandbox. The sandbox ought to be deterministic since it no longer recycles VM instances.

My guess is that it's a bug in the Node v10 version. Reducing such "Can't reproduce test failure" cases a second time could be interesting. If the failing case is reducible on the second run with the same options then it might suggest that it's hardware related, but it's still not definitive. Either way, it's not a minify issue. It would be just for the sake of curiosity.

You could also hack ufuzz to only return the original test case from this issue and run ufuzz for an hour with that version of Node to see what happens.

[alexlamsl]

Given the speculative nature of major processor design, I don't think we can rely on any determinism there, since we aren't the only thing running on it 😅

So I was wrong about this being repeatable − the bug popped up exactly once in the full run of this test case:
https://github.com/mishoo/UglifyJS/blob/491d6ce1d5f8e49730de25788cabf9e213470588/test/ufuzz/index.js#L1244
which is enough to generate everything we see here.

[kzc]

Given the speculative nature of major processor design

Not unlike the speculative nature of this discussion. :-)

I appreciate the inclination to suspect a hardware error from the -8.88053083e-316 versus -0 result. While hardware bugs are possible, the vast majority of the time it's usually a software error - whether it's the user code, or the incredibly complex multithreaded JIT engine, or the NodeJS multithreaded wrapper, or the C++ compiler that built them, or the system libs, or the underlying OS itself. And nothing can be proven without repeatability. Generally hardware is only examined after all software variables have been excluded.

[alexlamsl]

While hardware bugs are possible, the vast majority of the time it's usually a software error

For the record, I perfectly concur on that point − I raised this possibility in the original discussion as a fringe factor.

The most likely candidate as far as I am concerned is the inconsistent behaviour between interpreter and various optimising compilers of V8, and by inconsistent I mean at least one of those execution paths is buggy :ghost:

[alexlamsl]

Case in point: there are now quite a handful of false positives that when I investigate using --reduce-test locally with Node.js 10 or later, the process would literally rage-quit half way through, i.e. not even process.on("uncaughtException") could get you any printouts.

So far said undesirable behaviour has not been observed on Node.js 8 or below.

[alexlamsl]

Evidence to support this theory:

$ node
> var run = sandbox.run_code;
> code = UglifyJS.minify(fs.readFileSync("3965.js", "utf8"),{
      compress: false,
      mangle: false,
  }).code;
'var _calls_=10,a=100,b=10,c=0;function f0(Infinity,NaN,bar_1){{var brake1=5;while((1===1?a:b)&&--brake1>0){}}{var expr3=[(c=c+1)+/[abc4]/g.exec(([bar_1+=bar_1^=(c=c+1)+(typeof f2=="function"&&--_calls_>=0&&f2((c=1+c,-3>>"number">-4<=("a">=[,0][1])>>>(22>>>24..toString())),(c=1+c,(-4/4!=false-undefined)/((bar_1&&(bar_1[c=1+c,(""||Infinity)>("object",-2)^(25&[,0][1],-3&[,0].length===2)]/=NaN%22))>([,0][1]!==24..toString()))),(c=1+c,bar_1&&(bar_1.Infinity=[]!=2)||bar_1&&(bar_1[c=1+c,(22/"object"&1>"function")-((true>>>NaN)+(23..toString()<null))]=Infinity+false),([,0][1]>=23..toString())/(c=c+1,"a")))),a+++{Infinity:/[abc4]/g.exec(((c=1+c,bar_1=(null>>25)-[]/ /[a2][^e]+$/,""!==25&&NaN>Infinity)||b||5).toString()),in:(c=c+1)+(b+1-.1-.1-.1),0:(c=c+1)+ +(((NaN,[,0].length===2)|("undefined"|[,0][1]))>>("a"-24..toString()<"function">>>"a")),c:(c=c+1)+ +(c=c+1,38..toString()==="undefined"^"object"<-1)},(c=c+1)+function a_2(){{var brake4=5;while((c=1+c,((24..toString()===-2)>(22,"a"))<<(23..toString()<25&null<="foo"))&&--brake4>0){c=1+c,(c=c+1,"number"<<25)*+(c=c+1,true)}}try{c=1+c,(-3>>{},38..toString()!==true)|(NaN|"foo")>("foo"==0)}finally{}},typeof f0=="function"&&--_calls_>=0&&f0("a",{c:(c=1+c,((-1||"undefined")===25>{})>>>(25>>-5>>>(true||38..toString())))}),--b+(b+1-.1-.1-.1)]||b||5).toString()),b++,a+++void function(){c=c+1;c=c+1;{var b_2=function f1(a_2,bar){{var brake11=5;while((c=1+c,(a_2&&(a_2[(c=c+1)+[].undefined]=(a_2&&(a_2[c=1+c,(-0===23..toString())<=(a_2&&(a_2[c=1+c,(false<"c")-(bar_1%=-0+-4)<<(-4-"object"|true<"function")]=Infinity!=38..toString())),25-"c">=(2!=="number")]+="">>>null))*(a_2&&(a_2[c=1+c,(a_2=1>>{})+(3||-1)<("a"!==([,0].length===2))%("foo"===false)]=([,0].length===2,undefined)))))!=(""*-1||""<="c"))&&--brake11>0){c=1+c,4*null>>(bar_1&&(bar_1.c+={}===false))>>delete("number",true)}}switch(c=1+c,(a_2&=-3%"function"<("undefined"^"c"))<<(22&-3&&""^"a")){default:;case c=1+c,((22^-2)&(bar_1+=-5<"bar"))+((""^"b")!==(false==="object")):;break;case c=1+c,4%"a"&&3>"number"&&22>>>true===(24..toString()&&"function"):;break;case c=1+c,({}=="a")*(-0===4)*((undefined^[,0][1])<=("number"<=true)):;break}}(2,24..toString(),(c=c+1)+(typeof bar_1=="function"&&--_calls_>=0&&bar_1(-1,"foo")))}c=c+1}(),--b+b--];for(var key3 in expr3){for(var brake15=5;a+++(1===1?a:b)&&brake15>0;--brake15){var brake16=5;L14093:do{}while(a+++(b/=a)&&--brake16>0)}}}}var foo_1=f0(1,(c=c+1)+(b-=a),--b+([,0].length===2));console.log(null,a,b,c,Infinity,NaN,undefined);'
> expected = run(code, true);
'null 1486 -8.88053083e-316 506 Infinity NaN undefined\n'

> for (var i = 0; (actual = run(code,true)) === expected; i++);
> console.log(i, actual);
2268 'null 1486 -0 506 Infinity NaN undefined\n'

> for (var i = 0; (actual = run(code,true)) === expected; i++);
> console.log(i, actual);
496 'null 1486 -0 506 Infinity NaN undefined\n'

> for (var i = 0; (actual = run(code,true)) === expected; i++);
> console.log(i, actual);
0 'null 1486 -0 506 Infinity NaN undefined\n'

> for (var i = 0; (actual = run(code,true)) === expected; i++);
> console.log(i, actual);
0 'null 1486 -0 506 Infinity NaN undefined\n'

Only seems to affect Node.js 10 − both 8 or 12 run for a few minutes without failure.

[kzc]

Glad you were able to isolate it. A bug in Node v10 was the most likely explanation.

But Node 12+ has runtime characteristics that are undesirable for fuzzing - like overcommitting memory for huge arrays instead of immediately failing. So what version would you use going forward?

[alexlamsl]

I'm going to have some fun first hacking --reduce-test to try and narrow down the scope of this V8 bug, but after that I'll measure the performance difference just to make sure we can revert back to Node.js 8 without major loss of fuzzing rate.

[alexlamsl]

Unfortunately it requires a new process between run to test reproducibility, but here's reduced test case:

var a = 59, b = 0;
for (var k = 0; k < 10; k++) {
    a++;
    b--;
    for (var i = 0; i < 20; i++) {
        a++;
        for (var j = 0; j < 5; j++) {
            b /= ++a;
        }
    }
}
console.log(a, b);

[alexlamsl]

Code I use to make this fail with Node.js 10:

var code = require("fs").readFileSync("test.js", "utf8");
var run = require("./test/sandbox").run_code;
var expected = run(code, true);
for (var i = 0; run(code, true) === expected && i < 10000; i++);
console.log(i);

[alexlamsl]

Although Node.js 12 doesn't exhibit the same bug, it is a lot slower than Node.js 8 for some reasons.

[kzc]

It's a surprisingly simple test case without dynamic memory allocation. If you're curious as to the cause, you could git bisect NodeJS, but its local V8 source tree merges are probably too coarse-grained to see what fixed it. If the issue can be reproduced in d8 then the V8 mirror could be bisected to find the fine-grained fix. But it's possible that the bug might be in the NodeJS wrapper for V8 vm instances.

[kzc]

Were you able to hack test/reduce to make that test case?

[alexlamsl]

Unfortunately not, as you will need to spawn new node process for every comparison, so it's just quicker to do it by hand then to tweak reduce.js 😅

I reckon it's less of the fact that V8 fixed this issue, but rather they wrote yet another optimising compiler so the bug got thrown out with the bath water.

[alexlamsl]

Given that Node.js 12 is significantly slower than Node.js 10 when running this test, while my local testing with Node.js 8 shows <20% slowdown in rate of fuzzing, I think it's better to downgrade.

At least we are not getting bogged down with concerns over latest and greatest language features here :smirk:

[kzc]

If that's the case, maybe an earlier version of Node 10.x using an earlier version of V8 might not have this bug.

[alexlamsl]

Good point, let me try that...

[alexlamsl]

No luck so far − Node.js 10.0.0 still fails, albeit at a different number of iterations.

Node.js 10.10.0 fails at the same point as 10.20.1

[kzc]

Oh well, it was worth a try.

[alexlamsl]

I think they only match major releases of V8 to major versions of Node.js, and only picking up upstream patches for minor/patch versions.

[kzc]

What about the unsupported odd numbered versions like Node 11.x?

[alexlamsl]

Node.js 9.11.2 works 🆗
Node.js 11.15.0 fails ❌ (does so even faster than 10.20.1)

[kzc]

Do all versions of Node 11.x fail? If so, I guess it's a question of whether there's a stable version of Node 9.x that runs ufuzz appreciably faster than Node 8.x.

[kzc]

Also take a look at the node --v8-options to see if you can disable/enable JIT engines (crankshaft, turbofan, turboprop, etc) or specific JIT engine features on a given node version. With some luck you can avoid the bug and retain decent speed.

For example:

$ node-v14.3.0 --v8-options | grep turboprop
  --turboprop (enable experimental turboprop mid-tier compiler.)

The v8 options also include settings for SSE, AVX, etc which might influence the -0 error.

[alexlamsl]

Node.js 9 has around the same fuzz rate as Node.js 8

I know about the v8 flags, though I suspect it's the same buggy optimising compiler which is giving us the speed up 😅

Taking a 20% hit isn't the end of the world compared to Node.js 12 or above, which run at less than half the current fuzz rate (not to mention the undesirable behaviour when stringifying large arrays).

[alexlamsl]

Do all versions of Node 11.x fail?

I just assume that if first version of Node.js 10 and last version of Node.js 11 both fail, then anything in between is a bit of a lost cause 👻

[kzc]

I suppose. I didn't know what was the last version of 11.x, or at what point they swap in the new V8 engine in release cycle.

There's a lot of fine grained --v8-options you could experiment with if you want to keep the performance of Node 10. It's not necessarily a proposition of disabling the JIT altogether - just a specific feature.

Also, with --allow-natives-syntax you can also instruct V8 to deoptimize specific functions. Conceivably ufuzz and minify could remain jittable and sandbox code could be run deoptimized.

$ echo 'function add(x,y){return x+y} %DeoptimizeFunction(add); console.log(add(1,2));' | node --allow-natives-syntax
3

Other examples of V8 directives: https://github.com/v8/v8/search?q=%25DeoptimizeFunction&type=

[alexlamsl]

Ah the directives are still there after all these years − when Google Chrome first appeared I did study it for a while.