From e5492bf606d2054122e5a79e6ae9306da07f0742 Mon Sep 17 00:00:00 2001 From: chibash Date: Sun, 11 Aug 2013 21:52:08 +0900 Subject: [PATCH] fixed JASSIST-207 --- Readme.html | 2 +- .../bytecode/stackmap/BasicBlock.java | 13 ++-- .../javassist/bytecode/stackmap/MapMaker.java | 69 +++++++++++-------- .../javassist/bytecode/stackmap/Tracer.java | 2 +- src/test/Test.java | 16 +++++ src/test/javassist/JvstTest4.java | 16 +++++ src/test/javassist/bytecode/StackMapTest.java | 20 +++--- src/test/test4/JIRA207.java | 51 ++++++++++++++ 8 files changed, 144 insertions(+), 45 deletions(-) create mode 100644 src/test/Test.java create mode 100644 src/test/test4/JIRA207.java diff --git a/Readme.html b/Readme.html index 9820e64b..05386252 100644 --- a/Readme.html +++ b/Readme.html @@ -283,7 +283,7 @@ see javassist.Dump.

-version 3.19

diff --git a/src/main/javassist/bytecode/stackmap/BasicBlock.java b/src/main/javassist/bytecode/stackmap/BasicBlock.java index 6213e22a..43367307 100644 --- a/src/main/javassist/bytecode/stackmap/BasicBlock.java +++ b/src/main/javassist/bytecode/stackmap/BasicBlock.java @@ -97,6 +97,10 @@ public class BasicBlock { sbuf.append("}"); } + /** + * A Mark indicates the position of a branch instruction + * or a branch target. + */ static class Mark implements Comparable { int position; BasicBlock block; @@ -349,10 +353,11 @@ public class BasicBlock { } else { // the previous mark already has exits. - int prevPos = prev.position; - if (prevPos + prev.length < m.position) { - prev = makeBlock(prevPos + prev.length); - prev.length = m.position - prevPos; + if (prev.position + prev.length < m.position) { + // dead code is found. + prev = makeBlock(prev.position + prev.length); + blocks.add(prev); + prev.length = m.position - prev.position; // the incoming flow from dead code is not counted // bb.incoming++; prev.exit = makeArray(bb); diff --git a/src/main/javassist/bytecode/stackmap/MapMaker.java b/src/main/javassist/bytecode/stackmap/MapMaker.java index 16d3f294..b1aa7008 100644 --- a/src/main/javassist/bytecode/stackmap/MapMaker.java +++ b/src/main/javassist/bytecode/stackmap/MapMaker.java @@ -307,34 +307,63 @@ public class MapMaker extends Tracer { // Phase 1.5 /* - * Javac may generate an exception handler that catches only an exception + * Javac may generate an exception handler that catches only the exception * thrown within the handler itself. It is dead code. + * See javassist.JvstTest4.testJIRA195(). */ private void findDeadCatchers(byte[] code, TypedBlock[] blocks) throws BadBytecode { int len = blocks.length; for (int i = 0; i < len; i++) { TypedBlock block = blocks[i]; - if (block.localsTypes == null) { // if block is dead code + if (!block.alreadySet()) { + fixDeadcode(code, block); BasicBlock.Catch handler = block.toCatch; - while (handler != null) - if (handler.body == block) { - BasicBlock.Catch handler2 - = new BasicBlock.Catch(block, handler.typeIndex, null); - traceException(code, handler2); - break; + if (handler != null) { + TypedBlock tb = (TypedBlock)handler.body; + if (!tb.alreadySet()) { + // tb is a handler that catches only the exceptions + // thrown from dead code. + recordStackMap(tb, handler.typeIndex); + fixDeadcode(code, tb); + tb.incoming = 1; } - else - handler = handler.next; + } + } } } + private void fixDeadcode(byte[] code, TypedBlock block) throws BadBytecode { + int pos = block.position; + int len = block.length - 3; + if (len < 0) { + // if the dead-code length is shorter than 3 bytes. + if (len == -1) + code[pos] = Bytecode.NOP; + + code[pos + block.length - 1] = (byte)Bytecode.ATHROW; + block.incoming = 1; + recordStackMap(block, 0); + return; + } + + // if block.incomping > 0, all the incoming edges are from + // other dead code blocks. So set block.incoming to 0. + block.incoming = 0; + + for (int k = 0; k < len; k++) + code[pos + k] = Bytecode.NOP; + + code[pos + len] = (byte)Bytecode.GOTO; + ByteArray.write16bit(-len, code, pos + len + 1); + } + // Phase 2 /* * This method first finds strongly connected components (SCCs) - * on a graph made by TypeData by Tarjan's algorithm. + * in a TypeData graph by Tarjan's algorithm. * SCCs are TypeData nodes sharing the same type. * Since SCCs are found in the topologically sorted order, * their types are also fixed when they are found. @@ -345,9 +374,7 @@ public class MapMaker extends Tracer { int index = 0; for (int i = 0; i < len; i++) { TypedBlock block = blocks[i]; - if (block.localsTypes == null) // if block is dead code - fixDeadcode(code, block); - else { + if (block.alreadySet()) { // if block is not dead code int n = block.localsTypes.length; for (int j = 0; j < n; j++) index = block.localsTypes[j].dfs(preOrder, index, classPool); @@ -359,20 +386,6 @@ public class MapMaker extends Tracer { } } - private void fixDeadcode(byte[] code, TypedBlock block) throws BadBytecode { - int pos = block.position; - int len = block.length - 3; - if (len < 0) - throw new BadBytecode("dead code detected at " + pos - + ". No stackmap table generated."); - - for (int k = 0; k < len; k++) - code[pos + k] = Bytecode.NOP; - - code[pos + len] = (byte)Bytecode.GOTO; - ByteArray.write16bit(-len, code, pos + len + 1); - } - // Phase 3 public StackMapTable toStackMap(TypedBlock[] blocks) { diff --git a/src/main/javassist/bytecode/stackmap/Tracer.java b/src/main/javassist/bytecode/stackmap/Tracer.java index e5216165..4111d589 100644 --- a/src/main/javassist/bytecode/stackmap/Tracer.java +++ b/src/main/javassist/bytecode/stackmap/Tracer.java @@ -81,7 +81,7 @@ public abstract class Tracer implements TypeTag { return doOpcode148_201(pos, code, op); } catch (ArrayIndexOutOfBoundsException e) { - throw new BadBytecode("inconsistent stack height " + e.getMessage()); + throw new BadBytecode("inconsistent stack height " + e.getMessage(), e); } } diff --git a/src/test/Test.java b/src/test/Test.java new file mode 100644 index 00000000..51591cf9 --- /dev/null +++ b/src/test/Test.java @@ -0,0 +1,16 @@ +import javassist.*; + +public class Test { + public static void main(String[] args) throws Exception { + ClassPool cp = ClassPool.getDefault(); + // ClassPool cp = new ClassPool(); + cp.insertClassPath("./target/test-classes"); + CtClass cc = cp.get("test4.JIRA207"); + // cc.getClassFile().setMajorVersion(javassist.bytecode.ClassFile.JAVA_4); + CtMethod cm = cc.getDeclaredMethod("foo"); + cm.insertBefore("throw new Exception();"); + CtMethod cm2 = cc.getDeclaredMethod("run2"); + cm2.insertBefore("throw new Exception();"); + cc.writeFile(); + } +} diff --git a/src/test/javassist/JvstTest4.java b/src/test/javassist/JvstTest4.java index b3b04c29..85ee4b0f 100644 --- a/src/test/javassist/JvstTest4.java +++ b/src/test/javassist/JvstTest4.java @@ -903,4 +903,20 @@ public class JvstTest4 extends JvstTestRoot { Object obj = make(cc.getName()); assertEquals(15, invoke(obj, "run")); } + + public void testJIRA207() throws Exception { + CtClass cc = sloader.get("test4.JIRA207"); + CtMethod cm = cc.getDeclaredMethod("foo"); + cm.insertBefore("throw new Exception();"); + + CtMethod cm2 = cc.getDeclaredMethod("run2"); + cm2.insertBefore("throw new Exception();"); + cc.writeFile(); + Object obj = make(cc.getName()); + try { + invoke(obj, "run2"); + fail("run2"); + } + catch (Exception e) {} + } } diff --git a/src/test/javassist/bytecode/StackMapTest.java b/src/test/javassist/bytecode/StackMapTest.java index 4fadc890..fc54c592 100644 --- a/src/test/javassist/bytecode/StackMapTest.java +++ b/src/test/javassist/bytecode/StackMapTest.java @@ -680,18 +680,16 @@ public class StackMapTest extends TestCase { public void testJIRA175b() throws Exception { CtClass cc = loader.get("javassist.bytecode.StackMapTest$C6"); - try { - cc.getDeclaredMethod("setter").instrument(new javassist.expr.ExprEditor() { - public void edit(javassist.expr.FieldAccess f) throws javassist.CannotCompileException { - if (!f.where().getMethodInfo().isMethod()) - return; + cc.getDeclaredMethod("setter").instrument(new javassist.expr.ExprEditor() { + public void edit(javassist.expr.FieldAccess f) throws javassist.CannotCompileException { + if (!f.where().getMethodInfo().isMethod()) + return; - f.replace("{ $_ = $proceed($$); return $_;}"); - } - }); - fail("deadcode detection"); - } - catch (javassist.CannotCompileException e) {} + // this will make a 1-byte dead code. + f.replace("{ $_ = $proceed($$); return $_;}"); + } + }); + cc.writeFile(); } public static class C6 { diff --git a/src/test/test4/JIRA207.java b/src/test/test4/JIRA207.java new file mode 100644 index 00000000..c0e6752e --- /dev/null +++ b/src/test/test4/JIRA207.java @@ -0,0 +1,51 @@ +package test4; + +public class JIRA207 { + public int run() { + int i = 3; + return foo(i); + } + + public int foo(int i) { + int k = i + 3; + if (k > 0) + return k * k; + else + return k; + } + + public int run2() { + int i = 0; + int p = i; + int q = p; + int r = q; + for (int k = 1; k < 3; ++k) + p += k; + + for (int k = 3; k > 0; --k) + try { + foo(k); + p++; + } + finally { + p++; + } + + try { + foo(p); + } + catch (RuntimeException e) { + if (p > 0) + throw e; + } + + switch (p) { + case 1: + p = 100; + break; + default : + ++p; + } + return p + r; + } +} -- 2.39.5