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

[Tobias Hartmann]

Hi,

all tests passed (link is attached to the bug). Is everyone fine with the latest webrev?
http://cr.openjdk.java.net/~thartmann/8156760/webrev.02/

Thanks,
Tobias

On 25.05.2016 17:40, Tobias Hartmann wrote:
> Hi,
> 
> On 23.05.2016 11:25, Tobias Hartmann wrote:
>> Hi,
>>
>> please review the following patch:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8156760
>> http://cr.openjdk.java.net/~thartmann/8156760/webrev.00/
>>
>> While working on JDK-8155643, I found several problems when running tests with -XX:-ReduceInitialCardMarks:
>>
>> Problem 1:
>> C2 crashes with "missing G1 post barrier" while trying to eliminate the card mark emitted by the Object.clone() intrinsic (after removing the allocation of the destination object) [1]. The problem is that the shape of the card mark code is different for the Object.clone() intrinsic because we don't emit any checks (see [4]). However, PhaseMacroExpand::eliminate_card_mark() tries to find and collapse a region check. Usually, this is not a problem because with ReduceInitialCardMarks we don't emit a post barrier for Object.clone().
>>
>> Problem 2:
>> The VM crashes during GC verification in G1SATBCardTableModRefBS::verify_g1_young_region() with "there should not have been any failures" because we expect cards for the young generation to be always set to 'g1_young_gen' [2]:
>>
>> [1.478s][error][gc,verify] == CT verification failed: [0x00007f097c167800,0x00007f097c167fff]
>> [1.478s][error][gc,verify] == expecting value: 32
>> [1.478s][error][gc,verify] == card 0x00007f097c167800 [0x00000006d8900000,0x00000006d8900200], val: 0
>> [1.478s][error][gc,verify] == card 0x00007f097c167801 [0x00000006d8900200,0x00000006d8900400], val: 0
>> ...
>>
>> With !ReduceInitialCardMarks, the Object.clone() intrinsic emits card marks that set the cards of the newly allocated destination object to 'dirty' (!= 'g1_young_gen') and thus causing the verification to fail.
>>
>> I fixed problems 1 and 2 by removing the card marking code for the Object.clone() intrinsic with G1. This should be fine because the destination object is always in Eden and we therefore don't need to mark the cards.
> 
> As Dean and Thomas pointed out, if ReduceInitialCardsMarks is disabled, the GC relies on the compiler always emitting card marks after object allocation because very large objects may not be allocated in Eden. To fix problem 2, we need to add a runtime check for the card value to make sure that we only emit card marks if the object is 'humongous' and therefore allocated in the old generation.
> 
> Problem 1 is fixed by adapting PhaseMacroExpand::eliminate_card_mark() to handle the graph shape of the card marking code for the Object.clone() intrinsic (we simply fold the new card value check).
> 
>> 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.
>>
>> I changed 'TestInstanceCloneAsLoadsStores' to be also executed with -XX:-ReduceInitialCardMarks. This triggers problem 1 and 2. Problem 3 can be reproduced by running the modified test with XX:+UseConcMarkSweepGC.
> 
> While testing, I found a 4th issue with -XX:-ReduceInitialCardMarks:
> TestEliminatedArrayCopyDeopt fails because the result of a read from an eliminated object allocation returns the wrong result after deoptimization. This is similar to JDK-8130847 but the problem is that the fix does not expect clones to write to objects that are allocated with card marks. As a result, the clone is treated as independent and not taken into account when eliminating the allocation of the destination object. I adapted ArrayCopyNode::may_modify() to skip card marks and correctly return the dependent clone ArrayCopyNode.
> 
> Here is the new webrev:
> http://cr.openjdk.java.net/~thartmann/8156760/webrev.02/
> 
> I'll re-run the tests.
> 
> Thanks,
> Tobias
> 
>> Tested with failing tests, JPRT and RBT (running).
>>
>> Thanks,
>> Tobias
>>
>> [1] https://bugs.openjdk.java.net/secure/attachment/59410/hs_err_pid28313.log
>> [2] https://bugs.openjdk.java.net/secure/attachment/59661/hs_err_pid30301.log
>> [3] https://bugs.openjdk.java.net/secure/attachment/59660/hs_err_pid22570.log
>> [4] https://bugs.openjdk.java.net/secure/attachment/59564/graph.png
>>