RFR: Value types consistency checks

[Karen Kinnear]

Frederic (and Harold who offered to push this for the JIT folks to build on):

The code looks good - ship it.

Harold offered to look into the EnableValhalla issues with classfileparser/verifier as well.

I am walking code paths to see if we need to add any additional checks, but that could be
a follow-on.

And the test itself could be a follow-on as well.

thanks,
Karen

> On Jul 6, 2018, at 4:26 PM, Frederic Parain <frederic.parain at oracle.com> wrote:
> 
> Karen,
> 
> Thank you for the review.
> 
> Here’s a updated version of the patch:
> http://cr.openjdk.java.net/~fparain/VTAttributeChecks/webrev.02/index.html
> 
> I’ve added support for declarer/overrider checks, fixed a few bugs and
> added asserts to check “L” in array signatures.
> 
> I’ve tried to generate ValueTypes attribute meta-data only when
> EnableValhalla was set to true, unfortunately, it causes some
> failures with verifier tests. I’ll look at his in more details after
> my vacations.
> 
> Testing is tricky, I’m currently using some Java source files
> and a script to test mismatch scenarios, but in their current
> form, they cannot be integrated with jtreg. I’ve attached the
> test files to this mail if you want to use them.
> 
> Regards,
> 
> Fred
> 
> <Parent.java><Point.java><Provider.java><run.sh><Test.java><ValuePoint.java><WrongRemoteMethod.java><ObjectPoint.java><InterfaceWithDefault.java>
> 
> 
>> On Jun 29, 2018, at 16:51, Karen Kinnear <KAREN.KINNEAR at ORACLE.COM> wrote:
>> 
>> Frederic,
>> 
>> Many thanks for doing this and doing this so cleanly.
>> 
>> Couple of questions:
>> 1. instanceKlass.cpp: 
>> if (has_value_types_attribute) line 632 — does this want to also be if EnableValhalla?
>> my understanding is that extra attributes are ignored so classFileParser should just allow them
>> 
>> 2. instanceKlass.cpp check_symbol/signature_for_value_types_consistency
>> In the array part, did you want to check for “L” or is that just me being over cautious?
>> 
>> 3. klassVtable.cpp
>> In update_inherited_vtable when you override a method (this is the vtable part),
>> there is a if (check constraints & !targer_method()->is_overpass() - which needs
>> a declarer/overridder match check. Also in initialize_itable_for_interface under if (check constraints)
>> 
>> 4. Thank you for adding the ValueTypes attributes in the class files for the tests.
>> Did you add any tests that fail consistency?
>> 
>> many thanks,
>> Karen
>> 
>>> On Jun 28, 2018, at 4:38 PM, Frederic Parain <frederic.parain at oracle.com> wrote:
>>> 
>>> Please review this changeset implementing consistency checks based on the
>>> ValueTypes attribute. These checks ensure that the assumptions a class has
>>> about value types, as encoded in its ValueTypes attribute, match the
>>> reality, or the assumptions of another class it links to.
>>> 
>>> The details of the consistency checks have been summarized by Karen in
>>> this document:
>>> http://cr.openjdk.java.net/~acorn/value-types-consistency-checking-details.pdf
>>> 
>>> If the implementation doesn’t match the document, this is likely to be bug,
>>> so please, report it.
>>> 
>>> Some tests using the Bytecodes API have been updated to include a
>>> ValueTypes attribute in the class files they generate (thanks to Srikanth
>>> for adding this feature to the Bytecodes API).
>>> 
>>> Webrev:
>>> http://cr.openjdk.java.net/~fparain/VTAttributeChecks/webrev.01/
>>> 
>>> Thank you,
>>> 
>>> Fred
>>> 
>> 
>