diff --git a/asm-tree/src/main/java/org/objectweb/asm/tree/InsnList.java b/asm-tree/src/main/java/org/objectweb/asm/tree/InsnList.java index 8fe315b510bb514e5860d999bf8b55ff5f47a236..67769a4390b02a202a918af92b8f861670c59d42 100644 --- a/asm-tree/src/main/java/org/objectweb/asm/tree/InsnList.java +++ b/asm-tree/src/main/java/org/objectweb/asm/tree/InsnList.java @@ -222,6 +222,7 @@ public class InsnList { * @param insnNode an instruction, which must not belong to any {@link InsnList}. */ public void add(final AbstractInsnNode insnNode) { + ensureNotOwned(insnNode); ++size; if (lastInsn == null) { firstInsn = insnNode; @@ -475,6 +476,20 @@ public class InsnList { } } + /** Ensures that the given node does not already belong to an InsnList */ + private void ensureNotOwned(AbstractInsnNode insnNode) { + if (insnNode.previousInsn != null || insnNode.nextInsn != null || insnNode.index != -1) { + // Adding an instruction that still refers to others (in the same or another InsnList) + // leads to hard to debug bugs. + // Initially everything may look ok (e.g. iteration follows `next` thus a stale `prev` + // isn't noticed). However, a stale link brings the doubly-linked into disarray + // e.g. upon removing an element, which results in the `next` of a stale `prev` being + // updated, among other failure scenarios. Better fail early. + throw new IllegalArgumentException( + "Instruction " + insnNode + " belongs to another InsnList."); + } + } + // Note: this class is not generified because it would create bridges. @SuppressWarnings("rawtypes") private final class InsnListIterator implements ListIterator { diff --git a/asm-tree/src/test/java/org/objectweb/asm/tree/InsnListTest.java b/asm-tree/src/test/java/org/objectweb/asm/tree/InsnListTest.java index a89d46501ac4327a520e8514feae61c06d0b01e2..96f7199c581a4ed59d245747aa3e226bf2fb28a3 100644 --- a/asm-tree/src/test/java/org/objectweb/asm/tree/InsnListTest.java +++ b/asm-tree/src/test/java/org/objectweb/asm/tree/InsnListTest.java @@ -54,6 +54,8 @@ public class InsnListTest { private InsnList list2; + private InsnList list3Unchecked; + private InsnNode insn1; private InsnNode insn2; @@ -66,6 +68,7 @@ public class InsnListTest { insn2 = new InsnNode(0); list2.add(insn1); list2.add(insn2); + list3Unchecked = new InsnList(); } void assertEqualInsnArrays(final AbstractInsnNode[] expected, final AbstractInsnNode[] value) { @@ -747,6 +750,11 @@ public class InsnListTest { assertNotSame(label, labelNode.getLabel()); } + @Test + public void testAddNodeAssociatedWithAnotherList() { + assertThrows(IllegalArgumentException.class, () -> list3Unchecked.add(insn1)); + } + /** An InsnList which checks that its methods are properly used. */ static class CheckedInsnList extends InsnList {