hpi-swa / RSqueak

A Squeak/Smalltalk VM written in RPython.
BSD 3-Clause "New" or "Revised" License
84 stars 15 forks source link

investigate in rpython translation of that #95

Closed abstraktor closed 9 years ago

abstraktor commented 9 years ago

This python code

size = 700000000000
if len(chunk.data) != size:
            # remove trailing alignment slots
            chunk.data = chunk.data[:size]

translates with rpython to that c:

    block46:
        l_v188404 = RPyField(l_chunk_75, ic_inst_data);
        l_v188675 = l_v188404->length;
        l_v188676 = (unsigned long long)(l_v188675);
        OP_ULLONG_NE(l_v188676, l_size_59, l_v188677);
        if (l_v188677) {
                goto block73;
        }
        goto block47;
   //. . . 
    block73:
        l_v188737 = (Signed)(l_size_59);
        l_v188738 = (&pypy_g_rpython_memory_gctypelayout_GCData)->gcd_inst_root_stack_top;
        OP_ADR_ADD(l_v188738, (sizeof(void*) * 2), l_v188739);
        (&pypy_g_rpython_memory_gctypelayout_GCData)->gcd_inst_root_stack_top = l_v188739;
        l_v188741 = (void*)l_chunk_75;
        ((void* *) (((char *)l_v188738) + 0))[0] = l_v188741;
        l_v188743 = (void*)l_self_1363;
        ((void* *) (((char *)l_v188738) + sizeof(void*)))[0] = l_v188743;
        l_v188413 = pypy_g_ll_listslice_startstop__GcArray_SignedLlT_arrayP(l_v188404, 0L, l_v188737);
        l_v188745 = (&pypy_g_rpython_memory_gctypelayout_GCData)->gcd_inst_root_stack_top;
        OP_ADR_SUB(l_v188745, (sizeof(void*) * 2), l_v188746);
        (&pypy_g_rpython_memory_gctypelayout_GCData)->gcd_inst_root_stack_top = l_v188746;
        l_v188748 = ((void* *) (((char *)l_v188746) + 0))[0];
        l_chunk_75 = l_v188748; /* for moving GCs */
        l_v188750 = ((void* *) (((char *)l_v188746) + sizeof(void*)))[0];
        l_self_1363 = l_v188750; /* for moving GCs */
        l_v188752 = (&pypy_g_ExcData)->ed_exc_type;
        l_v188753 = (l_v188752 == NULL);
        if (!l_v188753) {
                goto block77;
        }
        goto block74;
krono commented 9 years ago

what is the issue?

j4yk commented 9 years ago

It raises a MemoryError. I assume that for yet undetermined reasons size is larger (needs a long) than chunk.size but instead of simply keeping the list or copying it, the MemoryError appears.

When the python code is interpreted instead of translated, the overgrown size value does not appear at all and no MemoryErrors occur.

krono commented 9 years ago

Why len(chunk.data) != size and not len(chunk.data) >= size, btw?

j4yk commented 9 years ago

Actually, the only reason to assume that chunk.size is indeed smaller than size in this case is that allocating a list with a length that is > 2**31 would throw a MemoryError in the first place and not when trying to slice it.

Reply to != vs. >=: the code omitted here before should make sure that len(chunk.data) can only exceed or equal size usually (it is actually a rounding up for 64bit alignment).

arigo commented 9 years ago

Fwiw:

krono commented 9 years ago

(RSqueak is 32bit only, still)

arigo commented 9 years ago

Ah, sorry. Then indeed you're doing "chunk.data[:size]", where size is probably a "r_ulonglong". It gets converted to a signed 32-bit number, which for a number like 700000000000 gives a negative result. But the RPython annotator "proved" that the size cannot be negative here (because it is ulonglong). It confuses the resulting code a lot.

Don't try to use slicing with numbers that are so huge. RPython would normally give a runtime error if you try to slice with a number larger than the length of the string (at least when compiled in lldebug mode), but in this case it's probably fooled by the fact that the number becomes negative.

j4yk commented 9 years ago

@abstraktor: in #pypy arigo suggested building with --lldebug to see if any assertions are violated. But the lldebug build result does not play nicely with the Windows C library here, so I would appreciate you taking this over.

On the other hand I will try to make sure that those bogus size values do not appear at all. ;-)

krono commented 9 years ago

Any progress?

j4yk commented 9 years ago

The bogus size values have been fixed, so the problem no longer occurs. We were only curious about the behavior after translation back in these days.