From 7bbb7cf3ca25cb8727a6ec226de1ed1fc5bf47e9 Mon Sep 17 00:00:00 2001 From: Reinier Zwitserloot Date: Tue, 28 May 2013 13:03:54 +0200 Subject: Fixes for issue 470: VerifyErrors when using @SneakyThrows. --- .../bytecode/PreventNullAnalysisRemover.java | 2 +- src/core/lombok/bytecode/SneakyThrowsRemover.java | 115 ++++++++++++++++++++- src/core/lombok/core/PostCompiler.java | 4 +- src/core/lombok/eclipse/EclipseAST.java | 15 +-- src/core/lombok/eclipse/EclipseAstProblemView.java | 9 +- src/core/lombok/eclipse/TransformEclipseAST.java | 2 +- .../eclipse/handlers/EclipseHandlerUtil.java | 2 +- .../lombok/eclipse/agent/EclipsePatcher.java | 2 +- .../lombok/eclipse/agent/PatchFixes.java | 7 +- 9 files changed, 130 insertions(+), 28 deletions(-) (limited to 'src') diff --git a/src/core/lombok/bytecode/PreventNullAnalysisRemover.java b/src/core/lombok/bytecode/PreventNullAnalysisRemover.java index 3342eacb..751e691f 100644 --- a/src/core/lombok/bytecode/PreventNullAnalysisRemover.java +++ b/src/core/lombok/bytecode/PreventNullAnalysisRemover.java @@ -44,7 +44,7 @@ public class PreventNullAnalysisRemover implements PostCompilerTransformation { byte[] fixedByteCode = fixJSRInlining(original); ClassReader reader = new ClassReader(fixedByteCode); - ClassWriter writer = new FixedClassWriter(reader, ClassWriter.COMPUTE_MAXS | ClassWriter.COMPUTE_FRAMES); + ClassWriter writer = new FixedClassWriter(reader, 0); final AtomicBoolean changesMade = new AtomicBoolean(); diff --git a/src/core/lombok/bytecode/SneakyThrowsRemover.java b/src/core/lombok/bytecode/SneakyThrowsRemover.java index c54495d4..914c313a 100644 --- a/src/core/lombok/bytecode/SneakyThrowsRemover.java +++ b/src/core/lombok/bytecode/SneakyThrowsRemover.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010-2012 The Project Lombok Authors. + * Copyright (C) 2010-2013 The Project Lombok Authors. * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -32,19 +32,21 @@ import org.mangosdk.spi.ProviderFor; import org.objectweb.asm.ClassReader; import org.objectweb.asm.ClassVisitor; import org.objectweb.asm.ClassWriter; +import org.objectweb.asm.Handle; +import org.objectweb.asm.Label; import org.objectweb.asm.MethodVisitor; import org.objectweb.asm.Opcodes; @ProviderFor(PostCompilerTransformation.class) public class SneakyThrowsRemover implements PostCompilerTransformation { - @Override public byte[] applyTransformations(byte[] original, String fileName, DiagnosticsReceiver diagnostics) { + @Override public byte[] applyTransformations(byte[] original, String fileName, final DiagnosticsReceiver diagnostics) { if (!new ClassFileMetaData(original).usesMethod("lombok/Lombok", "sneakyThrow")) return null; byte[] fixedByteCode = fixJSRInlining(original); ClassReader reader = new ClassReader(fixedByteCode); - ClassWriter writer = new FixedClassWriter(reader, ClassWriter.COMPUTE_MAXS | ClassWriter.COMPUTE_FRAMES); + ClassWriter writer = new ClassWriter(reader, 0); final AtomicBoolean changesMade = new AtomicBoolean(); @@ -53,6 +55,8 @@ public class SneakyThrowsRemover implements PostCompilerTransformation { super(Opcodes.ASM4, mv); } + private boolean methodInsnQueued = false; + @Override public void visitMethodInsn(int opcode, String owner, String name, String desc) { if ( opcode == Opcodes.INVOKESTATIC && @@ -60,16 +64,117 @@ public class SneakyThrowsRemover implements PostCompilerTransformation { "lombok/Lombok".equals(owner) && "(Ljava/lang/Throwable;)Ljava/lang/RuntimeException;".equals(desc)) { - changesMade.set(true); if (System.getProperty("lombok.debugAsmOnly", null) != null) { super.visitMethodInsn(opcode, owner, name, desc); // DEBUG for issue 470! } else { - super.visitInsn(Opcodes.ATHROW); + methodInsnQueued = true; } } else { super.visitMethodInsn(opcode, owner, name, desc); } } + + private void handleQueue() { + if (!methodInsnQueued) return; + super.visitMethodInsn(Opcodes.INVOKESTATIC, "lombok/Lombok", "sneakyThrow", "(Ljava/lang/Throwable;)Ljava/lang/RuntimeException;"); + methodInsnQueued = false; + diagnostics.addWarning("Proper usage is: throw lombok.Lombok.sneakyThrow(someException);. You did not 'throw' it. Because of this, the call to sneakyThrow " + + "remains in your classfile and you will need lombok.jar on the classpath at runtime."); + } + + @Override public void visitInsn(int arg0) { + if (methodInsnQueued && arg0 == Opcodes.ATHROW) { + changesMade.set(true); + // As expected, the required ATHROW. We can now safely 'eat' the previous call. + methodInsnQueued = false; + } + handleQueue(); + super.visitInsn(arg0); + } + @Override public void visitFrame(int arg0, int arg1, Object[] arg2, int arg3, Object[] arg4) { + handleQueue(); + super.visitFrame(arg0, arg1, arg2, arg3, arg4); + } + + @Override public void visitIincInsn(int arg0, int arg1) { + handleQueue(); + super.visitIincInsn(arg0, arg1); + } + + @Override public void visitFieldInsn(int arg0, String arg1, String arg2, String arg3) { + handleQueue(); + super.visitFieldInsn(arg0, arg1, arg2, arg3); + } + + @Override public void visitIntInsn(int arg0, int arg1) { + handleQueue(); + super.visitIntInsn(arg0, arg1); + } + + @Override public void visitEnd() { + handleQueue(); + super.visitEnd(); + } + + @Override public void visitInvokeDynamicInsn(String arg0, String arg1, Handle arg2, Object... arg3) { + handleQueue(); + super.visitInvokeDynamicInsn(arg0, arg1, arg2, arg3); + } + + @Override public void visitLabel(Label arg0) { + handleQueue(); + super.visitLabel(arg0); + } + + @Override public void visitJumpInsn(int arg0, Label arg1) { + handleQueue(); + super.visitJumpInsn(arg0, arg1); + } + + @Override public void visitLdcInsn(Object arg0) { + handleQueue(); + super.visitLdcInsn(arg0); + } + + @Override public void visitLocalVariable(String arg0, String arg1, String arg2, Label arg3, Label arg4, int arg5) { + handleQueue(); + super.visitLocalVariable(arg0, arg1, arg2, arg3, arg4, arg5); + } + + @Override public void visitMaxs(int arg0, int arg1) { + handleQueue(); + super.visitMaxs(arg0, arg1); + } + + @Override public void visitLookupSwitchInsn(Label arg0, int[] arg1, Label[] arg2) { + handleQueue(); + super.visitLookupSwitchInsn(arg0, arg1, arg2); + } + + @Override public void visitMultiANewArrayInsn(String arg0, int arg1) { + handleQueue(); + super.visitMultiANewArrayInsn(arg0, arg1); + } + + @Override public void visitVarInsn(int arg0, int arg1) { + handleQueue(); + super.visitVarInsn(arg0, arg1); + } + + @Override public void visitTryCatchBlock(Label arg0, Label arg1, Label arg2, String arg3) { + handleQueue(); + super.visitTryCatchBlock(arg0, arg1, arg2, arg3); + } + + @Override public void visitTableSwitchInsn(int arg0, int arg1, Label arg2, Label... arg3) { + handleQueue(); + super.visitTableSwitchInsn(arg0, arg1, arg2, arg3); + } + + @Override public void visitTypeInsn(int arg0, String arg1) { + handleQueue(); + super.visitTypeInsn(arg0, arg1); + } } reader.accept(new ClassVisitor(Opcodes.ASM4, writer) { diff --git a/src/core/lombok/core/PostCompiler.java b/src/core/lombok/core/PostCompiler.java index 1e40cca2..4a099862 100644 --- a/src/core/lombok/core/PostCompiler.java +++ b/src/core/lombok/core/PostCompiler.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010 The Project Lombok Authors. + * Copyright (C) 2010-2013 The Project Lombok Authors. * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -33,6 +33,7 @@ public final class PostCompiler { private static List transformations; public static byte[] applyTransformations(byte[] original, String fileName, DiagnosticsReceiver diagnostics) { + if (System.getProperty("lombok.disablePostCompiler", null) != null) return original; init(diagnostics); byte[] previous = original; for (PostCompilerTransformation transformation : transformations) { @@ -59,6 +60,7 @@ public final class PostCompiler { } public static OutputStream wrapOutputStream(final OutputStream originalStream, final String fileName, final DiagnosticsReceiver diagnostics) throws IOException { + if (System.getProperty("lombok.disablePostCompiler", null) != null) return originalStream; return new ByteArrayOutputStream() { @Override public void close() throws IOException { // no need to call super diff --git a/src/core/lombok/eclipse/EclipseAST.java b/src/core/lombok/eclipse/EclipseAST.java index 612dcff7..eed3c0b6 100644 --- a/src/core/lombok/eclipse/EclipseAST.java +++ b/src/core/lombok/eclipse/EclipseAST.java @@ -24,6 +24,7 @@ package lombok.eclipse; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.ArrayList; +import org.eclipse.jdt.internal.compiler.CompilationResult; import java.util.Collection; import java.util.Collections; import java.util.List; @@ -123,7 +124,8 @@ public class EclipseAST extends AST { } void addToCompilationResult() { - addProblemToCompilationResult((CompilationUnitDeclaration) top().get(), + CompilationUnitDeclaration cud = (CompilationUnitDeclaration) top().get(); + addProblemToCompilationResult(cud.getFileName(), cud.compilationResult, isWarning, message, sourceStart, sourceEnd); } } @@ -147,11 +149,10 @@ public class EclipseAST extends AST { * Adds a problem to the provided CompilationResult object so that it will show up * in the Problems/Warnings view. */ - public static void addProblemToCompilationResult(CompilationUnitDeclaration ast, + public static void addProblemToCompilationResult(char[] fileNameArray, CompilationResult result, boolean isWarning, String message, int sourceStart, int sourceEnd) { - if (ast.compilationResult == null) return; try { - EcjReflectionCheck.addProblemToCompilationResult.invoke(null, ast, isWarning, message, sourceStart, sourceEnd); + EcjReflectionCheck.addProblemToCompilationResult.invoke(null, fileNameArray, result, isWarning, message, sourceStart, sourceEnd); } catch (NoClassDefFoundError e) { //ignore, we don't have access to the correct ECJ classes, so lombok can't possibly //do anything useful here. @@ -168,7 +169,7 @@ public class EclipseAST extends AST { //do anything useful here. } } - + private final CompilationUnitDeclaration compilationUnitDeclaration; private boolean completeParse; @@ -358,7 +359,7 @@ public class EclipseAST extends AST { } private static class EcjReflectionCheck { - private static final String CUD_TYPE = "org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration"; + private static final String COMPILATIONRESULT_TYPE = "org.eclipse.jdt.internal.compiler.CompilationResult"; public static Method addProblemToCompilationResult; public static final Throwable problem; @@ -367,7 +368,7 @@ public class EclipseAST extends AST { Throwable problem_ = null; Method m = null; try { - m = EclipseAstProblemView.class.getMethod("addProblemToCompilationResult", Class.forName(CUD_TYPE), boolean.class, String.class, int.class, int.class); + m = EclipseAstProblemView.class.getMethod("addProblemToCompilationResult", char[].class, Class.forName(COMPILATIONRESULT_TYPE), boolean.class, String.class, int.class, int.class); } catch (Throwable t) { // That's problematic, but as long as no local classes are used we don't actually need it. // Better fail on local classes than crash altogether. diff --git a/src/core/lombok/eclipse/EclipseAstProblemView.java b/src/core/lombok/eclipse/EclipseAstProblemView.java index a2d5b833..c1179666 100644 --- a/src/core/lombok/eclipse/EclipseAstProblemView.java +++ b/src/core/lombok/eclipse/EclipseAstProblemView.java @@ -3,7 +3,6 @@ package lombok.eclipse; import org.eclipse.jdt.core.compiler.CategorizedProblem; import org.eclipse.jdt.internal.compiler.CompilationResult; -import org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration; import org.eclipse.jdt.internal.compiler.problem.DefaultProblem; import org.eclipse.jdt.internal.compiler.problem.ProblemSeverities; import org.eclipse.jdt.internal.compiler.util.Util; @@ -13,14 +12,12 @@ public class EclipseAstProblemView { * Adds a problem to the provided CompilationResult object so that it will show up * in the Problems/Warnings view. */ - public static void addProblemToCompilationResult(CompilationUnitDeclaration ast, + public static void addProblemToCompilationResult(char[] fileNameArray, CompilationResult result, boolean isWarning, String message, int sourceStart, int sourceEnd) { - if (ast.compilationResult == null) return; - char[] fileNameArray = ast.getFileName(); + if (result == null) return; if (fileNameArray == null) fileNameArray = "(unknown).java".toCharArray(); int lineNumber = 0; int columnNumber = 1; - CompilationResult result = ast.compilationResult; int[] lineEnds = null; lineNumber = sourceStart >= 0 ? Util.getLineNumber(sourceStart, lineEnds = result.getLineSeparatorPositions(), 0, lineEnds.length-1) @@ -33,7 +30,7 @@ public class EclipseAstProblemView { fileNameArray, message, 0, new String[0], isWarning ? ProblemSeverities.Warning : ProblemSeverities.Error, sourceStart, sourceEnd, lineNumber, columnNumber); - ast.compilationResult.record(ecProblem, null); + result.record(ecProblem, null); } private static class LombokProblem extends DefaultProblem { diff --git a/src/core/lombok/eclipse/TransformEclipseAST.java b/src/core/lombok/eclipse/TransformEclipseAST.java index 47e620f6..11caf5c2 100644 --- a/src/core/lombok/eclipse/TransformEclipseAST.java +++ b/src/core/lombok/eclipse/TransformEclipseAST.java @@ -144,7 +144,7 @@ public class TransformEclipseAST { try { String message = "Lombok can't parse this source: " + t.toString(); - EclipseAST.addProblemToCompilationResult(ast, false, message, 0, 0); + EclipseAST.addProblemToCompilationResult(ast.getFileName(), ast.compilationResult, false, message, 0, 0); t.printStackTrace(); } catch (Throwable t2) { try { diff --git a/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java b/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java index d47b6715..7703336f 100644 --- a/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java +++ b/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java @@ -142,7 +142,7 @@ public class EclipseHandlerUtil { } catch (NoClassDefFoundError e) { //standalone ecj does not jave Platform, ILog, IStatus, and friends. new TerminalLogger().error(message, bundleName, error); } - if (cud != null) EclipseAST.addProblemToCompilationResult(cud, false, message + " - See error log.", 0, 0); + if (cud != null) EclipseAST.addProblemToCompilationResult(cud.getFileName(), cud.compilationResult, false, message + " - See error log.", 0, 0); } /** diff --git a/src/eclipseAgent/lombok/eclipse/agent/EclipsePatcher.java b/src/eclipseAgent/lombok/eclipse/agent/EclipsePatcher.java index f9b53e68..8c6011bc 100644 --- a/src/eclipseAgent/lombok/eclipse/agent/EclipsePatcher.java +++ b/src/eclipseAgent/lombok/eclipse/agent/EclipsePatcher.java @@ -267,7 +267,7 @@ public class EclipsePatcher extends Agent { .build()); } - + private static void patchPostCompileHookEclipse(ScriptManager sm) { sm.addScript(ScriptBuilder.wrapMethodCall() .target(new MethodTarget("org.eclipse.jdt.internal.core.builder.IncrementalImageBuilder", "writeClassFileContents")) diff --git a/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java b/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java index cb3ee817..0002e26e 100644 --- a/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java +++ b/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010-2012 The Project Lombok Authors. + * Copyright (C) 2010-2013 The Project Lombok Authors. * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -329,19 +329,16 @@ public class PatchFixes { return newSimpleNames; } - public static byte[] runPostCompiler(byte[] bytes, String fileName) { - if (System.getProperty("lombok.disablePostCompiler", null) != null) return bytes; + public static byte[] runPostCompiler(byte[] bytes, String fileName) { byte[] transformed = PostCompiler.applyTransformations(bytes, fileName, DiagnosticsReceiver.CONSOLE); return transformed == null ? bytes : transformed; } public static OutputStream runPostCompiler(OutputStream out) throws IOException { - if (System.getProperty("lombok.disablePostCompiler", null) != null) return out; return PostCompiler.wrapOutputStream(out, "TEST", DiagnosticsReceiver.CONSOLE); } public static BufferedOutputStream runPostCompiler(BufferedOutputStream out, String path, String name) throws IOException { - if (System.getProperty("lombok.disablePostCompiler", null) != null) return out; String fileName = path + "/" + name; return new BufferedOutputStream(PostCompiler.wrapOutputStream(out, fileName, DiagnosticsReceiver.CONSOLE)); } -- cgit