RFR (S): 8026478: -XX:+VerifyAdapterSharing is broken

[Vladimir Kozlov]

Good.

Thanks,
Vladimir

On 10/15/13 10:55 PM, Albert Noll wrote:
> Christian, Vladimir, thanks for the reviews.
>
> @Christian: yes, I removed the signature since it was unused. I'll file an RFE to rename the two fields.
> @Vladimir: your suggestions make the code simpler. I adopted them in the new webrev.
>
> http://cr.openjdk.java.net/~anoll/8026478/webrev.01/ <http://cr.openjdk.java.net/%7Eanoll/8026478/webrev.01/>
>
> Best,
> Albert
>
> On 15.10.2013 21:03, Vladimir Kozlov wrote:
>> Albert,
>>
>> contains_all_stubs name is incorrect. It should be contains_all_checks. Adapter code does not have stubs in it.
>>
>> I think the next check should be:
>>
>> bool contains_all_checks = (StubRoutines::code2() != NULL) || !VerifyAdapterCalls;
>>
>> because code does not change when VerifyAdapterCalls is false.
>> Then you don't need to check VerifyAdapterCalls in next condition:
>>
>> +       if ((VerifyAdapterCalls || VerifyAdapterSharing) && !entry->contains_all_checks()) {
>>
>> And we need to regenerate adapter regardless VerifyAdapterSharing because, as you said, VerifyAdapterCalls potentially
>> reports a false error if check for code2 is not generated.
>>
>> So the condition should be simple "if (!entry->contains_all_checks())".
>>
>> Could you leave next lines unchanged (use 2 lines for arguments)?:
>> ! buffer.insts()->initialize_shared_locs((relocInfo*)buffer_locs,
>>
>> sizeof(buffer_locs)/sizeof(relocInfo));
>>         MacroAssembler _masm(&buffer);
>>
>> Cleanup is fine.
>>
>> Thanks,
>> Vladimir
>>
>> On 10/15/13 4:14 AM, Albert Noll wrote:
>>> Hi,
>>>
>>> could I have reviews for this small patch?
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8026478
>>> webrev: http://cr.openjdk.java.net/~anoll/8026478/webrev.00/
>>> <http://cr.openjdk.java.net/%7Eanoll/8026478/webrev.00/>
>>>
>>> Problem: This flag is broken. The reason is that adapters
>>> (gen_i2c_adapter()) that are generated during VM startup do not include
>>> the range for StubRoutines::code2() if -XX:+VerifyAdapterCalls is
>>> specified. StubRoutines::code2() is only allocated later during startup;
>>> consequently, code2() is NULL. As a result, the size of the generated
>>> adapter as well as the code itself does not mactch.
>>>
>>> Solution: Defer check (VerifyAdapterSharing) until VM is initialzied.
>>> Note that this patch also fixes potential issues with
>>> -XX:+VerifyAdapterCalls, which is a dignostic flag. The patch also
>>> removes some unused field of VerifyAdapterSharing.
>>>
>>>
>>> Many thanks in advance,
>>> Albert
>>> **
>