Re: [Tinycc-devel] Bug: array references with long long ints on i386

[Thomas Preud'homme]

Le mercredi 11 juillet 2012 19:45:23, Thomas Preud'homme a écrit :
> Le samedi 9 juin 2012 15:57:03, address@hidden a écrit :
> > Hi list,
> > 
> > Here is a simple C program that does not compile and execute correclty
> > with TCC (on i386, both with TCC 0.9.25 and with the latest sources; it
> > works on AMD64):
> > 
> > #include <stdio.h>
> > 
> > long long int ll[] = { 1LL, 2LL };
> > unsigned long long int ull[] = { 1ULL, 2ULL };
> > 
> > int main ()
> > {
> > 
> >    long long int lli;
> >    unsigned long long int ulli;
> >    lli = 1LL;
> >    printf ("%lld ", ll[lli]); printf ("%lld ", ll[1LL]); printf
> >    ("%lld\n",
> > 
> > ll[1]); printf ("%lld %lld %lld\n", ll[lli], ll[1LL], ll[1]);
> > 
> >    if (ll[lli] == 2LL) printf ("OK "); else printf ("KO ");
> >    if (ll[1LL] == 2LL) printf ("OK "); else printf ("KO ");
> >    if (ll[1] == 2LL) printf ("OK\n"); else printf ("KO\n");
> >    ulli = 1ULL;
> >    printf ("%llu ", ull[ulli]); printf ("%llu ", ull[1ULL]); printf
> > 
> > ("%llu\n", ull[1]); printf ("%llu %llu %llu\n", ull[ulli], ull[1ULL],
> > ull[1]);
> > 
> >    if (ull[ulli] == 2ULL) printf ("OK "); else printf ("KO ");
> >    if (ull[1ULL] == 2ULL) printf ("OK "); else printf ("KO ");
> >    if (ull[1] == 2ULL) printf ("OK\n"); else printf ("KO\n");
> >    return 0;
> > 
> > }
> > 
> > It should obviously print:
> > 
> > 2 2 2
> > 2 2 2
> > OK OK OK
> > 2 2 2
> > 2 2 2
> > OK OK OK
> > 
> > but it prints:
> > 
> > 0 2 2
> > 2 2 2
> > KO OK OK
> > 0 2 2
> > 2 2 2
> > KO OK OK
> > 
> > --ghe
> 
> Damn, there is at least 2 bugs there :(
> 
> I fixed a first one with the simple following test case:
> 
> long long int ll[] = { 1LL, 2LL };
> 
> int main ()
> {
>    long long int lli;
>    lli = 1LL;
>    return ll[lli];
> }
> 
> What happen is that in the gv(int rc) function, just after the comment
> /* allocate second register */ the get_reg can causes the value loaded
> previously into r to be saved onto the stack. The bug happen because at the
> end of the function, just before the #ifdef TCC_TARGET_C67 vtop->r is set
> again to r whereas the value is no longer in r.
> 
> Attached is a patch. It's not the best solution, but that's the first I
> found. Improvement welcome.
> 
> Despite this, the following testcase still fails:
> 
> #include <stdio.h>
> 
> long long int ll[] = { 1LL, 2LL };
> 
> int main ()
> {
>    long long int lli;
>    lli = 1LL;
>    printf ("%lld %lld\n", ll[lli], ll[1LL]);
>    printf ("%lld %lld\n", ll[lli], ll[lli]);
>    printf ("%lld\n", ll[lli]);
>    return 0;
> }
> 
> I probably won't have time to look into it before quite some time so help
> welcome to find the bug.

Ok, I found it.

The source is the same in fact. I just didn't understand what was wrong in gv 
previously. If a value needs 2 registers (like a long long on i386) after gv 
has been called the value must be found in the registers. So here what happen 
is that for the same reason as before part of the long long is saved on stack 
after gv returns. This times it blows up in gfunc_call which call gv(RC_INT) 
and then if it's a long it saves vtop->r2 on stack and later on saves vtop->r 
on stack. I did a hacky patch to test:

diff --git a/i386-gen.c b/i386-gen.c
index 6635559..3845b0b 100644
--- a/i386-gen.c
+++ b/i386-gen.c
@@ -359,6 +359,7 @@ static void gcall_or_jmp(int is_jmp)
 static uint8_t fastcall_regs[3] = { TREG_EAX, TREG_EDX, TREG_ECX };
 static uint8_t fastcallw_regs[2] = { TREG_ECX, TREG_EDX };
 
+void lexpand(void);
 /* Generate function call. The function address is pushed first, then
    all the parameters in call order. This functions pops all the
    parameters and the function address. */
@@ -402,11 +403,16 @@ ST_FUNC void gfunc_call(int nb_args)
         } else {
             /* simple type (currently always same size) */
             /* XXX: implicit cast ? */
-            r = gv(RC_INT);
             if ((vtop->type.t & VT_BTYPE) == VT_LLONG) {
+                lexpand();
+                gv(RC_INT);
                 size = 8;
-                o(0x50 + vtop->r2); /* push r */
+                o(0x50 + vtop->r); /* push r */
+                vtop--;
+                gv(RC_INT);
+                r = vtop->r;
             } else {
+                r = gv(RC_INT);
                 size = 4;
             }
             o(0x50 + r); /* push r */

If you don't hear from me in the next days consider I didn't have time to 
propose a fix and try it yourself. The relevant part is the get_reg in gv() 
after the comment /* allocate second register */ which can take the same 
register as the previous one did.

> 
> Best regards,
> 
> Thomas Preud'homme
> 

ditto

signature.asc
Description: This is a digitally signed message part.