tinycc-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Christian JULLIEN
Subject: Re: [Tinycc-devel] Fixed an issue with switch/case statements on unsigned 64 bits
Date: Tue, 18 Aug 2020 18:45:29 +0200 (CEST)

Recent change has an issue with some tests


macOS x64 and Linux ARM

------------ test3 ------------

tcctest.c:1939: error: duplicate case value

make[2]: *** [test3] Error 1

------------ memtest ------------

../tests/tcctest.c:1939: error: duplicate case value

make[2]: *** [memtest] Error 1


Linux Aarch64:

../arm64-gen.c:1382: error: duplicate case value

make[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 value

make[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

_______________________________________________
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

reply via email to

[Prev in Thread] Current Thread [Next in Thread]