1
0
Fork 0

Fix switch/case on uint64_t

The switch/case operation was entirely performed on int64_t, resulting
in a warning and bad code to be emitted on 64 bit machines when used on
an unsigned long with a case range whose signed representation starts
positive and ends negative like in the example below:

  #include <limits.h>
  #include <stdio.h>
  #include <stdlib.h>

  int nbdg(unsigned long n)
  {
  	switch (n) {
  	case                    1UL ...                   9UL: return 1;
  	case                   10UL ...                  99UL: return 2;
  	case                  100UL ...                 999UL: return 3;
  	case                 1000UL ...                9999UL: return 4;
  	case                10000UL ...               99999UL: return 5;
  	case               100000UL ...              999999UL: return 6;
  	case              1000000UL ...             9999999UL: return 7;
  	case             10000000UL ...            99999999UL: return 8;
  	case            100000000UL ...           999999999UL: return 9;
  	case           1000000000UL ...          9999999999UL: return 10;
  	case          10000000000UL ...         99999999999UL: return 11;
  	case         100000000000UL ...        999999999999UL: return 12;
  	case        1000000000000UL ...       9999999999999UL: return 13;
  	case       10000000000000UL ...      99999999999999UL: return 14;
  	case      100000000000000UL ...     999999999999999UL: return 15;
  	case     1000000000000000UL ...    9999999999999999UL: return 16;
  	case    10000000000000000UL ...   99999999999999999UL: return 17;
  	case   100000000000000000UL ...  999999999999999999UL: return 18;
  	case  1000000000000000000UL ... 9999999999999999999UL: return 19; // this one
  	case 10000000000000000000UL ...             ULONG_MAX: return 20;
  	}
  	return 0;
  }

  int main(int argc, char **argv)
  {
  	unsigned long v = strtoul(argc > 1 ? argv[1] : "1111", NULL, 0);
  	printf("%lu : %d\n", v, nbdg(v));
  	return 0;
  }

  $ tcc dg.c
  dg.c:26: warning: empty case range
  $ x="";for i in 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0; do x=$x$i; ./a.out $x;done
  1 : 1
  12 : 2
  123 : 3
  1234 : 4
  12345 : 5
  123456 : 6
  1234567 : 7
  12345678 : 8
  123456789 : 9
  1234567890 : 10
  12345678901 : 11
  123456789012 : 12
  1234567890123 : 13
  12345678901234 : 14
  123456789012345 : 15
  1234567890123456 : 16
  12345678901234567 : 17
  123456789012345678 : 18
  1234567890123456789 : 0
  12345678901234567890 : 20

What this patch does is to use a separate set of signed and unsigned
case_cmp functions depending on whether the expression is signed or
unsigned, and also does this to decide when to emit the warning.

The bad code on output was caused by the removal of the unsigned bit
resulting from the signed sort, which causes only signed comparisons
to be emitted in the asm code. As such some sets could not match.

Note that there is no way to rely on the values only to sort properly
nor to emit the warning because we're effectively dealing with 65-bit
arithmetic here and any two values will have a different behavior
depending on the signed or unsigned expectation.

For unsigned expressions now the warning only happens when bounds are
switched, For signed expressions (e.g. if the input is signed long
above), the warning remains and the abnormal output as well. In both
cases this remains consistent with what gcc produces.
This commit is contained in:
Willy Tarreau 2020-08-18 11:08:44 +02:00
parent 3613a11454
commit b107f7bdd9
1 changed files with 15 additions and 7 deletions

View File

@ -6706,13 +6706,20 @@ static void check_func_return(void)
/* ------------------------------------------------------------------------- */
/* switch/case */
static int case_cmp(const void *pa, const void *pb)
static int case_cmpi(const void *pa, const void *pb)
{
int64_t a = (*(struct case_t**) pa)->v1;
int64_t b = (*(struct case_t**) pb)->v1;
return a < b ? -1 : a > b;
}
static int case_cmpu(const void *pa, const void *pb)
{
uint64_t a = (uint64_t)(*(struct case_t**) pa)->v1;
uint64_t b = (uint64_t)(*(struct case_t**) pb)->v1;
return a < b ? -1 : a > b;
}
static void gtst_addr(int t, int a)
{
gsym_addr(gvtst(0, t), a);
@ -7118,16 +7125,16 @@ again:
/* case lookup */
gsym(b);
qsort(sw->p, sw->n, sizeof(void*), case_cmp);
if (sw->sv.type.t & VT_UNSIGNED)
qsort(sw->p, sw->n, sizeof(void*), case_cmpu);
else
qsort(sw->p, sw->n, sizeof(void*), case_cmpi);
for (b = 1; b < sw->n; b++)
if (sw->p[b - 1]->v2 >= sw->p[b]->v1)
tcc_error("duplicate case value");
/* Our switch table sorting is signed, so the compared
value needs to be as well when it's 64bit. */
vpushv(&sw->sv);
if ((vtop->type.t & VT_BTYPE) == VT_LLONG)
vtop->type.t &= ~VT_UNSIGNED;
gv(RC_INT);
d = 0, gcase(sw->p, sw->n, &d);
vpop();
@ -7150,7 +7157,8 @@ again:
if (gnu_ext && tok == TOK_DOTS) {
next();
cr->v2 = expr_const64();
if (cr->v2 < cr->v1)
if ((!(cur_switch->sv.type.t & VT_UNSIGNED) && cr->v2 < cr->v1)
|| (cur_switch->sv.type.t & VT_UNSIGNED && (uint64_t)cr->v2 < (uint64_t)cr->v1))
tcc_warning("empty case range");
}
cr->sym = gind();