Uglifyjs: Bug: fallthrough should not execute case expression

[pvdz]

• Bug report or feature request?
  Bug. Found by fuzzer. Might be dupe of #1679

• uglify-js version (uglifyjs -V)
  uglify-js 2.8.16 (git master, post #1677)

I _think_ the return in the second clause of the useless switch is causing dead code elimination for the third clause which actually decrements the variable. Or maybe it's something else and I have no idea :)

var a = 100, b = 10;
function f254() {
  switch (666) {
    default:
    case 0:
      break;
    case (b >>>= a):
      return
    case --b:
      break;
  }
}
f254();

console.log(a, b);

$ bin/uglifyjs s.js -c
function f254(){switch(666){default:case 0:case--b:break;case b>>>=a:return}}var a=100,b=10;f254(),console.log(a,b);

$ node s.js && bin/uglifyjs s.js -c | node
100 -1
100 0

[pvdz]

I think this is the same one:

switch (1) {
  case 1:
  case console.log('FAIL'):
}

(edited) That should make it quite apparent what's going on here :)

[alexlamsl]

https://github.com/mishoo/UglifyJS2/issues/1680#issue-217007401 is the same issue as #1679

[alexlamsl]

https://github.com/mishoo/UglifyJS2/issues/1680#issuecomment-289242263 is a different bug.

[kzc]

This would make a great interview question - what does the following output?

$ echo 'function f(x){console.log(x)} switch(1){case f(0): case 1: case f(2): case f(3): f(4);}' | node
0
4

No one would get the job.

Latest master:

$ echo 'function f(x){console.log(x)} switch(1){case f(0): case 1: case f(2): case f(3): f(4);}' | bin/uglifyjs -c | node
0
2
3
4

$ echo 'function f(x){console.log(x)} switch(1){case f(0): case 1: case f(2): case f(3): f(4);}' | bin/uglifyjs -c
function f(x){console.log(x)}switch(1){case f(0):case 1:f(2);f(3);f(4)}

[kzc]

@alexlamsl It appears that case conditions are executed only up to the point of the matching case condition. Could you add the test case above in your eventual fix?

[alexlamsl]

@kzc sure thing. And thanks for making me lose that hypothetical job :wink:

With the current ~hype~ fascination on AI, may be we can automate developer job interviews using uglify-js :ghost:

[kzc]

The switch logic makes sense from the point of view of a first generation JS interpreter. Execute each case condition one at a time from the beginning until one matches. Then only execute non-case statements until break is hit, or the end of the switch is reached.

But who knows - I could be wrong. Reading the ECMAScript spec is probably best.

[alexlamsl]

Makes perfect sense, yeah - and I was banking on test/ufuzz.js with the OPT(AST_Switch) rewrite to catch all these corner cases.

BTW, I know how to fix this already. I'm chatting around because I don't wanna risk coding on 5" touchscreen over hot pot. Will get on this in about an hour.

[kzc]

Remove this line and commit with your PR:

https://github.com/mishoo/UglifyJS2/blob/94f84727ce454c3ecf5206ac79dba4a21ec6deb6/test/ufuzz.js#L204

[pvdz]

Reading the ECMAScript spec is probably best.

Well that's what I've done which is why I knew default is a hot pocket for bugs.

In JS a switch will first traverse all cases until one matches. Once one does, it (obviously?) skips case expression evaluation but acts like you would expect (execute all following case bodies until one breaks/etc).

When no case matches, the switch will go back and find a default case. If one is present, regardless of its clause index, it will execute the body and that of any case that follows it until it breaks/returns/etc.

I don't think I've ever seen anyone use this feature in the wild, but there it is :)