From 5d8dbd432e306f5a1b6b08e495af43180c9124f4 Mon Sep 17 00:00:00 2001 From: Eric Bruneton Date: Fri, 27 May 2022 08:08:48 +0200 Subject: [PATCH 1/2] Make sure to check all Label references. --- .../asm/util/CheckMethodAdapter.java | 39 ++++++++----------- .../asm/util/CheckMethodAdapterTest.java | 2 +- 2 files changed, 17 insertions(+), 24 deletions(-) diff --git a/asm-util/src/main/java/org/objectweb/asm/util/CheckMethodAdapter.java b/asm-util/src/main/java/org/objectweb/asm/util/CheckMethodAdapter.java index 08e8961fd..0a27ff43e 100644 --- a/asm-util/src/main/java/org/objectweb/asm/util/CheckMethodAdapter.java +++ b/asm-util/src/main/java/org/objectweb/asm/util/CheckMethodAdapter.java @@ -30,7 +30,6 @@ package org.objectweb.asm.util; import java.io.PrintWriter; import java.io.StringWriter; import java.util.ArrayList; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -784,9 +783,8 @@ public class CheckMethodAdapter extends MethodVisitor { checkVisitCodeCalled(); checkVisitMaxsNotCalled(); checkOpcodeMethod(opcode, Method.VISIT_JUMP_INSN); - checkLabel(label, false, "label"); + checkLabel(label, /* checkVisited = */ false, "label"); super.visitJumpInsn(opcode, label); - referencedLabels.add(label); ++insnCount; } @@ -794,7 +792,7 @@ public class CheckMethodAdapter extends MethodVisitor { public void visitLabel(final Label label) { checkVisitCodeCalled(); checkVisitMaxsNotCalled(); - checkLabel(label, false, "label"); + checkLabel(label, /* checkVisited = */ false, "label"); if (labelInsnIndices.get(label) != null) { throw new IllegalStateException("Already visited label"); } @@ -830,15 +828,14 @@ public class CheckMethodAdapter extends MethodVisitor { throw new IllegalArgumentException( "Max = " + max + " must be greater than or equal to min = " + min); } - checkLabel(dflt, false, "default label"); + checkLabel(dflt, /* checkVisited = */ false, "default label"); if (labels == null || labels.length != max - min + 1) { throw new IllegalArgumentException("There must be max - min + 1 labels"); } for (int i = 0; i < labels.length; ++i) { - checkLabel(labels[i], false, "label at index " + i); + checkLabel(labels[i], /* checkVisited = */ false, "label at index " + i); } super.visitTableSwitchInsn(min, max, dflt, labels); - Collections.addAll(referencedLabels, labels); ++insnCount; } @@ -846,16 +843,14 @@ public class CheckMethodAdapter extends MethodVisitor { public void visitLookupSwitchInsn(final Label dflt, final int[] keys, final Label[] labels) { checkVisitMaxsNotCalled(); checkVisitCodeCalled(); - checkLabel(dflt, false, "default label"); + checkLabel(dflt, /* checkVisited = */ false, "default label"); if (keys == null || labels == null || keys.length != labels.length) { throw new IllegalArgumentException("There must be the same number of keys and labels"); } for (int i = 0; i < labels.length; ++i) { - checkLabel(labels[i], false, "label at index " + i); + checkLabel(labels[i], /* checkVisited = */ false, "label at index " + i); } super.visitLookupSwitchInsn(dflt, keys, labels); - referencedLabels.add(dflt); - Collections.addAll(referencedLabels, labels); ++insnCount; } @@ -909,9 +904,9 @@ public class CheckMethodAdapter extends MethodVisitor { final Label start, final Label end, final Label handler, final String type) { checkVisitCodeCalled(); checkVisitMaxsNotCalled(); - checkLabel(start, false, START_LABEL); - checkLabel(end, false, END_LABEL); - checkLabel(handler, false, "handler label"); + checkLabel(start, /* checkVisited = */ false, START_LABEL); + checkLabel(end, /* checkVisited = */ false, END_LABEL); + checkLabel(handler, /* checkVisited = */ false, "handler label"); if (labelInsnIndices.get(start) != null || labelInsnIndices.get(end) != null || labelInsnIndices.get(handler) != null) { @@ -955,8 +950,8 @@ public class CheckMethodAdapter extends MethodVisitor { if (signature != null) { CheckClassAdapter.checkFieldSignature(signature); } - checkLabel(start, true, START_LABEL); - checkLabel(end, true, END_LABEL); + checkLabel(start, /* checkVisited = */ true, START_LABEL); + checkLabel(end, /* checkVisited = */ true, END_LABEL); checkUnsignedShort(index, INVALID_LOCAL_VARIABLE_INDEX); int startInsnIndex = labelInsnIndices.get(start).intValue(); int endInsnIndex = labelInsnIndices.get(end).intValue(); @@ -993,8 +988,8 @@ public class CheckMethodAdapter extends MethodVisitor { "Invalid start, end and index arrays (must be non null and of identical length"); } for (int i = 0; i < start.length; ++i) { - checkLabel(start[i], true, START_LABEL); - checkLabel(end[i], true, END_LABEL); + checkLabel(start[i], /* checkVisited = */ true, START_LABEL); + checkLabel(end[i], /* checkVisited = */ true, END_LABEL); checkUnsignedShort(index[i], INVALID_LOCAL_VARIABLE_INDEX); int startInsnIndex = labelInsnIndices.get(start[i]).intValue(); int endInsnIndex = labelInsnIndices.get(end[i]).intValue(); @@ -1012,7 +1007,7 @@ public class CheckMethodAdapter extends MethodVisitor { checkVisitCodeCalled(); checkVisitMaxsNotCalled(); checkUnsignedShort(line, "Invalid line number"); - checkLabel(start, true, START_LABEL); + checkLabel(start, /* checkVisited = */ true, START_LABEL); super.visitLineNumber(line, start); } @@ -1029,9 +1024,6 @@ public class CheckMethodAdapter extends MethodVisitor { for (int i = 0; i < handlers.size(); i += 2) { Integer startInsnIndex = labelInsnIndices.get(handlers.get(i)); Integer endInsnIndex = labelInsnIndices.get(handlers.get(i + 1)); - if (startInsnIndex == null || endInsnIndex == null) { - throw new IllegalStateException("Undefined try catch block labels"); - } if (endInsnIndex.intValue() <= startInsnIndex.intValue()) { throw new IllegalStateException("Emty try catch block handler range"); } @@ -1092,7 +1084,7 @@ public class CheckMethodAdapter extends MethodVisitor { if (value instanceof String) { checkInternalName(version, (String) value, "Invalid stack frame value"); } else if (value instanceof Label) { - referencedLabels.add((Label) value); + checkLabel((Label) value, /* checkVisited = */ false, "label"); } else { throw new IllegalArgumentException("Invalid stack frame value: " + value); } @@ -1452,6 +1444,7 @@ public class CheckMethodAdapter extends MethodVisitor { if (checkVisited && labelInsnIndices.get(label) == null) { throw new IllegalArgumentException(INVALID + message + " (must be visited first)"); } + referencedLabels.add(label); } static class MethodWriterWrapper extends MethodVisitor { diff --git a/asm-util/src/test/java/org/objectweb/asm/util/CheckMethodAdapterTest.java b/asm-util/src/test/java/org/objectweb/asm/util/CheckMethodAdapterTest.java index 9b3e48313..60d01d3e7 100644 --- a/asm-util/src/test/java/org/objectweb/asm/util/CheckMethodAdapterTest.java +++ b/asm-util/src/test/java/org/objectweb/asm/util/CheckMethodAdapterTest.java @@ -1060,7 +1060,7 @@ class CheckMethodAdapterTest extends AsmTest implements Opcodes { Executable visitMaxs = () -> checkMethodAdapter.visitMaxs(0, 0); Exception exception = assertThrows(IllegalStateException.class, visitMaxs); - assertEquals("Undefined try catch block labels", exception.getMessage()); + assertEquals("Undefined label used", exception.getMessage()); } @Test -- GitLab From 355fedd4fcfa654035381f2a9d252be59ff1e69b Mon Sep 17 00:00:00 2001 From: Eric Bruneton Date: Sat, 28 May 2022 15:12:49 +0200 Subject: [PATCH 2/2] Fix typo. --- .../main/java/org/objectweb/asm/util/CheckMethodAdapter.java | 2 +- .../java/org/objectweb/asm/util/CheckMethodAdapterTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/asm-util/src/main/java/org/objectweb/asm/util/CheckMethodAdapter.java b/asm-util/src/main/java/org/objectweb/asm/util/CheckMethodAdapter.java index 0a27ff43e..8b4ba7a92 100644 --- a/asm-util/src/main/java/org/objectweb/asm/util/CheckMethodAdapter.java +++ b/asm-util/src/main/java/org/objectweb/asm/util/CheckMethodAdapter.java @@ -1025,7 +1025,7 @@ public class CheckMethodAdapter extends MethodVisitor { Integer startInsnIndex = labelInsnIndices.get(handlers.get(i)); Integer endInsnIndex = labelInsnIndices.get(handlers.get(i + 1)); if (endInsnIndex.intValue() <= startInsnIndex.intValue()) { - throw new IllegalStateException("Emty try catch block handler range"); + throw new IllegalStateException("Empty try catch block handler range"); } } checkUnsignedShort(maxStack, "Invalid max stack"); diff --git a/asm-util/src/test/java/org/objectweb/asm/util/CheckMethodAdapterTest.java b/asm-util/src/test/java/org/objectweb/asm/util/CheckMethodAdapterTest.java index 60d01d3e7..6652d29be 100644 --- a/asm-util/src/test/java/org/objectweb/asm/util/CheckMethodAdapterTest.java +++ b/asm-util/src/test/java/org/objectweb/asm/util/CheckMethodAdapterTest.java @@ -1078,7 +1078,7 @@ class CheckMethodAdapterTest extends AsmTest implements Opcodes { Executable visitMaxs = () -> checkMethodAdapter.visitMaxs(0, 0); Exception exception = assertThrows(IllegalStateException.class, visitMaxs); - assertEquals("Emty try catch block handler range", exception.getMessage()); + assertEquals("Empty try catch block handler range", exception.getMessage()); } @Test -- GitLab