Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

-f with -b/-g generates incorrect dispatch on fill labels #119

Closed
skvadrik opened this issue Aug 25, 2015 · 1 comment

Comments

@skvadrik
Copy link
Owner

commented Aug 25, 2015

Consider the following example 1.re:

/*!re2c
    "" {}
*/

With -if, re2c would generate correct dispatch:

$ re2c -if 1.re
/* Generated by re2c 0.14.3 on Tue Aug 25 10:41:45 2015 */


        switch (YYGETSTATE()) {
        default: goto yy0;
        case 0: goto yyFillLabel0;
        }
yy0:
        YYSETSTATE(0);
yyFillLabel0:
        {}

With -bif all positive YYGETSTATE() values will lead to yyFillLabel0, which is clearly an error: values that are greater than 0 should lead to yy0:

$ re2c -bif 1.re
/* Generated by re2c 0.14.3 on Tue Aug 25 10:40:32 2015 */


        if (YYGETSTATE() < 0) {
                goto yy0;
        } else {
                goto yyFillLabel0;
        }
yy0:
        YYSETSTATE(0);
yyFillLabel0:
        {}

With -gif the error is different: all values greater than 0 now cause undefined behaviour (access to memory not within array bounds):

$ re2c -gif 1.re
/* Generated by re2c 0.14.3 on Tue Aug 25 10:47:41 2015 */

{

        static void *yystable[] = {
                &&yyFillLabel0,
        };

        if (YYGETSTATE() < 0) {
                goto yy0;
        }
        goto *yystable[YYGETSTATE()];
yy0:
        YYSETSTATE(0);
yyFillLabel0:
        {}
}

The situation gets even more tricky if we add re2c:state:abort configuration (2.re):

/*!re2c
    re2c:state:abort = 1;
    "" {}
*/

Correct dispatch with -if looks like this:

$ re2c -if 2.re
/* Generated by re2c 0.14.3 on Tue Aug 25 10:51:35 2015 */


        switch (YYGETSTATE()) {
        default: abort();
        case -1: goto yy0;
        case 0: goto yyFillLabel0;
        }
yy0:
        YYSETSTATE(0);
yyFillLabel0:
        {}

With -bif call to abort() is unreachable:

$ re2c -bif 2.re
/* Generated by re2c 0.14.3 on Tue Aug 25 10:54:00 2015 */


        if (YYGETSTATE() < 0) {
                goto yy0;
        } else {
                goto yyFillLabel0;
        }
        abort();
yy0:
        YYSETSTATE(0);
yyFillLabel0:
        {}

With -gif we still get undefined behaviour for all values greater than 0:

$ re2c -gif 2.re
/* Generated by re2c 0.14.3 on Tue Aug 25 10:54:38 2015 */

{

        static void *yystable[] = {
                &&yyFillLabel0,
        };

        if (YYGETSTATE() == -1) {
                goto yy0;
        } else if (YYGETSTATE() < -1) {
                abort();
        }
        goto *yystable[YYGETSTATE()];
yy0:
        YYSETSTATE(0);
yyFillLabel0:
        {}
}

Besides, YYGETSTATE() macro is called multiple times with -b and -g, which is not so good. Additional checks with -g don't help either.

@skvadrik skvadrik self-assigned this Aug 25, 2015

skvadrik added a commit that referenced this issue Aug 25, 2015

Fixed bug #119: "-f with -b/-g generates incorrect dispatch on fill l…
…abels".

Consider the following example 1.re:
    /*!re2c
        "" {}
    */
With -if, re2c would generate correct dispatch:
    $ re2c -if 1.re
    /* Generated by re2c 0.14.3 on Tue Aug 25 10:41:45 2015 */
            switch (YYGETSTATE()) {
            default: goto yy0;
            case 0: goto yyFillLabel0;
            }
    yy0:
            YYSETSTATE(0);
    yyFillLabel0:
            {}

With -bif all positive YYGETSTATE() values will lead to yyFillLabel0,
which is clearly an error: values that are greater than 0 should lead
to yy0:
    $ re2c -bif 1.re
    /* Generated by re2c 0.14.3 on Tue Aug 25 10:40:32 2015 */
            if (YYGETSTATE() < 0) {
                    goto yy0;
            } else {
                    goto yyFillLabel0;
            }
    yy0:
            YYSETSTATE(0);
    yyFillLabel0:
            {}

With -gif the error is different: all values greater than 0 now cause
undefined behaviour (access to memory not within array bounds):
    $ re2c -gif 1.re
    /* Generated by re2c 0.14.3 on Tue Aug 25 10:47:41 2015 */
    {
            static void *yystable[] = {
                    &&yyFillLabel0,
            };
            if (YYGETSTATE() < 0) {
                    goto yy0;
            }
            goto *yystable[YYGETSTATE()];
    yy0:
            YYSETSTATE(0);
    yyFillLabel0:
            {}
    }

The situation gets even more tricky with re2c:state:abort configuration.
Besides, YYGETSTATE() macro is called multiple times with -b and -g,
which is not so good. Additional checks with -g don't help gain performance
either.

Fix: always generate simple switch.
@skvadrik

This comment has been minimized.

Copy link
Owner Author

commented Aug 25, 2015

Fixed, see this commit. Now re2c with -f always generates simple switch.

@skvadrik skvadrik closed this Aug 25, 2015

skvadrik added a commit that referenced this issue Oct 16, 2015

Fixed tests for bug #119: "-f with -b/-g generates incorrect dispatch…
… on fill labels".

Somehow configuration 're2c:state:abort = 1;' was present in all the
tests; it was meant to be only in half of them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.