Re: RFR: 8333748: javap crash - Fatal error: Unmatched bit position 0x2 for location CLASS [v3]
On Mon, 17 Jun 2024 15:56:56 GMT, Adam Sotona wrote:
>> Chen Liang has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - Improve tests to check unmatched bit position and failure for
>> non-inner-classes
>> - Report error for flag problems
>
> src/jdk.jdeps/share/classes/com/sun/tools/javap/BasicWriter.java line 84:
>
>> 82: } catch (IllegalArgumentException ex) {
>> 83: mask &= LOCATION_MASKS.get(location);
>> 84: report(ex);
>
> Unfortunately the original exception message is missing any info that it is
> related to access flags and it is not very clear what "Error: Unmatched bit
> position 0x2 for location CLASS" means.
> I would add at least a message prefix, for example "Error: Access Flags:
> Unmatched bit position 0x2 for location CLASS".
Agreed, even though this message is already printed right by the access
modifiers.
> test/langtools/tools/javap/UndefinedAccessFlagTest.java line 105:
>
>> 103: .writeAll()
>> 104: .getOutputLines(Task.OutputKind.DIRECT);
>> 105: assertTrue(lines.stream().anyMatch(l -> l.contains("Unmatched
>> bit position")), () -> String.join("\n", lines));
>
> I would add a check the "fatal" error does not occur or any other way to
> confirm correct recovery.
Great suggestion! I will filter the lines starting with `Error:` and assert
they `allMatch(st -> st.contains("Access Flags:"))`
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19708#discussion_r1643072918
PR Review Comment: https://git.openjdk.org/jdk/pull/19708#discussion_r1643073763
Re: RFR: 8333748: javap crash - Fatal error: Unmatched bit position 0x2 for location CLASS [v3]
On Mon, 17 Jun 2024 13:55:41 GMT, Chen Liang wrote:
>> Currently, javap crashes for class files that have set non-zero values for
>> undefined access flag bits, as
>> `java.lang.reflect.AccessFlag.maskToAccessFlag` and
>> `java.lang.classfile.AccessFlags.flags` fail. In contrast, the JVMS, though
>> asking for these bits to be set to 0, requires VM to proceed and ignore
>> these bits. javap should emulate the VM behavior and proceed rendering,
>> ignoring these undefined bits.
>>
>> In addition, a few bytecode generation utilities in the JDK set unused bits,
>> such as in
>> `java.lang.invoke.MethodHandleImpl.BindCaller#generateInvokerTemplate` and
>> `java.lang.invoke.GenerateJLIClassesHelper#generateCodeBytesForLFs`. Those
>> can be updated in a later cleanup.
>
> Chen Liang has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Improve tests to check unmatched bit position and failure for
> non-inner-classes
> - Report error for flag problems
src/jdk.jdeps/share/classes/com/sun/tools/javap/BasicWriter.java line 84:
> 82: } catch (IllegalArgumentException ex) {
> 83: mask &= LOCATION_MASKS.get(location);
> 84: report(ex);
Unfortunately the original exception message is missing any info that it is
related to access flags and it is not very clear what "Error: Unmatched bit
position 0x2 for location CLASS" means.
I would add at least a message prefix, for example "Error: Access Flags:
Unmatched bit position 0x2 for location CLASS".
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19708#discussion_r1643041633
Re: RFR: 8333748: javap crash - Fatal error: Unmatched bit position 0x2 for location CLASS [v3]
On Mon, 17 Jun 2024 13:55:41 GMT, Chen Liang wrote:
>> Currently, javap crashes for class files that have set non-zero values for
>> undefined access flag bits, as
>> `java.lang.reflect.AccessFlag.maskToAccessFlag` and
>> `java.lang.classfile.AccessFlags.flags` fail. In contrast, the JVMS, though
>> asking for these bits to be set to 0, requires VM to proceed and ignore
>> these bits. javap should emulate the VM behavior and proceed rendering,
>> ignoring these undefined bits.
>>
>> In addition, a few bytecode generation utilities in the JDK set unused bits,
>> such as in
>> `java.lang.invoke.MethodHandleImpl.BindCaller#generateInvokerTemplate` and
>> `java.lang.invoke.GenerateJLIClassesHelper#generateCodeBytesForLFs`. Those
>> can be updated in a later cleanup.
>
> Chen Liang has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Improve tests to check unmatched bit position and failure for
> non-inner-classes
> - Report error for flag problems
test/langtools/tools/javap/UndefinedAccessFlagTest.java line 105:
> 103: .writeAll()
> 104: .getOutputLines(Task.OutputKind.DIRECT);
> 105: assertTrue(lines.stream().anyMatch(l -> l.contains("Unmatched
> bit position")), () -> String.join("\n", lines));
I would add a check the "fatal" error does not occur or any other way to
confirm correct recovery.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19708#discussion_r1642972527
Re: RFR: 8333748: javap crash - Fatal error: Unmatched bit position 0x2 for location CLASS [v3]
> Currently, javap crashes for class files that have set non-zero values for > undefined access flag bits, as > `java.lang.reflect.AccessFlag.maskToAccessFlag` and > `java.lang.classfile.AccessFlags.flags` fail. In contrast, the JVMS, though > asking for these bits to be set to 0, requires VM to proceed and ignore these > bits. javap should emulate the VM behavior and proceed rendering, ignoring > these undefined bits. > > In addition, a few bytecode generation utilities in the JDK set unused bits, > such as in > `java.lang.invoke.MethodHandleImpl.BindCaller#generateInvokerTemplate` and > `java.lang.invoke.GenerateJLIClassesHelper#generateCodeBytesForLFs`. Those > can be updated in a later cleanup. Chen Liang has updated the pull request incrementally with two additional commits since the last revision: - Improve tests to check unmatched bit position and failure for non-inner-classes - Report error for flag problems - Changes: - all: https://git.openjdk.org/jdk/pull/19708/files - new: https://git.openjdk.org/jdk/pull/19708/files/84506788..0f3e1a97 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19708=02 - incr: https://webrevs.openjdk.org/?repo=jdk=19708=01-02 Stats: 124 lines in 4 files changed: 55 ins; 34 del; 35 mod Patch: https://git.openjdk.org/jdk/pull/19708.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19708/head:pull/19708 PR: https://git.openjdk.org/jdk/pull/19708
