[Tinycc-devel] save_reg_upstack miscompile

[Michael Matz]

Hi grischka,

commit b69158578 "tccgen: arm/i386: save_reg_upstack" miscompiles tccboot.
It ultimately boils down to this routine:

% cat bug.c
#define APIC_DIVISOR 16
#define HZ 1000
extern long getl();
extern unsigned long long getull();
extern void printk(const char *, ...);
int calibrate_APIC_clock(int cond)
{
        unsigned long long t1 = 0, t2 = 0;
        long tt1, tt2;
        long result;
        int i;
        const int LOOPS = HZ/10;

        tt1 = getl();
        tt2 = getl();
        t1 = getull();
        t2 = getull();

        result = (tt1-tt2)*APIC_DIVISOR/LOOPS;
        if (cond)
                printk("..... CPU clock speed is %ld.%04ld MHz.\n",
                        ((long)(t2-t1)/LOOPS)/(1000000/HZ),
                        ((long)(t2-t1)/LOOPS)%(1000000/HZ));

        printk("..... host bus clock speed is %ld.%04ld MHz.\n",
                result/(1000000/HZ),
                result%(1000000/HZ));

        return result;
}

Compiling this with i386-tcc will amongst other things result in this 
assembly:

  a1:   f7 f9                   idiv   %ecx
...
  c6:   8b 55 c4                mov    -0x3c(%ebp),%edx
  c9:   19 52 0a                sbb    %edx,0xa(%edx)
  cc:   89 45 bc                mov    %eax,-0x44(%ebp)

Note the store into 0xa(%edx), which is invalid in TCCs setting.  That is 
the result of some invalid "registers" on the vstack, namely 0x132 (i.e. 
LVAL|LOCAL), remaining after gv2(RC_INT,RC_INT), so that the instruction 
output machinery generates wrong instructions.

The below patch adds some quick checking for gv2, and triggers after your 
save_reg_upstack change:

@@ -969,6 +969,8 @@ ST_FUNC void gv2(int rc1, int rc2)
             gv(rc2);
         }
     }
+    if (vtop[0].r >= VT_CONST || vtop[-1].r >= VT_CONST)
+      tcc_error("internal error");
 }

 #ifndef TCC_TARGET_ARM64


I haven't traced it to the root cause yet.  It's either something to do 
with leaving r2 set to some register (not resetting to VT_CONST) even 
after changing the type away from LLONG (so that later save_regs are 
confused) or something else.

Just reversing the '#if 0' in gv() guarding the save_reg_upstack call 
makes the above check not trigger and the resulting tccboot kernel 
working.

Sorry, need to get some sleep, not investigating this further right now :)


Ciao,
Michael.