1125489 - Compiler generates wrong switch statements

Status: RESOLVED FIXED

(Firefox Graveyard :: Shumway, defect)

Description

[Tobias Schneider [:tobytailor]]

Attachment: test case

In the attached test case the following function:

function map(type) {
    switch (type) {
        case 'flv':
            return 'flv';
        case 'mp4':
            return 'mp4';
        case 'hls':
            if (hasHls) {
                return 'hls';
            }
            break;
        case 'rmtp':
            return 'rmtp';
    }
    return null;
}

gets compiled to:

function public$$Bgmap(A) {
    var v5, v50, $L, v105, v87;
    v5 = A;
    if ("flv" !== v5) {
        {
            if ("mp4" !== v5) {
                {
                    if ("hls" !== v5) {
                        {
                            if ("rmtp" !== v5) {
                                {
                                    v50 = 4;
                                    $L = 2;
                                }
                            } else {
                                {
                                    v50 = 3;
                                    $L = 2;
                                }
                            }
                            $L = 2;
                        }
                    } else {
                        {
                            v50 = 2;
                            $L = 2;
                        }
                    }
                    $L = 2;
                }
            } else {
                {
                    v50 = 1;
                    $L = 2;
                }
            }
            $L = 2;
        }
    } else {
        {
            v50 = 0;
            $L = 2;
        }
    }
    switch (v50) {
        case 0:
            {
                v105 = "flv";
                $L = 0;
                break;
            };
        case 1:
            {
                v105 = "mp4";
                $L = 0;
                break;
            };
        case 2:
            {
                v87 = this.$BghasHls;
                if (!v87) {
                    {
                        $L = 18;
                    }
                } else {
                    {
                        v105 = "hls";
                        $L = 0;
                    }
                }
            };
        case 3:
            {
                v105 = "rmtp";
                $L = 0;
                break;
            };
        case 4:
            {
                $L = 20;
                break;
            };
        default:
            {
                $L = 20;
                break;
            }
    };
    if ($L === 20) {
        {
            $L = 18;
        }
    }
    if ($L === 18) {
        {
            v105 = null;
            $L = 0;
        }
    }
    return v105;
}

which, when passing in "hls", should set the return value to "hls" and break out of the switch statement. Instead it falls through and sets the return value to "rtmp" in the following case block.

When running the test case in interpreter mode, the result is as expected.

Comment 1

[Till Schneidereit [:till]]

The bytecode Flash's compiler creates for switch statements is beyond horrible, really.

That does not, however, excuse the fact that we somehow forget to emit the return and break statements for the "hls" case.

Here's a reduced test case:

function map(test) {
	var foo: Boolean = true;
    switch (test) {
        case 1:
            if (foo) {
                return 1;
            }
            break;
        case 2:
            return 2;
    }
    return 3;
}

trace(map(1));

Note that changing the `break` to a `return` makes the issue go away, as does changing the `return 1` to a `break`. I'll take a look in a bit, but needinfo'ing mbx for now as he's far better equipped for debugging this than I am.

Comment 2

[Michael Bebenita [:mbx]]

This is a problem in the relooper algorithm. Needing info from shu since he knows more about it. I'll provide him with a reduced test case / debug build.

On another note, if this is ends up too hard to fix we may want to look into using Emscripten's relooper since it is tested much more thoroughly.

Comment 3

[Chris Peterson [:cpeterson]]

Till says this bug affects the PBS.org video player.

Comment 4

[Yury Delendik (:yury)]

See https://github.com/mozilla/shumway/pull/2045