From 7e55ed3b106ed08956d2d38b7c99fb81704667c9 Mon Sep 17 00:00:00 2001 From: Chen Liang Date: Fri, 21 Jun 2024 22:38:38 +0000 Subject: [PATCH] 8333748: javap crash - Fatal error: Unmatched bit position 0x2 for location CLASS Reviewed-by: asotona --- .../com/sun/tools/javap/AttributeWriter.java | 4 +- .../com/sun/tools/javap/BasicWriter.java | 42 +++++- .../com/sun/tools/javap/ClassWriter.java | 70 ++-------- .../tools/javap/UndefinedAccessFlagTest.java | 128 ++++++++++++++++++ 4 files changed, 184 insertions(+), 60 deletions(-) create mode 100644 test/langtools/tools/javap/UndefinedAccessFlagTest.java diff --git a/src/jdk.jdeps/share/classes/com/sun/tools/javap/AttributeWriter.java b/src/jdk.jdeps/share/classes/com/sun/tools/javap/AttributeWriter.java index 839ac2fd041..3b9336c499d 100644 --- a/src/jdk.jdeps/share/classes/com/sun/tools/javap/AttributeWriter.java +++ b/src/jdk.jdeps/share/classes/com/sun/tools/javap/AttributeWriter.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2007, 2020, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2007, 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -208,7 +208,7 @@ public class AttributeWriter extends BasicWriter { indent(+1); first = false; } - for (var flag : info.flags()) { + for (var flag : maskToAccessFlagsReportUnknown(access_flags, AccessFlag.Location.INNER_CLASS)) { if (flag.sourceModifier() && (flag != AccessFlag.ABSTRACT || !info.has(AccessFlag.INTERFACE))) { print(Modifier.toString(flag.mask()) + " "); diff --git a/src/jdk.jdeps/share/classes/com/sun/tools/javap/BasicWriter.java b/src/jdk.jdeps/share/classes/com/sun/tools/javap/BasicWriter.java index 8629957f907..64eecf92082 100644 --- a/src/jdk.jdeps/share/classes/com/sun/tools/javap/BasicWriter.java +++ b/src/jdk.jdeps/share/classes/com/sun/tools/javap/BasicWriter.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2007, 2014, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2007, 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -26,6 +26,12 @@ package com.sun.tools.javap; import java.io.PrintWriter; +import java.lang.classfile.AccessFlags; +import java.lang.reflect.AccessFlag; +import java.lang.reflect.Modifier; +import java.util.EnumMap; +import java.util.Map; +import java.util.Set; import java.util.function.Supplier; /* @@ -38,6 +44,26 @@ import java.util.function.Supplier; * deletion without notice. */ public class BasicWriter { + private static final Map LOCATION_MASKS; + + static { + var map = new EnumMap(AccessFlag.Location.class); + for (var loc : AccessFlag.Location.values()) { + map.put(loc, 0); + } + + for (var flag : AccessFlag.values()) { + for (var loc : flag.locations()) { + map.compute(loc, (_, v) -> v | flag.mask()); + } + } + + // Peculiarities from AccessFlag.maskToAccessFlag + map.compute(AccessFlag.Location.METHOD, (_, v) -> v | Modifier.STRICT); + + LOCATION_MASKS = map; + } + protected BasicWriter(Context context) { lineWriter = LineWriter.instance(context); out = context.get(PrintWriter.class); @@ -46,6 +72,20 @@ public class BasicWriter { throw new AssertionError(); } + protected Set flagsReportUnknown(AccessFlags flags) { + return maskToAccessFlagsReportUnknown(flags.flagsMask(), flags.location()); + } + + protected Set maskToAccessFlagsReportUnknown(int mask, AccessFlag.Location location) { + try { + return AccessFlag.maskToAccessFlags(mask, location); + } catch (IllegalArgumentException ex) { + mask &= LOCATION_MASKS.get(location); + report("Access Flags: " + ex.getMessage()); + return AccessFlag.maskToAccessFlags(mask, location); + } + } + protected void print(String s) { lineWriter.print(s); } diff --git a/src/jdk.jdeps/share/classes/com/sun/tools/javap/ClassWriter.java b/src/jdk.jdeps/share/classes/com/sun/tools/javap/ClassWriter.java index edf3d803af3..2483e99e49a 100644 --- a/src/jdk.jdeps/share/classes/com/sun/tools/javap/ClassWriter.java +++ b/src/jdk.jdeps/share/classes/com/sun/tools/javap/ClassWriter.java @@ -417,7 +417,7 @@ public class ClassWriter extends BasicWriter { return; var flags = AccessFlags.ofField(f.flags().flagsMask()); - writeModifiers(flags.flags().stream().filter(fl -> fl.sourceModifier()) + writeModifiers(flagsReportUnknown(flags).stream().filter(fl -> fl.sourceModifier()) .map(fl -> Modifier.toString(fl.mask())).toList()); print(() -> sigPrinter.print( f.findAttribute(Attributes.signature()) @@ -446,7 +446,7 @@ public class ClassWriter extends BasicWriter { if (options.verbose) writeList(String.format("flags: (0x%04x) ", flags.flagsMask()), - flags.flags().stream().map(fl -> "ACC_" + fl.name()).toList(), + flagsReportUnknown(flags).stream().map(fl -> "ACC_" + fl.name()).toList(), "\n"); if (options.showAllAttrs) { @@ -478,7 +478,7 @@ public class ClassWriter extends BasicWriter { int flags = m.flags().flagsMask(); var modifiers = new ArrayList(); - for (var f : AccessFlags.ofMethod(flags).flags()) + for (var f : flagsReportUnknown(m.flags())) if (f.sourceModifier()) modifiers.add(Modifier.toString(f.mask())); String name = "???"; @@ -561,7 +561,7 @@ public class ClassWriter extends BasicWriter { StringBuilder sb = new StringBuilder(); String sep = ""; sb.append(String.format("flags: (0x%04x) ", flags)); - for (var f : AccessFlags.ofMethod(flags).flags()) { + for (var f : flagsReportUnknown(m.flags())) { sb.append(sep).append("ACC_").append(f.name()); sep = ", "; } @@ -794,17 +794,9 @@ public class ClassWriter extends BasicWriter { } } - private static Set getClassModifiers(int mask) { - return getModifiers(AccessFlags.ofClass((mask & ACC_INTERFACE) != 0 - ? mask & ~ACC_ABSTRACT : mask).flags()); - } - - private static Set getMethodModifiers(int mask) { - return getModifiers(AccessFlags.ofMethod(mask).flags()); - } - - private static Set getFieldModifiers(int mask) { - return getModifiers(AccessFlags.ofField(mask).flags()); + private Set getClassModifiers(int mask) { + return getModifiers(flagsReportUnknown(AccessFlags.ofClass((mask & ACC_INTERFACE) != 0 + ? mask & ~ACC_ABSTRACT : mask))); } private static Set getModifiers(Set flags) { @@ -814,16 +806,16 @@ public class ClassWriter extends BasicWriter { return s; } - private static Set getClassFlags(int mask) { - return getFlags(mask, AccessFlags.ofClass(mask).flags()); + private Set getClassFlags(int mask) { + return getFlags(mask, flagsReportUnknown(AccessFlags.ofClass(mask))); } - private static Set getMethodFlags(int mask) { - return getFlags(mask, AccessFlags.ofMethod(mask).flags()); + private Set getMethodFlags(int mask) { + return getFlags(mask, flagsReportUnknown(AccessFlags.ofMethod(mask))); } - private static Set getFieldFlags(int mask) { - return getFlags(mask, AccessFlags.ofField(mask).flags()); + private Set getFieldFlags(int mask) { + return getFlags(mask, flagsReportUnknown(AccessFlags.ofField(mask))); } private static Set getFlags(int mask, Set flags) { @@ -840,42 +832,6 @@ public class ClassWriter extends BasicWriter { return s; } - public static enum AccessFlag { - ACC_PUBLIC (ClassFile.ACC_PUBLIC, "public", true, true, true, true ), - ACC_PRIVATE (ClassFile.ACC_PRIVATE, "private", false, true, true, true ), - ACC_PROTECTED (ClassFile.ACC_PROTECTED, "protected", false, true, true, true ), - ACC_STATIC (ClassFile.ACC_STATIC, "static", false, true, true, true ), - ACC_FINAL (ClassFile.ACC_FINAL, "final", true, true, true, true ), - ACC_SUPER (ClassFile.ACC_SUPER, null, true, false, false, false), - ACC_SYNCHRONIZED(ClassFile.ACC_SYNCHRONIZED, "synchronized", false, false, false, true ), - ACC_VOLATILE (ClassFile.ACC_VOLATILE, "volatile", false, false, true, false), - ACC_BRIDGE (ClassFile.ACC_BRIDGE, null, false, false, false, true ), - ACC_TRANSIENT (ClassFile.ACC_TRANSIENT, "transient", false, false, true, false), - ACC_VARARGS (ClassFile.ACC_VARARGS, null, false, false, false, true ), - ACC_NATIVE (ClassFile.ACC_NATIVE, "native", false, false, false, true ), - ACC_INTERFACE (ClassFile.ACC_INTERFACE, null, true, true, false, false), - ACC_ABSTRACT (ClassFile.ACC_ABSTRACT, "abstract", true, true, false, true ), - ACC_STRICT (ClassFile.ACC_STRICT, "strictfp", false, false, false, true ), - ACC_SYNTHETIC (ClassFile.ACC_SYNTHETIC, null, true, true, true, true ), - ACC_ANNOTATION (ClassFile.ACC_ANNOTATION, null, true, true, false, false), - ACC_ENUM (ClassFile.ACC_ENUM, null, true, true, true, false), - ACC_MODULE (ClassFile.ACC_MODULE, null, true, false, false, false); - - public final int flag; - public final String modifier; - public final boolean isClass, isInnerClass, isField, isMethod; - - AccessFlag(int flag, String modifier, boolean isClass, - boolean isInnerClass, boolean isField, boolean isMethod) { - this.flag = flag; - this.modifier = modifier; - this.isClass = isClass; - this.isInnerClass = isInnerClass; - this.isField = isField; - this.isMethod = isMethod; - } - } - private final Options options; private final AttributeWriter attrWriter; private final CodeWriter codeWriter; diff --git a/test/langtools/tools/javap/UndefinedAccessFlagTest.java b/test/langtools/tools/javap/UndefinedAccessFlagTest.java new file mode 100644 index 00000000000..bb531fa369c --- /dev/null +++ b/test/langtools/tools/javap/UndefinedAccessFlagTest.java @@ -0,0 +1,128 @@ +/* + * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test 8333748 + * @summary javap should not fail if reserved access flag bits are set to 1 + * @library /tools/lib + * @modules jdk.jdeps/com.sun.tools.javap + * @enablePreview + * @run junit UndefinedAccessFlagTest + */ + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; +import toolbox.JavapTask; +import toolbox.Task; +import toolbox.ToolBox; + +import java.lang.classfile.AccessFlags; +import java.lang.classfile.ClassModel; +import java.lang.classfile.FieldModel; +import java.lang.classfile.MethodModel; +import java.lang.classfile.attribute.InnerClassInfo; +import java.lang.classfile.attribute.InnerClassesAttribute; +import java.nio.file.Files; +import java.nio.file.Path; + +import static java.lang.classfile.ClassFile.*; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class UndefinedAccessFlagTest { + + final ToolBox toolBox = new ToolBox(); + + enum TestLocation { + NONE(false), CLASS, FIELD, METHOD, INNER_CLASS(false); + + final boolean fails; + TestLocation() { this(true); } + TestLocation(boolean fails) { this.fails = fails; } + } + + @ParameterizedTest + @EnumSource(TestLocation.class) + void test(TestLocation location) throws Throwable { + var cf = of(); + ClassModel cm; + try (var is = UndefinedAccessFlagTest.class.getResourceAsStream( + "/UndefinedAccessFlagTest$SampleInnerClass.class" + )) { + cm = cf.parse(is.readAllBytes()); + } + var bytes = cf.transform(cm, (cb, ce) -> { + switch (ce) { + case AccessFlags flags when location == TestLocation.CLASS -> cb + .withFlags(flags.flagsMask() | ACC_PRIVATE); + case FieldModel f when location == TestLocation.FIELD -> cb + .transformField(f, (fb, fe) -> { + if (fe instanceof AccessFlags flags) { + fb.withFlags(flags.flagsMask() | ACC_SYNCHRONIZED); + } else { + fb.with(fe); + } + }); + case MethodModel m when location == TestLocation.METHOD -> cb + .transformMethod(m, (mb, me) -> { + if (me instanceof AccessFlags flags) { + mb.withFlags(flags.flagsMask() | ACC_INTERFACE); + } else { + mb.with(me); + } + }); + case InnerClassesAttribute attr when location == TestLocation.INNER_CLASS -> cb + .with(InnerClassesAttribute.of(attr.classes().stream() + .map(ic -> InnerClassInfo.of(ic.innerClass(), ic.outerClass(), ic.innerName(), ic.flagsMask() | 0x0020)) + .toList())); + default -> cb.with(ce); + } + }); + + Files.write(Path.of("transformed.class"), bytes); + + var lines = new JavapTask(toolBox) + .classes("transformed.class") + .options("-c", "-p", "-v") + .run(location.fails ? Task.Expect.FAIL : Task.Expect.SUCCESS) + .writeAll() + .getOutputLines(Task.OutputKind.DIRECT); + + // No termination when access flag error happens + assertTrue(lines.stream().anyMatch(l -> l.contains("java.lang.String field;"))); + assertTrue(lines.stream().anyMatch(l -> l.contains("UndefinedAccessFlagTest$SampleInnerClass();"))); + assertTrue(lines.stream().anyMatch(l -> l.contains("void method();"))); + assertTrue(lines.stream().anyMatch(l -> l.contains("SampleInnerClass=class UndefinedAccessFlagTest$SampleInnerClass of class UndefinedAccessFlagTest"))); + + // Remove non-error lines + assertTrue(lines.removeIf(st -> !st.startsWith("Error:"))); + // Desired locations has errors + assertTrue(location == TestLocation.NONE || !lines.isEmpty()); + // Access Flag errors only + assertTrue(lines.stream().allMatch(l -> l.contains("Access Flags:")), () -> String.join("\n", lines)); + } + + static class SampleInnerClass { + String field; + void method() {} + } +}