[From nobody Tue Jul 10 18:22:40 2018 Received: from [10.39.253.145] (/10.39.253.145) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Fri, 06 Jul 2018 13:26:48 -0700 Content-Type: multipart/mixed; boundary="Apple-Mail=_7A8BA831-A468-4650-9F26-1DC7036779BA" Mime-Version: 1.0 (Mac OS X Mail 11.4 \(3445.8.2\)) Subject: Re: RFR: Value types consistency checks From: Frederic Parain <frederic.parain@oracle.com> In-Reply-To: <4E9AA19D-1954-4838-896E-3E2D8D1A8A81@oracle.com> Date: Fri, 6 Jul 2018 16:26:47 -0400 Cc: valhalla-dev <valhalla-dev@openjdk.java.net> Message-Id: <98D40945-3D6C-4656-98C7-A1B72904BEBA@oracle.com> References: <71EB54F5-FAA9-4A9F-AEF7-DC1E756DF7E0@oracle.com> <4E9AA19D-1954-4838-896E-3E2D8D1A8A81@oracle.com> To: Karen Kinnear <KAREN.KINNEAR@ORACLE.COM> X-Mailer: Apple Mail (2.3445.8.2) --Apple-Mail=_7A8BA831-A468-4650-9F26-1DC7036779BA Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Karen, Thank you for the review. Here=E2=80=99s a updated version of the patch: = http://cr.openjdk.java.net/~fparain/VTAttributeChecks/webrev.02/index.html= I=E2=80=99ve added support for declarer/overrider checks, fixed a few = bugs and added asserts to check =E2=80=9CL=E2=80=9D in array signatures. I=E2=80=99ve tried to generate ValueTypes attribute meta-data only when EnableValhalla was set to true, unfortunately, it causes some failures with verifier tests. I=E2=80=99ll look at his in more details = after my vacations. Testing is tricky, I=E2=80=99m 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=E2=80=99ve attached the test files to this mail if you want to use them. Regards, Fred --Apple-Mail=_7A8BA831-A468-4650-9F26-1DC7036779BA Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 > On Jun 29, 2018, at 16:51, Karen Kinnear <KAREN.KINNEAR@ORACLE.COM> = wrote: >=20 > Frederic, >=20 > Many thanks for doing this and doing this so cleanly. >=20 > Couple of questions: > 1. instanceKlass.cpp:=20 > if (has_value_types_attribute) line 632 =E2=80=94 does this want to = also be if EnableValhalla? > my understanding is that extra attributes are ignored so = classFileParser should just allow them >=20 > 2. instanceKlass.cpp = check_symbol/signature_for_value_types_consistency > In the array part, did you want to check for =E2=80=9CL=E2=80=9D or = is that just me being over cautious? >=20 > 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) >=20 > 4. Thank you for adding the ValueTypes attributes in the class files = for the tests. > Did you add any tests that fail consistency? >=20 > many thanks, > Karen >=20 >> On Jun 28, 2018, at 4:38 PM, Frederic Parain = <frederic.parain@oracle.com> wrote: >>=20 >> 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. >>=20 >> 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 >>=20 >> If the implementation doesn=E2=80=99t match the document, this is = likely to be bug, >> so please, report it. >>=20 >> 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). >>=20 >> Webrev: >> http://cr.openjdk.java.net/~fparain/VTAttributeChecks/webrev.01/ >>=20 >> Thank you, >>=20 >> Fred >>=20 >=20 --Apple-Mail=_7A8BA831-A468-4650-9F26-1DC7036779BA-- ]