[9] RFR(S): 8156760: VM crashes if -XX:-ReduceInitialCardMarks is set

[Vladimir Kozlov]

Looks good to me.
I don't see RBT link in bug report.

Thanks,
Vladimir

On 5/23/16 3:29 AM, Tobias Hartmann wrote:
> Hi Roland,
>
> thanks for the review! Please see comments below.
>
> On 23.05.2016 11:34, Roland Westrelin wrote:
>>
>> Hi Tobias,
>>
>>> Problem 3: C2 crashes with SIGSEGV in
>>> ArrayCopyNode::prepare_array_copy() because we expect an array
>>> clone/copy and dereference 'src_type->isa_aryptr()' but actually have
>>> a non-array Object.clone() [3]. This is because with
>>> !ReduceInitialCardMarks, ArrayCopyNode::try_clone_instance() does not
>>> capture the Object.clone() intrinsic because we emit card marking
>>> code (we bail out in 'ArrayCopyNode::finish_transform()'). We
>>> continue assuming that the array copy is a non-instance copy. I added
>>> an additional check to bail out in this case.
>>
>> One problem I noticed in this code is that
>> ArrayCopyNode::try_clone_instance() returns NULL to mean both this is
>> not a basic clone:
>>
>>   if (!is_clonebasic()) {
>>     return NULL;
>>   }
>>
>> and the clone failed:
>>
>>   if (!finish_transform(phase, can_reshape, ctl, mem)) {
>>     return NULL;
>>   }
>>
>> ArrayCopyNode::finish_transform() would fail with
>> !ReduceInitialCardMarks. The way I fixed this locally is to return
>> NodeSentinel when the clone fails so the caller can distinguish not a
>> clone from a failure. And then ArrayCopyNode::finish_transform():
>>
>>   Node* mem = try_clone_instance(phase, can_reshape, count);
>>   if (mem != NULL) {
>>     return mem == NodeSentinel ? NULL: mem;
>>   }
>>
>> Does that solve the same problem you're seeing?
>
> Yes, that's a better solution and solves the problem as well.
>
> Here is the updated webrev:
> http://cr.openjdk.java.net/~thartmann/8156760/webrev.01/
>
> Thanks,
> Tobias
>
>>
>> Roland.
>>