Commit bc33e850 authored by Lukas Rytz's avatar Lukas Rytz Committed by Jason Zaugg

Ensure instructions belong only to one list

Fail early when adding an instruction that already belongs to a
different instruction list.
parent c4a9e49c
......@@ -222,6 +222,7 @@ public class InsnList {
* @param insnNode an instruction, <i>which must not belong to any {@link InsnList}</i>.
*/
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 <tt>InsnList</tt> */
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 {
......
......@@ -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 {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment