From d042e71e9f96cd2635e9d18622c4a2aa007821a4 Mon Sep 17 00:00:00 2001 From: Michael Matz Date: Tue, 18 Oct 2016 03:31:14 +0200 Subject: [PATCH] Fix miscompile with dead switches In certain very specific situations (involving switches with asms inside dead statement expressions) we could generate invalid code (clobbering the buffer so much that we generated invalid instructions). Don't emit the decision table if the switch itself is dead. --- tccgen.c | 16 +++++++++------- tests/tcctest.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 7 deletions(-) diff --git a/tccgen.c b/tccgen.c index b490c04c..2cc57bd4 100644 --- a/tccgen.c +++ b/tccgen.c @@ -5692,13 +5692,15 @@ static void block(int *bsym, int *csym, int is_expr) a = gjmp(a); /* add implicit break */ /* case lookup */ gsym(b); - qsort(sw.p, sw.n, sizeof(void*), case_cmp); - for (b = 1; b < sw.n; b++) - if (sw.p[b - 1]->v2 >= sw.p[b]->v1) - tcc_error("duplicate case value"); - gcase(sw.p, sw.n, c, &a); - if (sw.def_sym) - gjmp_addr(sw.def_sym); + if (!nocode_wanted) { + qsort(sw.p, sw.n, sizeof(void*), case_cmp); + for (b = 1; b < sw.n; b++) + if (sw.p[b - 1]->v2 >= sw.p[b]->v1) + tcc_error("duplicate case value"); + gcase(sw.p, sw.n, c, &a); + if (sw.def_sym) + gjmp_addr(sw.def_sym); + } dynarray_reset(&sw.p, &sw.n); cur_switch = saved; /* break label */ diff --git a/tests/tcctest.c b/tests/tcctest.c index bdbcb602..7b1ac1aa 100644 --- a/tests/tcctest.c +++ b/tests/tcctest.c @@ -2964,6 +2964,56 @@ void test_high_clobbers(void) #endif } +static long cpu_number; +void trace_console(long len, long len2) +{ + /* This generated invalid code when the emission of the switch + table isn't disabled. The asms are necessary to show the bug, + normal statements don't work (they need to generate some code + even under nocode_wanted, which normal statements don't do, + but asms do). Also at least these number of cases is necessary + to generate enough "random" bytes. They ultimately are enough + to create invalid instruction patterns to which the first + skip-to-decision-table jump jumps. If decision table emission + is disabled all of this is no problem. + + It also is necessary that the switches are in a statement expression + (which has the property of not being enterable from outside. no + matter what). */ + if (0 + && + ({ + long pscr_ret__; + switch(len) { + case 4: + { + long pfo_ret__; + switch (len2) { + case 8: printf("bla"); pfo_ret__ = 42; break; + } + pscr_ret__ = pfo_ret__; + } + break; + case 8: + { + long pfo_ret__; + switch (len2) { + case 1:asm("movq %1,%0": "=r" (pfo_ret__) : "m" (cpu_number)); break; + case 2:asm("movq %1,%0": "=r" (pfo_ret__) : "m" (cpu_number)); break; + case 4:asm("movq %1,%0": "=r" (pfo_ret__) : "m" (cpu_number)); break; + case 8:asm("movq %1,%0": "=r" (pfo_ret__) : "m" (cpu_number)); break; + default: printf("impossible\n"); + } + pscr_ret__ = pfo_ret__; + }; + break; + } + pscr_ret__; + })) + { + printf("huh?\n"); + } +} void asm_test(void) { char buf[128]; @@ -3044,6 +3094,7 @@ void asm_test(void) asm volatile ("mov $0x4243, %%esi" : "=r" (regvar)); printf ("regvar=%x\n", regvar); test_high_clobbers(); + trace_console(8, 8); return; label1: goto label2;