[Tinycc-devel] Fixed an issue with switch/case statements on unsigned 64 bits

[Willy Tarreau]

Hello,

I managed to build all of haproxy using the latest tcc by applying very
minor cosmetic changes that I'm going to commit. However I faced an issue
that was really a bug. We have a function that's used to count digits for
unsigned long long integers (well, unsigned long on 64-bit). And I was
getting a warning that one statement was empty, and I could verify that
the corresponding code was not matched. I simplified it to this test case:

  #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;
        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;
  }
  
When executed you get this:

  $ 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     <----- note this one
  12345678901234567890 : 20

The reason is a sign change between the bounds in the range below :

    case  1000000000000000000UL ... 9999999999999999999UL: return 19;

  $ printf "%016x\n" 1000000000000000000
  0de0b6b3a7640000
  $ printf "%016x\n" 9999999999999999999
  8ac7230489e7ffff

And there were actually two issues resulting from this:
  - the qsort performed on integer values was no correct since it would
    always be performed on signed integers ;
  - the emitted code would still use signed comparisons (jg/jle) instead
    of unsigned ones (ja/jbe).

So I could fix by taking care of using two distinct case_cmp functions
depending on the sign of the expression, and emitting the warning
appropriately.

While looking for how to submit the fix back, I applied the procedure
described on https://repo.or.cz/w/tinycc.git expecting my patch would
possibly land into a queue or that I would be asked to register or
anything, but I was surprised to see it merged (commit b107f7bdd). I
hope I didn't do any mistake, otherwise please let me know.

Last, I'm not subscribed to the list (too many lists already on my side)
so please keep me in CC for any replies.

Oh by the way, when working on porting haproxy, I discovered a nasty
issue in glibc that #defines __attribute__() to empty when not on GCC.
The nasty effect is that constructors are silently ignored and not called.
I found a workaround that I'm going to apply to haproxy, which consists
in undefining __attribute__() and redefining it to itself so that if it's
redefined we get an error. It solved my problem, but I thought that others
might want to be aware of this.

Thanks!
Willy