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

[Alexander Strasser]

Hi Christian,

did you put Willy in Cc?

If yes, sorry for the noise.


Best regards,
  Alexander


On 2020-08-18 18:45 +0200, Christian JULLIEN wrote:
> Recent change has an issue with some tests
>
> macOS x64 and Linux ARM------------ test3 ------------tcctest.c:1939: error: 
> duplicate case valuemake[2]: *** [test3] Error 1------------ memtest 
> ------------../tests/tcctest.c:1939: error: duplicate case valuemake[2]: *** 
> [memtest] Error 1
> Linux Aarch64:../arm64-gen.c:1382: error: duplicate case valuemake[2]: *** 
> [Makefile:107: libtest_mt] Error 1------------ test3 ------------In file 
> included from ../tcc.c:23:In file included from 
> ../libtcc.c:35:../arm64-gen.c:1382: error: duplicate case valuemake[2]: *** 
> [Makefile:139: test3] Error 1------------ memtest ------------
>
>
>  Le : 18 août 2020 à 15:40 (GMT +02:00)
> De : "Willy Tarreau" <w@1wt.eu>
> À : "tinycc-devel@nongnu.org" <tinycc-devel@nongnu.org>
> Objet : [Tinycc-devel] Fixed an issue with switch/case statements on
>  unsigned 64 bits
>
>
> 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