Uglifyjs: Bug: var within catch with the same name means madness

[pvdz]

• Bug report or feature request?
  Bug. Found by fuzzer.

• uglify-js version (uglifyjs -V)
  Git master

I don't know what's going on. This is as low as I can go.

var b = 10, c = 0;
function f() {
    try {
        throw c + 1 + ~b
    } catch (b) {
        var b = --b + b;
        ++b && ++c
    }
}
f();
console.log(b, c);

$ bin/uglifyjs s.js -c && bin/uglifyjs s.js -c passes=3 && bin/uglifyjs s.js -c passes=3 -m 
function f(){try{throw c+1+~b}catch(b){var b=--b+b;++b&&++c}}var b=10,c=0;f(),console.log(b,c);
function f(){try{throw c+1+~b}catch(b){var b=--b+b;++b&&++c}}var b=10,c=0;f(),console.log(b,c);
function f(){try{throw c+1+~o}catch(r){var o=--r+r;++r&&++c}}var b=10,c=0;f(),console.log(b,c);



md5-3bf1193c6a8d0368e0fe3c5c11b7d446



$ cat s.js | node && node s.js && bin/uglifyjs s.js -c | node && bin/uglifyjs s.js -c passes=3 | node && bin/uglifyjs s.js -c passes=3 -m | node
10 1
10 1
10 1
10 1
10 0

[alexlamsl]

Almost tempted to submit an issue report to reality itself :rofl:

[pvdz]

Note that https://github.com/mishoo/UglifyJS2/issues/1704#issuecomment-289565574 was derived from this case. But when removing the function scope a different pattern emerged so I think those are two different ones.

[pvdz]

And this one is a sibling, note that the output is unrelated to mangling. But perhaps still the same origin.

The splitting difference was when throw had 1 instead of the var references above. (The difference being that it was no longer a mangling problem only)

var a = 100, b = 10, c = 0;
function f() {
    try {
        throw 1; // different bug
    } catch (b) {
        var b = (--b) + (1 === 1 ? a : b), c_56 = (a++) + (delete b);
        for (var i = 5; ++b && i > 0; --i){
            ++a && --b && ++c
        }
    }
}
f();
console.log(null, a, b, c);

$ bin/uglifyjs s.js -c && bin/uglifyjs s.js -c passes=3 && bin/uglifyjs s.js -c passes=3 -m 
WARN: Condition always true [s.js:27,24]
WARN: Side effects in initialization of unused variable b [s.js:27,12]
WARN: Side effects in initialization of unused variable c_56 [s.js:27,43]
function f(){try{throw 1}catch(b){for(var i=(--b,a++,delete b,5);++b&&i>0;--i)++a&&--b&&++c}}var a=100,b=10,c=0;f(),console.log(null,a,b,c);
WARN: Condition always true [s.js:27,24]
WARN: Side effects in initialization of unused variable b [s.js:27,12]
WARN: Side effects in initialization of unused variable c_56 [s.js:27,43]
function f(){try{throw 1}catch(b){for(var i=(--b,a++,delete b,5);++b&&i>0;--i)++a&&--b&&++c}}var a=100,b=10,c=0;f(),console.log(null,a,b,c);
WARN: Condition always true [s.js:27,24]
WARN: Side effects in initialization of unused variable b [s.js:27,12]
WARN: Side effects in initialization of unused variable c_56 [s.js:27,43]
function f(){try{throw 1}catch(l){for(var o=(--l,a++,delete l,5);++l&&o>0;--o)++a&&--l&&++c}}var a=100,b=10,c=0;f(),console.log(null,a,b,c);



md5-3bf1193c6a8d0368e0fe3c5c11b7d446



$ cat s.js | node && node s.js && bin/uglifyjs s.js -c | node && bin/uglifyjs s.js -c passes=3 | node && bin/uglifyjs s.js -c passes=3 -m | node
null 106 10 5
null 106 10 5
WARN: Condition always true [s.js:27,24]
WARN: Side effects in initialization of unused variable b [s.js:27,12]
WARN: Side effects in initialization of unused variable c_56 [s.js:27,43]
null 106 10 5
WARN: Condition always true [s.js:27,24]
WARN: Side effects in initialization of unused variable b [s.js:27,12]
WARN: Side effects in initialization of unused variable c_56 [s.js:27,43]
null 106 10 5
WARN: Condition always true [s.js:27,24]
WARN: Side effects in initialization of unused variable b [s.js:27,12]
WARN: Side effects in initialization of unused variable c_56 [s.js:27,43]
null 106 10 5

[alexlamsl]

There is also this:

try {
    throw 'FAIL1';
} catch (a) {
    var a = 'FAIL2';
}
console.log(a);

Note the absence of top-level declaration of a. Yet, instead of ReferenceError: 'a' is undefined, this prints undefined. This will get the ReferenceError as expected:

try {
    throw 'FAIL1';
} catch (a) {
}
console.log(a);

[pvdz]

That's because the catch var name gets its own unique scope. However, the var inside the catch clause is going to the upper scope, global in this case. This makes perfect sense what are you talking about ;)

[pvdz]

(Also, the initializer inside the catch clause actually updates the inner most `a, since the declaratio and initialization are basically two different timestamps. But you probably figured that out after my previous comment.)

[pvdz]

Ah and keep in mind that in the ie8 era, pretty much all browsers got this different. The catch var scope was one you couldn't rely on at all. Nowadays I think it's alright since browsers put in more effort be spec compliant. Edge case and all.

[alexlamsl]

I'm aware of the need to test IE8 - there is this screw_ie8 flag that we need to support, after all :wink:

[pvdz]

Yeah that's why I'm mentioning it :)

[alexlamsl]

Right, I think it's best for me to sit back and work out the best representation within the AST before I patch this up.

And 4:30am is probably not the best time to do that. :zzz:

[alexlamsl]

Another great interview question - what is the output from the following?

var a = 1;
!function(){
  try{
    throw 2;
  } catch (a) {
    var a = 3;
  }
  a = 4;
}();
console.log(a);

[pvdz]

Typo? This should be 1 because the function doesn't get invoked.

[alexlamsl]

Fixed typo - Github & touchscreen don't get along well.

[pvdz]

I'm sure 5am helps though! ;) (Your trick did not fool me, but it's a solid question for sure.)

[kzc]

@alexlamsl jxcore/Spidermonkey gets a different result in piped mode only:

$ cat interview.js 
var a = 1;
!function(){
  try{
    throw 2;
  } catch (a) {
    var a = 3;
  }
  a = 4;
}();
console.log(a);

$ cat interview.js | node690
1

$ cat interview.js | node421
1

$ cat interview.js | jx
4

$ node690 interview.js 
1

$ node421 interview.js 
1

$ jx interview.js 
1

I wonder what FF 52.0.1 produces... checking... it produces 1.

[alexlamsl]

I guess we'll go with the populate vote then. 😅

[kzc]

The var in the catch block gives a function scope, which explains the 1 result.

[pvdz]

Similar case;

var b = 'PASS';
try {
    throw 1
} catch (b) {
    var b = 'FAIL2', b = 'FAIL1';
}
console.log(b);

I'm not sure if the second var of the same name causes a problem here. May just be an irrelevant case.

[alexlamsl]

@qfox https://github.com/mishoo/UglifyJS2/issues/1708#issuecomment-289691495 works with #1711