8333748: javap crash - Fatal error: Unmatched bit position 0x2 for location CLASS

Reviewed-by: asotona

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()) + " ");

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.</b>
  */
 public class BasicWriter {
+    private static final Map<AccessFlag.Location, Integer> LOCATION_MASKS;
+
+    static {
+        var map = new EnumMap<AccessFlag.Location, Integer>(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<AccessFlag> flagsReportUnknown(AccessFlags flags) {
+        return maskToAccessFlagsReportUnknown(flags.flagsMask(), flags.location());
+    }
+
+    protected Set<AccessFlag> 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);
     }

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<String>();
-        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<String> getClassModifiers(int mask) {
+    private Set<String> getClassModifiers(int mask) {
-        return getModifiers(AccessFlags.ofClass((mask & ACC_INTERFACE) != 0
+        return getModifiers(flagsReportUnknown(AccessFlags.ofClass((mask & ACC_INTERFACE) != 0
-                ? mask & ~ACC_ABSTRACT : mask).flags());
+                ? mask & ~ACC_ABSTRACT : mask)));
-    }
-
-    private static Set<String> getMethodModifiers(int mask) {
-        return getModifiers(AccessFlags.ofMethod(mask).flags());
-    }
-
-    private static Set<String> getFieldModifiers(int mask) {
-        return getModifiers(AccessFlags.ofField(mask).flags());
     }
 
     private static Set<String> getModifiers(Set<java.lang.reflect.AccessFlag> flags) {
@@ -814,16 +806,16 @@ public class ClassWriter extends BasicWriter {
         return s;
     }
 
-    private static Set<String> getClassFlags(int mask) {
+    private Set<String> getClassFlags(int mask) {
-        return getFlags(mask, AccessFlags.ofClass(mask).flags());
+        return getFlags(mask, flagsReportUnknown(AccessFlags.ofClass(mask)));
     }
 
-    private static Set<String> getMethodFlags(int mask) {
+    private Set<String> getMethodFlags(int mask) {
-        return getFlags(mask, AccessFlags.ofMethod(mask).flags());
+        return getFlags(mask, flagsReportUnknown(AccessFlags.ofMethod(mask)));
     }
 
-    private static Set<String> getFieldFlags(int mask) {
+    private Set<String> getFieldFlags(int mask) {
-        return getFlags(mask, AccessFlags.ofField(mask).flags());
+        return getFlags(mask, flagsReportUnknown(AccessFlags.ofField(mask)));
     }
 
     private static Set<String> getFlags(int mask, Set<java.lang.reflect.AccessFlag> 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;

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() {}
+    }
+}