Uglifyjs: Bug: mangling and shadowed var in catch clause

[pvdz]

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

• uglify-js version (uglifyjs -V)
  git master aa3f647656ce46469d20bd477b600ca09bc3f964

This one is nasty. So in JS the catch clause is its own scope, but only for the catch-clause-variable (commonly e).

var a = 'FAIL';
try {
    throw 1;
} catch (arguments) {
    var a = 'PASS';
}
console.log(a);

$ bin/uglifyjs s.js -c && bin/uglifyjs s.js -c passes=3 && bin/uglifyjs s.js -c passes=3 -m 
var a="FAIL";try{throw 1}catch(arguments){var a="PASS"}console.log(a);
var a="FAIL";try{throw 1}catch(arguments){var a="PASS"}console.log(a);
var a="FAIL";try{throw 1}catch(a){var a="PASS"}console.log(a);



md5-5e494f97e676c84e678992b6ea4a35fd



$ 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
PASS
PASS
PASS
PASS
FAIL

[kzc]

The test case fails even if arguments is renamed to args:

$ cat catch.js
var a = 'FAIL';
try {
    throw 1;
} catch (args) {
    var a = 'PASS';
}
console.log(a);

$ node catch.js
PASS



md5-5e494f97e676c84e678992b6ea4a35fd



$ bin/uglifyjs catch.js -m | node
FAIL



md5-5e494f97e676c84e678992b6ea4a35fd



$ bin/uglifyjs catch.js -m 
var a="FAIL";try{throw 1}catch(a){var a="PASS"}console.log(a);

FWIW, this bug also happens in [email protected].

[alexlamsl]

And here I thought screw_ie8 was bad enough... :see_no_evil:

[kzc]

Irony of ironies...

$ bin/uglifyjs catch.js --screw-ie8 -m | node
FAIL

$ bin/uglifyjs catch.js --support-ie8 -m | node
PASS



md5-5e494f97e676c84e678992b6ea4a35fd



$ bin/uglifyjs catch.js --support-ie8 -m
var a="FAIL";try{throw 1}catch(args){var a="PASS"}console.log(a);

Again, same behavior with current master and [email protected].

[pvdz]

Perhaps related?

var a = 'FAIL';
try {
    throw 1
} catch (e) {
    var a = 'PASS';
}
console.log(a);

Otherwise I'll file a new bug.

(Also with mangling only)

[kzc]

It's the same bug. The catch variable name can be anything.

[alexlamsl]

Thanks for the detailed analysis. I think I know how to fix this, plus expect_stdout is going to save a lot of guesswork.

[kzc]

@alexlamsl Be sure to add this different mangle test case for both screw_ie8 false and true:

var a = 'FAIL';
try {
    throw 1;
} catch (args) {
    a = 'PASS';
}
console.log(a);

which currently passes with mangle in both screw_ie8=true and screw_ie8=false modes.

[alexlamsl]

I wouldn't envy the guy who's going to merge this fix onto harmony... oh wait, that's me.

[pvdz]

Wouldn't envy the guys fixing all the es6 stuff I'll find next. Oh wait... ;)

[kzc]

@qfox No one's really maintaining harmony/ES6 at the moment. So feel free to queue up the bugs, just don't expect a lot of action on them. Block scoping doesn't work too well with mangle and most compress transforms. Finding bugs would be pretty easy. Uglify/harmony users would be well advised to disable compress and mangle altogether.

[pvdz]

Nah. If nobody's plumbing that then a: the fuzzer will trip up over the same bug over and over again, and b: it'll report many bugs for work in progress code. I'll fuzz it when there's a stable release.

[pvdz]

This didn't resolve the issue. Example:

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

$ 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
PASS
PASS
PASS
PASS
FAIL2

Open a new ticket or reopen this one?

[pvdz]

Actually, it may have resolved _an_ issue, but this example test seems very similar. In this case the catch var name must be the same. This was tested on 984a21704e126616a74d65a1e8790aeccd02f548

[alexlamsl]

Yeah, this is a similar but different issue.

And to think that ES6 isn't getting any better... :sweat_smile:

[alexlamsl]

Don't worry about opening another issue - I'll just open a PR directly to fix this.