Merge pull request #18932 from rmuir/painless_debug_exception

improve Debugger to print code even if it hits exception

modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java

@@ -22,6 +22,7 @@ package org.elasticsearch.painless;
 import org.elasticsearch.bootstrap.BootstrapInfo;
 import org.elasticsearch.painless.antlr.Walker;
 import org.elasticsearch.painless.node.SSource;
+import org.objectweb.asm.util.Printer;
 
 import java.net.MalformedURLException;
 import java.net.URL;
@@ -98,7 +99,7 @@ final class Compiler {
                 " plugin if a script longer than this length is a requirement.");
         }
 
-        SSource root = Walker.buildPainlessTree(name, source, settings);
+        SSource root = Walker.buildPainlessTree(name, source, settings, null);
 
         root.analyze();
         root.write();
@@ -120,14 +121,14 @@ final class Compiler {
      * @param settings The CompilerSettings to be used during the compilation.
      * @return The bytes for compilation.
      */
-    static byte[] compile(String name, String source, CompilerSettings settings) {
+    static byte[] compile(String name, String source, CompilerSettings settings, Printer debugStream) {
         if (source.length() > MAXIMUM_SOURCE_LENGTH) {
             throw new IllegalArgumentException("Scripts may be no longer than " + MAXIMUM_SOURCE_LENGTH +
                 " characters.  The passed in script is " + source.length() + " characters.  Consider using a" +
                 " plugin if a script longer than this length is a requirement.");
         }
 
-        SSource root = Walker.buildPainlessTree(name, source, settings);
+        SSource root = Walker.buildPainlessTree(name, source, settings, debugStream);
 
         root.analyze();
         root.write();

modules/lang-painless/src/main/java/org/elasticsearch/painless/MethodWriter.java

@@ -22,7 +22,7 @@ package org.elasticsearch.painless;
 import org.elasticsearch.painless.Definition.Cast;
 import org.elasticsearch.painless.Definition.Sort;
 import org.elasticsearch.painless.Definition.Type;
-import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.ClassVisitor;
 import org.objectweb.asm.Label;
 import org.objectweb.asm.Opcodes;
 import org.objectweb.asm.commons.GeneratorAdapter;
@@ -82,7 +82,7 @@ public final class MethodWriter extends GeneratorAdapter {
     private final Deque<List<org.objectweb.asm.Type>> stringConcatArgs =
         (INDY_STRING_CONCAT_BOOTSTRAP_HANDLE == null) ?  null : new ArrayDeque<>();
 
-    public MethodWriter(int access, Method method, ClassWriter cw, BitSet statements) {
+    public MethodWriter(int access, Method method, ClassVisitor cw, BitSet statements) {
         super(Opcodes.ASM5, cw.visitMethod(access, method.getName(), method.getDescriptor(), null, null),
                 access, method.getName(), method.getDescriptor());
 

modules/lang-painless/src/main/java/org/elasticsearch/painless/antlr/Walker.java

@@ -146,6 +146,7 @@ import org.elasticsearch.painless.node.SSource;
 import org.elasticsearch.painless.node.SThrow;
 import org.elasticsearch.painless.node.STry;
 import org.elasticsearch.painless.node.SWhile;
+import org.objectweb.asm.util.Printer;
 
 import java.util.ArrayDeque;
 import java.util.ArrayList;
@@ -158,12 +159,13 @@ import java.util.List;
  */
 public final class Walker extends PainlessParserBaseVisitor<Object> {
 
-    public static SSource buildPainlessTree(String sourceName, String sourceText, CompilerSettings settings) {
+    public static SSource buildPainlessTree(String sourceName, String sourceText, CompilerSettings settings, Printer debugStream) {
-        return new Walker(sourceName, sourceText, settings).source;
+        return new Walker(sourceName, sourceText, settings, debugStream).source;
     }
 
     private final SSource source;
     private final CompilerSettings settings;
+    private final Printer debugStream;
     private final String sourceName;
     private final String sourceText;
 
@@ -171,7 +173,8 @@ public final class Walker extends PainlessParserBaseVisitor<Object> {
     private final List<SFunction> synthetic = new ArrayList<>();
     private int syntheticCounter = 0;
 
-    private Walker(String sourceName, String sourceText, CompilerSettings settings) {
+    private Walker(String sourceName, String sourceText, CompilerSettings settings, Printer debugStream) {
+        this.debugStream = debugStream;
         this.settings = settings;
         this.sourceName = Location.computeSourceName(sourceName, sourceText);
         this.sourceText = sourceText;
@@ -236,7 +239,8 @@ public final class Walker extends PainlessParserBaseVisitor<Object> {
         
         functions.addAll(synthetic);
 
-        return new SSource(sourceName, sourceText, (ExecuteReserved)reserved.pop(), location(ctx), functions, statements);
+        return new SSource(sourceName, sourceText, debugStream, 
+                          (ExecuteReserved)reserved.pop(), location(ctx), functions, statements);
     }
 
     @Override

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SFunction.java

@@ -31,6 +31,7 @@ import org.elasticsearch.painless.Locals.Variable;
 import org.elasticsearch.painless.Location;
 import org.elasticsearch.painless.MethodWriter;
 import org.elasticsearch.painless.WriterConstants;
+import org.objectweb.asm.ClassVisitor;
 import org.objectweb.asm.ClassWriter;
 import org.objectweb.asm.Handle;
 import org.objectweb.asm.Opcodes;
@@ -146,8 +147,8 @@ public class SFunction extends AStatement {
         locals.addConstant(location, WriterConstants.METHOD_HANDLE_TYPE, staticHandleFieldName, this::initializeConstant);
     }
     
-    /** Writes the function to given ClassWriter. */
+    /** Writes the function to given ClassVisitor. */
-    void write (ClassWriter writer, BitSet statements) {
+    void write (ClassVisitor writer, BitSet statements) {
         int access = Opcodes.ACC_PRIVATE | Opcodes.ACC_STATIC;
         if (synthetic) {
             access |= Opcodes.ACC_SYNTHETIC;

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SSource.java

@@ -29,9 +29,11 @@ import org.elasticsearch.painless.Locals.Variable;
 import org.elasticsearch.painless.WriterConstants;
 import org.elasticsearch.painless.Location;
 import org.elasticsearch.painless.MethodWriter;
-
+import org.objectweb.asm.ClassVisitor;
 import org.objectweb.asm.ClassWriter;
 import org.objectweb.asm.Opcodes;
+import org.objectweb.asm.util.Printer;
+import org.objectweb.asm.util.TraceClassVisitor;
 
 import java.util.BitSet;
 import java.util.Collections;
@@ -53,6 +55,7 @@ public final class SSource extends AStatement {
 
     final String name;
     final String source;
+    final Printer debugStream;
     final ExecuteReserved reserved;
     final List<SFunction> functions;
     final List<AStatement> statements;
@@ -61,12 +64,13 @@ public final class SSource extends AStatement {
     private BitSet expressions;
     private byte[] bytes;
 
-    public SSource(String name, String source, ExecuteReserved reserved, Location location,
+    public SSource(String name, String source, Printer debugStream, ExecuteReserved reserved, Location location, 
                    List<SFunction> functions, List<AStatement> statements) {
         super(location);
 
         this.name = name;
         this.source = source;
+        this.debugStream = debugStream;
         this.reserved = reserved;
         this.functions = Collections.unmodifiableList(functions);
         this.statements = Collections.unmodifiableList(statements);
@@ -132,13 +136,19 @@ public final class SSource extends AStatement {
         String classInterfaces[] = reserved.usesScore() ? new String[] { WriterConstants.NEEDS_SCORE_TYPE.getInternalName() } : null;
 
         ClassWriter writer = new ClassWriter(classFrames);
-        writer.visit(classVersion, classAccess, className, null, classBase, classInterfaces);
+        ClassVisitor visitor = writer;
-        writer.visitSource(Location.computeSourceName(name, source), null);
+        
+        if (debugStream != null) {
+            visitor = new TraceClassVisitor(visitor, debugStream, null);
+        }
+        visitor.visit(classVersion, classAccess, className, null, classBase, classInterfaces);
+        visitor.visitSource(Location.computeSourceName(name, source), null);
 
         expressions = new BitSet(source.length());
 
         // Write the constructor:
-        MethodWriter constructor = new MethodWriter(Opcodes.ACC_PUBLIC, CONSTRUCTOR, writer, expressions);
+        MethodWriter constructor = new MethodWriter(Opcodes.ACC_PUBLIC, CONSTRUCTOR, visitor, expressions);
+        constructor.visitCode();
         constructor.loadThis();
         constructor.loadArgs();
         constructor.invokeConstructor(org.objectweb.asm.Type.getType(Executable.class), CONSTRUCTOR);
@@ -146,20 +156,21 @@ public final class SSource extends AStatement {
         constructor.endMethod();
 
         // Write the execute method:
-        MethodWriter execute = new MethodWriter(Opcodes.ACC_PUBLIC, EXECUTE, writer, expressions);
+        MethodWriter execute = new MethodWriter(Opcodes.ACC_PUBLIC, EXECUTE, visitor, expressions);
+        execute.visitCode();
         write(execute);
         execute.endMethod();
         
         // Write all functions:
         for (SFunction function : functions) {
-            function.write(writer, expressions);
+            function.write(visitor, expressions);
         }
 
         // Write the constants
         if (false == locals.getConstants().isEmpty()) {
             // Fields
             for (Constant constant : locals.getConstants()) {
-                writer.visitField(
+                visitor.visitField(
                         Opcodes.ACC_FINAL | Opcodes.ACC_PRIVATE | Opcodes.ACC_STATIC,
                         constant.name,
                         constant.type.getDescriptor(),
@@ -169,7 +180,7 @@ public final class SSource extends AStatement {
 
             // Initialize the constants in a static initializer
             final MethodWriter clinit = new MethodWriter(Opcodes.ACC_STATIC, 
-                    WriterConstants.CLINIT, writer, expressions);
+                    WriterConstants.CLINIT, visitor, expressions);
             for (Constant constant : locals.getConstants()) {
                 constant.initializer.accept(clinit);
                 clinit.putStatic(CLASS_TYPE, constant.name, constant.type);
@@ -180,7 +191,7 @@ public final class SSource extends AStatement {
         
         // End writing the class and store the generated bytes.
 
-        writer.visitEnd();
+        visitor.visitEnd();
         bytes = writer.toByteArray();
     }
     

modules/lang-painless/src/test/java/org/elasticsearch/painless/Debugger.java

@@ -19,8 +19,8 @@
 
 package org.elasticsearch.painless;
 
-import org.objectweb.asm.ClassReader;
+import org.apache.lucene.util.IOUtils;
-import org.objectweb.asm.util.TraceClassVisitor;
+import org.objectweb.asm.util.Textifier;
 
 import java.io.PrintWriter;
 import java.io.StringWriter;
@@ -35,14 +35,19 @@ final class Debugger {
 
     /** compiles to bytecode, and returns debugging output */
     static String toString(String source, CompilerSettings settings) {
-        final byte[] bytes = Compiler.compile("<debugging>", source, settings);
+        StringWriter output = new StringWriter();
-        final StringWriter output = new StringWriter();
+        PrintWriter outputWriter = new PrintWriter(output);
-        final PrintWriter outputWriter = new PrintWriter(output);
+        Textifier textifier = new Textifier();
-        final ClassReader reader = new ClassReader(bytes);
+        try {
-
+            Compiler.compile("<debugging>", source, settings, textifier);
-        reader.accept(new TraceClassVisitor(outputWriter), 0);
+        } catch (Exception e) {
-        outputWriter.flush();
+            textifier.print(outputWriter);
+            e.addSuppressed(new Exception("current bytecode: \n" + output));
+            IOUtils.reThrowUnchecked(e);
+            throw new AssertionError();
+        }
         
+        textifier.print(outputWriter);
         return output.toString();
     }
 }