Closed Bug 1125489 Opened 9 years ago Closed 9 years ago

Compiler generates wrong switch statements

Categories

(Firefox Graveyard :: Shumway, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: tschneider, Assigned: yury)

References

Details

Attachments

(1 file)

343 bytes, application/x-applix-spreadsheet
Details
Attached file 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.
Blocks: shumway-jw1
Assignee: nobody → 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.
Flags: needinfo?(mbebenita)
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.
Flags: needinfo?(mbebenita) → needinfo?(shu)
Till says this bug affects the PBS.org video player.
Blocks: shumway-jw2
No longer blocks: shumway-jw1
See https://github.com/mozilla/shumway/pull/2045
Assignee: till → ydelendik
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: needinfo?(shu)
Resolution: --- → FIXED
No longer blocks: shumway-jw2
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: