bad PAST->PIR generation

[TiMBuS]

Ok, this one is just strange. My compiler has stopped working today, and I've pinned it down to a malformed PIR instruction when I try to construct a method call with exactly seven arguments. Yes, it works with any other number of arguments.

So, when given the following PAST:

"past" => PMC 'PAST;Block'  {
    <blocktype> => "declaration"
    <pirflags> => ":load"
    [0] => PMC 'PAST;Op'  {
        <name> => "push"
        <pasttype> => "callmethod"
        [0] => PMC 'PAST;Var'  {
            <name> => "funstack"
            <scope> => "package"
            <namespace> => "private"
        }
        [1] => PMC 'PAST;Val'  {
            <value> => "1"
            <returns> => "Integer"
        }
        [2] => PMC 'PAST;Val'  {
            <value> => "2"
            <returns> => "Integer"
        }
        [3] => PMC 'PAST;Val'  {
            <value> => "3"
            <returns> => "Integer"
        }
        [4] => PMC 'PAST;Val'  {
            <value> => "4"
            <returns> => "Integer"
        }
        [5] => PMC 'PAST;Val'  {
            <value> => "5"
            <returns> => "Integer"
        }
        [6] => PMC 'PAST;Val'  {
            <value> => "6"
            <returns> => "Integer"
        }
        [7] => PMC 'PAST;Val'  {
            <value> => "7"
            <returns> => "Integer"
        }
    }
}

I will get the following PIR:

.namespace []
.sub "_block11" :load :anon :subid("10")
    get_hll_global $P13, ["private"], "funstack"
    $P14 = 1."push"(2, 3, 4, 5, 6, 7, )
    .return ($P14)
.end

Which is broken. it should be $P14 = $P13."push"(1, 2, 3, 4, 5, 6, 7)

Sometimes, instead of the off-by-one thing above, it will get a crash and the following error:

*** glibc detected *** ../../parrot: realloc(): invalid next size: 0x00000000028197e0 ***
======= Backtrace: =========
/lib/libc.so.6[0x7fc189000938]
/lib/libc.so.6[0x7fc189004c31]
/lib/libc.so.6(realloc+0x12e)[0x7fc189005abe]
/home/timbus/parrot/blib/lib/libparrot.so.0.9.0(mem_sys_realloc+0x2b)[0x7fc18a0baa7b]
/home/timbus/parrot/blib/lib/libparrot.so.0.9.0(Parrot_ResizablePMCArray_set_integer_native+0x131)[0x7fc18a2c3531]
/home/timbus/parrot/blib/lib/libparrot.so.0.9.0(Parrot_ResizablePMCArray_push_pmc+0x40)[0x7fc18a2bf640]
/home/timbus/parrot/blib/lib/libparrot.so.0.9.0(Parrot_process_args+0x461)[0x7fc18a0c8ec1]
/home/timbus/parrot/blib/lib/libparrot.so.0.9.0(parrot_pass_args+0x1ab)[0x7fc18a0c928b]
/home/timbus/parrot/blib/lib/libparrot.so.0.9.0(Parrot_get_params_pc+0xd3)[0x7fc18a06f543]
/home/timbus/parrot/blib/lib/libparrot.so.0.9.0[0x7fc18a105365]
/home/timbus/parrot/blib/lib/libparrot.so.0.9.0[0x7fc18a0cd9c5]
/home/timbus/parrot/blib/lib/libparrot.so.0.9.0[0x7fc18a0ce4ca]
/home/timbus/parrot/blib/lib/libparrot.so.0.9.0[0x7fc18a0ce8a8]
/home/timbus/parrot/blib/lib/libparrot.so.0.9.0(Parrot_runops_fromc_args+0x17c)[0x7fc18a0cfc7c]
/home/timbus/parrot/blib/lib/libparrot.so.0.9.0(Parrot_runcode+0x27e)[0x7fc18a0a9e9e]
/home/timbus/parrot/blib/lib/libparrot.so.0.9.0[0x7fc18a2fad64]
/home/timbus/parrot/blib/lib/libparrot.so.0.9.0(imcc_run+0x377)[0x7fc18a2fb9f7]
../../parrot[0x400ba4]
/lib/libc.so.6(__libc_start_main+0xe6)[0x7fc188fa5466]
../../parrot[0x400a49]

