8325373: Improve StackCounter error reporting for bad switch cases

Reviewed-by: psandoz

src/java.base/share/classes/jdk/internal/classfile/impl/StackCounter.java

@@ -37,6 +37,8 @@ import java.util.List;
 import java.util.Queue;
 
 import static java.lang.classfile.ClassFile.*;
+import java.lang.constant.ClassDesc;
+import java.util.stream.Collectors;
 
 public final class StackCounter {
 
@@ -45,6 +47,7 @@ public final class StackCounter {
     static StackCounter of(DirectCodeBuilder dcb, BufWriterImpl buf) {
         return new StackCounter(
                 dcb,
+                buf.thisClass().asSymbol(),
                 dcb.methodInfo.methodName().stringValue(),
                 dcb.methodInfo.methodTypeSymbol(),
                 (dcb.methodInfo.methodFlags() & ACC_STATIC) != 0,
@@ -56,8 +59,11 @@ public final class StackCounter {
     private int stack, maxStack, maxLocals, rets;
 
     private final RawBytecodeHelper bcs;
+    private final ClassDesc thisClass;
     private final String methodName;
     private final MethodTypeDesc methodDesc;
+    private final boolean isStatic;
+    private final ByteBuffer bytecode;
     private final SplitConstantPool cp;
     private final Queue<Target> targets;
     private final BitSet visited;
@@ -91,14 +97,18 @@ public final class StackCounter {
     }
 
     public StackCounter(LabelContext labelContext,
+                     ClassDesc thisClass,
                      String methodName,
                      MethodTypeDesc methodDesc,
                      boolean isStatic,
                      ByteBuffer bytecode,
                      SplitConstantPool cp,
                      List<AbstractPseudoInstruction.ExceptionCatchImpl> handlers) {
+        this.thisClass = thisClass;
         this.methodName = methodName;
         this.methodDesc = methodDesc;
+        this.isStatic = isStatic;
+        this.bytecode = bytecode;
         this.cp = cp;
         targets = new ArrayDeque<>();
         maxStack = stack = rets = 0;
@@ -247,24 +257,24 @@ public final class StackCounter {
                             int low = bcs.getInt(alignedBci + 4);
                             int high = bcs.getInt(alignedBci + 2 * 4);
                             if (low > high) {
-                                error("low must be less than or equal to high in tableswitch");
+                                throw error("low must be less than or equal to high in tableswitch");
                             }
                             keys = high - low + 1;
                             if (keys < 0) {
-                                error("too many keys in tableswitch");
+                                throw error("too many keys in tableswitch");
                             }
                             delta = 1;
                         } else {
                             keys = bcs.getInt(alignedBci + 4);
                             if (keys < 0) {
-                                error("number of keys in lookupswitch less than 0");
+                                throw error("number of keys in lookupswitch less than 0");
                             }
                             delta = 2;
                             for (int i = 0; i < (keys - 1); i++) {
                                 int this_key = bcs.getInt(alignedBci + (2 + 2 * i) * 4);
                                 int next_key = bcs.getInt(alignedBci + (2 + 2 * i + 2) * 4);
                                 if (this_key >= next_key) {
-                                    error("Bad lookupswitch instruction");
+                                    throw error("Bad lookupswitch instruction");
                                 }
                             }
                         }
@@ -326,7 +336,7 @@ public final class StackCounter {
                         next();
                     }
                     default ->
-                        error(String.format("Bad instruction: %02x", opcode));
+                        throw error(String.format("Bad instruction: %02x", opcode));
                 }
             }
         }
@@ -360,15 +370,17 @@ public final class StackCounter {
             case TAG_CONSTANTDYNAMIC ->
                 addStackSlot(((ConstantDynamicEntry)cp.entryByIndex(index)).typeKind().slotSize());
             default ->
-                error("CP entry #%d %s is not loadable constant".formatted(index, cp.entryByIndex(index).tag()));
+                throw error("CP entry #%d %s is not loadable constant".formatted(index, cp.entryByIndex(index).tag()));
         }
     }
 
-    private void error(String msg) {
+    private IllegalArgumentException error(String msg) {
-        throw new IllegalArgumentException("%s at bytecode offset %d of method %s(%s)".formatted(
+        var sb = new StringBuilder("%s at bytecode offset %d of method %s(%s)".formatted(
                 msg,
                 bcs.bci,
                 methodName,
-                methodDesc.displayDescriptor()));
+                methodDesc.parameterList().stream().map(ClassDesc::displayName).collect(Collectors.joining(","))));
+        Util.dumpMethod(cp, thisClass, methodName, methodDesc, isStatic ? ACC_STATIC : 0, bytecode, sb::append);
+        return new IllegalArgumentException(sb.toString());
     }
 }

src/java.base/share/classes/jdk/internal/classfile/impl/StackMapGenerator.java

@@ -48,8 +48,6 @@ import java.lang.classfile.BufWriter;
 import java.lang.classfile.Label;
 import java.lang.classfile.attribute.StackMapTableAttribute;
 import java.lang.classfile.Attributes;
-import java.lang.classfile.components.ClassPrinter;
-import java.lang.classfile.attribute.CodeAttribute;
 
 /**
  * StackMapGenerator is responsible for stack map frames generation.
@@ -836,36 +834,7 @@ public final class StackMapGenerator {
                 offset,
                 methodName,
                 methodDesc.parameterList().stream().map(ClassDesc::displayName).collect(Collectors.joining(","))));
-        //try to attach debug info about corrupted bytecode to the message
+        Util.dumpMethod(cp, thisType.sym(), methodName, methodDesc, isStatic ? ACC_STATIC : 0, bytecode, sb::append);
-        try {
-            var cc = ClassFile.of();
-            var clm = cc.parse(cc.build(cp.classEntry(thisType.sym()), cp, clb ->
-                    clb.withMethod(methodName, methodDesc, isStatic ? ACC_STATIC : 0, mb ->
-                            ((DirectMethodBuilder)mb).writeAttribute(new UnboundAttribute.AdHocAttribute<CodeAttribute>(Attributes.CODE) {
-                                @Override
-                                public void writeBody(BufWriter b) {
-                                    b.writeU2(-1);//max stack
-                                    b.writeU2(-1);//max locals
-                                    b.writeInt(bytecode.limit());
-                                    b.writeBytes(bytecode.array(), 0, bytecode.limit());
-                                    b.writeU2(0);//exception handlers
-                                    b.writeU2(0);//attributes
-                                }
-                    }))));
-            ClassPrinter.toYaml(clm.methods().get(0).code().get(), ClassPrinter.Verbosity.TRACE_ALL, sb::append);
-        } catch (Error | Exception suppresed) {
-            //fallback to bytecode hex dump
-            bytecode.rewind();
-            while (bytecode.position() < bytecode.limit()) {
-                sb.append("%n%04x:".formatted(bytecode.position()));
-                for (int i = 0; i < 16 && bytecode.position() < bytecode.limit(); i++) {
-                    sb.append(" %02x".formatted(bytecode.get()));
-                }
-            }
-            var err = new IllegalArgumentException(sb.toString());
-            err.addSuppressed(suppresed);
-            return err;
-        }
         return new IllegalArgumentException(sb.toString());
     }
 

src/java.base/share/classes/jdk/internal/classfile/impl/Util.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2022, 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
@@ -33,6 +33,8 @@ import java.util.function.Function;
 
 import java.lang.classfile.Attribute;
 import java.lang.classfile.AttributeMapper;
+import java.lang.classfile.Attributes;
+import java.lang.classfile.BufWriter;
 import java.lang.classfile.ClassFile;
 import java.lang.classfile.Opcode;
 import java.lang.classfile.constantpool.ClassEntry;
@@ -43,6 +45,11 @@ import java.lang.reflect.AccessFlag;
 import jdk.internal.access.SharedSecrets;
 
 import static java.lang.classfile.ClassFile.ACC_STATIC;
+import java.lang.classfile.attribute.CodeAttribute;
+import java.lang.classfile.components.ClassPrinter;
+import java.lang.classfile.constantpool.ConstantPoolBuilder;
+import java.nio.ByteBuffer;
+import java.util.function.Consumer;
 
 /**
  * Helper to create and manipulate type descriptors, where type descriptors are
@@ -193,4 +200,41 @@ public class Util {
         char ch = desc.descriptorString().charAt(0);
         return ch == 'D' || ch == 'J';
     }
+
+    public static void dumpMethod(SplitConstantPool cp,
+                                  ClassDesc cls,
+                                  String methodName,
+                                  MethodTypeDesc methodDesc,
+                                  int acc,
+                                  ByteBuffer bytecode,
+                                  Consumer<String> dump) {
+
+        // try to dump debug info about corrupted bytecode
+        try {
+            var cc = ClassFile.of();
+            var clm = cc.parse(cc.build(cp.classEntry(cls), cp, clb ->
+                    clb.withMethod(methodName, methodDesc, acc, mb ->
+                            ((DirectMethodBuilder)mb).writeAttribute(new UnboundAttribute.AdHocAttribute<CodeAttribute>(Attributes.CODE) {
+                                @Override
+                                public void writeBody(BufWriter b) {
+                                    b.writeU2(-1);//max stack
+                                    b.writeU2(-1);//max locals
+                                    b.writeInt(bytecode.limit());
+                                    b.writeBytes(bytecode.array(), 0, bytecode.limit());
+                                    b.writeU2(0);//exception handlers
+                                    b.writeU2(0);//attributes
+                                }
+                    }))));
+            ClassPrinter.toYaml(clm.methods().get(0).code().get(), ClassPrinter.Verbosity.TRACE_ALL, dump);
+        } catch (Error | Exception _) {
+            // fallback to bytecode hex dump
+            bytecode.rewind();
+            while (bytecode.position() < bytecode.limit()) {
+                dump.accept("%n%04x:".formatted(bytecode.position()));
+                for (int i = 0; i < 16 && bytecode.position() < bytecode.limit(); i++) {
+                    dump.accept(" %02x".formatted(bytecode.get()));
+                }
+            }
+        }
+    }
 }