RFR: Value types consistency checks (original) (raw)

Karen Kinnear karen.kinnear at oracle.com
Wed Jul 11 16:05:07 UTC 2018

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 (hasvaluetypesattribute) 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 checksymbol/signatureforvaluetypesconsistency In the array part, did you want to check for “L” or is that just me being over cautious? 3. klassVtable.cpp In updateinheritedvtable when you override a method (this is the vtable part), there is a if (check constraints & !targermethod()->isoverpass() - which needs a declarer/overridder match check. Also in initializeitableforinterface 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

More information about the valhalla-dev mailing list