Bug and patch

[Gilles Dauphin]

Hi,

I think some code is wrong in the assembler code of Sparc/Kaffe:
file : config/sparc/threads.h

#define THREADSWITCH(to, from) {                                \
		...

       ld [%1],%2                                      \n\
       ld [%2+16],%%i7                                 \n\
       ld [%2+12],%%l7                <-- this produce incorrecte code.
and is translate with gcc -S by:
       ld [%l7+12],%l7		 <---- gloups!



I think , here is the equivalent correct assembler code for native compiler:

void ThreadSwitch( ctx * to,  ctx * from )
{
        int regstore[6];
        register int *reg;              /* %i5 */
        register uint8 ** frp;          /* %i4 */
        register uint8 ** trp;          /* %i3 */


        reg = regstore; /* avoid compiler optimisation */
        frp =  &((from)->restorePoint);
        trp =  &((to)->restorePoint);

        asm("                                                   \n\
                call 1f                                         \n\
                nop                                             \n\
1:                                                              \n\
                add %o7,%lo(2f-1b+8),%l0                        \n\
                                                                \n\
                ta 3                                            \n\
                st %l0,[%i5+0]                                  \n\
                st %fp,[%i5+4]                                  \n\
                st %sp,[%i5+8]                                  \n\
                st %i5,[%i5+12]                                 \n\
                st %i7,[%i5+16]                                 \n\
                st %i5,[%i4]                                    \n\
                                                                \n\
                ld [%i3],%l0                                    \n\
                ld [%l0+16],%i7                                 \n\
                ld [%l0+12],%i5                                 \n\ <-- good !
                ld [%l0+8],%sp                                  \n\
                ld [%l0+4],%fp                                  \n\
                ld [%l0+0],%l0                                  \n\
                                                                \n\
                jmpl %l0,%g0                                    \n\
                nop                                             \n\
2:              nop"
        );
}

Now I have a kaffe compiled with native compiler (kaffe interpreter only).
I check for memory access and discover other bug :

In file kaffe/kaffevm/constants.c:
-------------------------------------
void
readConstantPool(Class* this, classFile* fp)
{
        constants* info = CLASS_CONSTANTS (this);
        ConstSlot* pool;
        u1* tags;
        int i;
        u1 type;
        u2 len;
        u2 d2, d2b;
        u4 d4, d4b;
        bool error;
 
        readu2(&info->size, fp);
RDBG(   printf("constant_pool_count=%d\n", info->size); )
 
        /* Allocate space for tags and data */
        pool = gc_malloc((sizeof(ConstSlot) + sizeof(u1)) * info->size, GC_NORMAL);
        tags = (u1*)&pool[info->size];
        GC_WRITE(this, pool);
        info->data = pool;
        info->tags = tags;
 
        error = false;
        pool[0] = 0;
        tags[0] = CONSTANT_Unknown;
        for (i = 1; i < info->size; i++) {
 
                readu1(&type, fp);
RDBG(           printf("Constant type %d\n", type);                     )
                tags[i] = type;
 
                switch (type) {
                case CONSTANT_Utf8:
                        readu2(&len, fp);
PROBLEM HERE-->>>       pool[i] = (ConstSlot) makeUtf8Const (fp->buf, len);
                        fp->buf += len; 
                        break;  
---------------------------------------------

Some is wrong whith code in file gc-incremental.c
-------------------------------
void
walkObject(gcInfo* base)
{
        void** mem;
        gcInfo* obj;
        int32 i;

        assert(base->colour == GC_GREY);
        base->colour = GC_BLACK;
        if (base->final != GC_ROOT) {
                REMOVELIST(base);
                APPENDLIST(black, base);
        }

        gcStats.markedobj += 1;
        gcStats.markedmem += base->size;

DBG(    printf("walkObject: %d %x-%x\n", base->ref, GC2MEM(base),
                GC2MEM(base)+base->size);fflush(stdout);                )

        mem = GC2MEM(base);
        for (i = (base->size / sizeof(void*)) - 1; i >= 0; i--) {
                obj = MEM2GC(mem[i]);			<---- PROBLEM HERE

                /* Check we've got a valid object */
                if (((void*)obj < minMem || (void*)obj > maxMem) ||
                    (((uintp)obj % sizeof(void*)) != 0) ||
                    (!VALIDREF(obj->ref)) ||	       <------PROBLEM HERE
                    (GETREF(obj->ref) != (uintp)obj)) { <-----PROBLEM HERE
                        continue;
                }
-------------
MEM2GC is macro :
#define	MEM2GC(_m)		((gcInfo*)(((uint8*)(_m)) - sizeof(gcInfo)))

You are assuming in function readConstantPool that a pool of Constant
have *one* gcInfo, and in function walkObject you say that *each* Constant
have a gcInfo.

Maybe i am wrong. But when checking this code, debugger say there is problem.


Any body suggest a patch for this?

Thank's

Gilles