[Tinycc-devel] [PATCH] fix potential bug in handling of case_reg

[Edmund Grimley Evans]

The code in tccgen.c for handling a case label looks like this:

        /* since a case is like a label, we must skip it with a jmp */
        b = gjmp(0);
        gsym(*case_sym);
        vseti(case_reg, 0);
        vpushi(v1);
        if (v1 == v2) {
            gen_op(TOK_EQ);
            *case_sym = gtst(1, 0);
        } else {
            gen_op(TOK_GE);
            *case_sym = gtst(1, 0);
            vseti(case_reg, 0);
            vpushi(v2);
            gen_op(TOK_LE);
            *case_sym = gtst(1, *case_sym);
        }
        gsym(b);
        skip(':');

The variable case_reg is passed recursively into "block" but only set
in one place, thus:

        gexpr();
        /* XXX: other types than integer */
        case_reg = gv(RC_INT);
        vpop();

The code therefore makes the assumption that gen_op(comparison) and
gtst won't interfere with the register allocation, because case_reg
isn't even on the value stack when those back-end functions are
called. This assumption may be valid with the current back ends but
even if it is it seems unnecessary and likely to cause problems in
future.

To demonstrate such problems you can insert this code at the start of
gtst in i386-gen.c or x86_64-gen.c (or adapt it for some other back
end):

    {
        int i;
        for (i = 0; i < NB_REGS; i++) {
            int r = get_reg(RC_INT);
#ifndef REG_VALUE
#define REG_VALUE(r) (r)
#endif
            oad(0xb8 + REG_VALUE(r), 0);
        }
    }

I tried this and, sure enough, switch statements then failed:

$ make && ( cd tests/tests2 && make )
...
Test: 00_assignment...
Test: 01_comment...
Test: 02_printf...
Test: 03_struct...
Test: 04_for...
Test: 05_array...
Test: 06_case...
--- 06_case.expect      2015-02-16 08:53:35.517595987 +0000
+++ 06_case.output      2015-02-16 09:16:21.477699795 +0000
...

The attached patch should fix the problem, and I have checked that it
seems to make only a small difference to TCC's output so probably
doesn't significantly affect performance.

Here's my proposed commit message:

Fix handling of case_reg in switch statement.

The back end functions gen_op(comparison) and gtst() might allocate
registers so case_reg should be left on the value stack while they
are called and set again afterwards.


What's the procedure for contributing to TCC? Should I first send a
patch to this list and then push it to "mob" once someone has approved
it?

Edmund

2015-02-16-case_reg.patch
Description: Text Data