I'm not sure how to even work around this.. what is going on?

This is a clean checkout of parrot, r36168, running on ubuntu 8.10 64-bit.

[TiMBuS]

Addendum: This is bug was introduced in r36165. I get normal behaviour for all prior revisions.

[cotto]

minimal test case for bad PIR generation

[cotto]

I hacked up a minimal test case from examples/past/01-sub.pir. As reported, it's r36165 that breaks it. From running the code with instrumented PMC_x_val macros, it looks like there aren't any stray UnionVal accessor macros that are getting called with *StringArray, so I don't know what the problem is. Either way, this should make it easier to debug.

[cotto]

The bug is triggered in compilers/pct/src/POST/Compiler.pir:167 when CodeString's emit method is called. Something in the PCC code taking the 1 from the arglist :flat arg and sticking it in the "i" named arg. The code misbehaves the same way when the method has 7, 15, 31, etc args but works correctly for other numbers.

[chromatic]

Does reverting r36159 help at all? I hope it's unrelated, but the behavior sounds suspicious.

[cotto]

Replying to chromatic:

Does reverting r36159 help at all? I hope it's unrelated, but the behavior sounds suspicious.

Nope. It looks like it's indeed unrelated.

[Util]

Problem found, in src/pmc/resizablestringarray.pmc, in function unshift_string(). Problem fixed, and regression test added.

Detail: Prior to r36165, the ResizableStringArray PMC had redundant code in unshift_string():

    VTABLE void unshift_string(STRING *value) {
        STRING **data = PMC_data_typed(SELF, STRING **);
        INTVAL   size = PMC_int_val(SELF);
        INTVAL   i;

        SELF.set_integer_native(size + 1);

        data = PMC_data_typed(SELF, STRING **);

        for (i = size; i; --i)
            data[i] = data[i - 1];

        SELF.set_string_keyed_int(0, value);
    }

Notice that data is populated twice; once before set_integer_native(), and once after.

In r36165, mixed in with the many ATTR changes, the redundancy was removed from unshift_string(), by removing the assignment before set_integer_native(). This change meant that if the call to set_integer_native() caused a re-alloc of the array, then data would still point to the old copy of the array data. Such a re-alloc is done whenever the number of array elements grows past the next power-of-2, starting with 8, hence the bug.

Moving the assignment to after set_integer_native() resolves the problem.

Here is a smaller test case:

    .include 'library/dumper.pir'

    .sub 'main' :main
        .local pmc rsarray
        rsarray = new ['ResizableStringArray']

        push rsarray, "1"
        push rsarray, "2"
        push rsarray, "3"
        push rsarray, "4"
        push rsarray, "5"
        push rsarray, "6"
        push rsarray, "7"
        push rsarray, "8"
    # rsarray is now:  1  2  3  4  5  6  7  8

    # This unshift will cause a resize larger than the
    # initial resize_threshold (8), triggering the bug.
        unshift rsarray, "0"

    # rsarray should now be   : 0  1  2  3  4  5  6  7  8
    # The bug causes it to be : 0  2  3  4  5  6  7  8  ""

        _dumper( rsarray, 'rsarray' )
    .end

Based on that test case, I added a regression test to t/pmc/resizablestringarray.t, as sub unshift_string_resize_threshold().

This change also fixes Perl6 spectest failure in t/spec/S02-literals/quoting-unicode.t (where a hash was assigned power-of-two elements), and possibly in other spec tests.

(Having trouble committing; Patch or rev# to follow shortly)

[Util]

Fixed in r36340