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

[Albert Noll]

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
>> **

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20131016/bf626f45/attachment-0001.html