Ticket #256 (closed bug: fixed)

Opened 6 years ago

Last modified 5 years ago

bad PAST->PIR generation

Reported by: TiMBuS Owned by: pmichaud
Priority: normal Milestone:
Component: PCT Version: trunk
Severity: medium Keywords:
Cc: Language:
Patch status: Platform: linux

Description

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.

Attachments

bad_pir_gen.pir Download (1.0 KB) - added by cotto 6 years ago.
minimal test case for bad PIR generation

Change History

  Changed 6 years ago by TiMBuS

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

Changed 6 years ago by cotto

minimal test case for bad PIR generation

  Changed 6 years ago by 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.

  Changed 5 years ago by 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.

follow-up: ↓ 5   Changed 5 years ago by chromatic

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

in reply to: ↑ 4   Changed 5 years ago by 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.

  Changed 5 years ago by 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)

  Changed 5 years ago by Util

Fixed in r36340

  Changed 5 years ago by Util

  • status changed from new to closed
  • resolution set to fixed
Note: See TracTickets for help on using tickets.