Skip to content

Commit a05b4cb

Browse files
lycoris106darko-marinov
lycoris106
authored and
darko-marinov
committed
add fix to identityhashmap, add one test class for it
add author comment and newline minimize table size to increase failing probability for removeAll Fixed incompatibility with Java 8 by inlining outer class methods increase test iteration number
1 parent 7429cd2 commit a05b4cb

File tree

2 files changed

+320
-0
lines changed

2 files changed

+320
-0
lines changed

‎nondex-instrumentation/src/main/java/edu/illinois/nondex/instr/IdentityHashMapShufflingAdder.java‎

Lines changed: 262 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,13 +163,272 @@ public void addHasNext() {
163163
mv.visitEnd();
164164
}
165165

166+
public void addRemove() {
167+
MethodVisitor mv = super.visitMethod(Opcodes.ACC_PUBLIC, "remove", "()V", null, null);
168+
mv.visitCode();
169+
170+
mv.visitVarInsn(Opcodes.ALOAD, 0);
171+
mv.visitFieldInsn(Opcodes.GETFIELD, "java/util/IdentityHashMap$IdentityHashMapIterator", "lastReturnedIndex",
172+
"I");
173+
mv.visitInsn(Opcodes.ICONST_M1);
174+
Label l0 = new Label();
175+
mv.visitJumpInsn(Opcodes.IF_ICMPNE, l0);
176+
mv.visitTypeInsn(Opcodes.NEW, "java/lang/IllegalStateException");
177+
mv.visitInsn(Opcodes.DUP);
178+
mv.visitMethodInsn(Opcodes.INVOKESPECIAL, "java/lang/IllegalStateException", "<init>", "()V", false);
179+
mv.visitInsn(Opcodes.ATHROW);
180+
mv.visitLabel(l0);
181+
mv.visitFrame(Opcodes.F_SAME, 0, null, 0, null);
182+
mv.visitVarInsn(Opcodes.ALOAD, 0);
183+
mv.visitFieldInsn(Opcodes.GETFIELD, "java/util/IdentityHashMap$IdentityHashMapIterator", "this$0",
184+
"Ljava/util/IdentityHashMap;");
185+
mv.visitFieldInsn(Opcodes.GETFIELD, "java/util/IdentityHashMap", "modCount", "I");
186+
mv.visitVarInsn(Opcodes.ALOAD, 0);
187+
mv.visitFieldInsn(Opcodes.GETFIELD, "java/util/IdentityHashMap$IdentityHashMapIterator", "expectedModCount",
188+
"I");
189+
Label l1 = new Label();
190+
mv.visitJumpInsn(Opcodes.IF_ICMPEQ, l1);
191+
mv.visitTypeInsn(Opcodes.NEW, "java/util/ConcurrentModificationException");
192+
mv.visitInsn(Opcodes.DUP);
193+
mv.visitMethodInsn(Opcodes.INVOKESPECIAL, "java/util/ConcurrentModificationException", "<init>", "()V", false);
194+
mv.visitInsn(Opcodes.ATHROW);
195+
196+
// Update modCount and expectedModCount
197+
mv.visitLabel(l1);
198+
mv.visitFrame(Opcodes.F_SAME, 0, null, 0, null);
199+
mv.visitVarInsn(Opcodes.ALOAD, 0);
200+
mv.visitVarInsn(Opcodes.ALOAD, 0);
201+
mv.visitFieldInsn(Opcodes.GETFIELD, "java/util/IdentityHashMap$IdentityHashMapIterator", "this$0",
202+
"Ljava/util/IdentityHashMap;");
203+
mv.visitInsn(Opcodes.DUP);
204+
mv.visitFieldInsn(Opcodes.GETFIELD, "java/util/IdentityHashMap", "modCount", "I");
205+
mv.visitInsn(Opcodes.ICONST_1);
206+
mv.visitInsn(Opcodes.IADD);
207+
mv.visitInsn(Opcodes.DUP_X1);
208+
mv.visitFieldInsn(Opcodes.PUTFIELD, "java/util/IdentityHashMap", "modCount", "I");
209+
mv.visitFieldInsn(Opcodes.PUTFIELD, "java/util/IdentityHashMap$IdentityHashMapIterator", "expectedModCount",
210+
"I");
211+
212+
// Initialize deletedSlot with lastReturnedIndex
213+
mv.visitVarInsn(Opcodes.ALOAD, 0);
214+
mv.visitFieldInsn(Opcodes.GETFIELD, "java/util/IdentityHashMap$IdentityHashMapIterator", "lastReturnedIndex",
215+
"I");
216+
mv.visitVarInsn(Opcodes.ISTORE, 1); // deletedSlot in local var 1
217+
218+
// Reset lastReturnedIndex
219+
mv.visitVarInsn(Opcodes.ALOAD, 0);
220+
mv.visitInsn(Opcodes.ICONST_M1);
221+
mv.visitFieldInsn(Opcodes.PUTFIELD, "java/util/IdentityHashMap$IdentityHashMapIterator", "lastReturnedIndex",
222+
"I");
223+
mv.visitVarInsn(Opcodes.ALOAD, 0);
224+
mv.visitVarInsn(Opcodes.ILOAD, 1);
225+
mv.visitFieldInsn(Opcodes.PUTFIELD, "java/util/IdentityHashMap$IdentityHashMapIterator", "index",
226+
"I");
227+
228+
// Reset indexValid
229+
mv.visitVarInsn(Opcodes.ALOAD, 0);
230+
mv.visitInsn(Opcodes.ICONST_0);
231+
mv.visitFieldInsn(Opcodes.PUTFIELD, "java/util/IdentityHashMap$IdentityHashMapIterator", "indexValid",
232+
"Z");
233+
234+
// Initialize tab as reference to traversalTable
235+
mv.visitVarInsn(Opcodes.ALOAD, 0);
236+
mv.visitFieldInsn(Opcodes.GETFIELD, "java/util/IdentityHashMap$IdentityHashMapIterator", "traversalTable",
237+
"[Ljava/lang/Object;");
238+
mv.visitVarInsn(Opcodes.ASTORE, 2); // tab in local var 2
239+
240+
mv.visitVarInsn(Opcodes.ALOAD, 2);
241+
mv.visitInsn(Opcodes.ARRAYLENGTH);
242+
mv.visitVarInsn(Opcodes.ISTORE, 3); // len in local var 3
243+
mv.visitVarInsn(Opcodes.ILOAD, 1);
244+
mv.visitVarInsn(Opcodes.ISTORE, 4); // d in local var 4
245+
246+
// Get key to be removed
247+
mv.visitVarInsn(Opcodes.ALOAD, 2);
248+
mv.visitVarInsn(Opcodes.ILOAD, 4);
249+
mv.visitInsn(Opcodes.AALOAD);
250+
mv.visitVarInsn(Opcodes.ASTORE, 5); // key in local var 5
251+
252+
// Remove key and value
253+
mv.visitVarInsn(Opcodes.ALOAD, 2);
254+
mv.visitVarInsn(Opcodes.ILOAD, 4);
255+
mv.visitInsn(Opcodes.ACONST_NULL);
256+
mv.visitInsn(Opcodes.AASTORE);
257+
258+
mv.visitVarInsn(Opcodes.ALOAD, 2);
259+
mv.visitVarInsn(Opcodes.ILOAD, 4);
260+
mv.visitInsn(Opcodes.ICONST_1);
261+
mv.visitInsn(Opcodes.IADD);
262+
mv.visitInsn(Opcodes.ACONST_NULL);
263+
mv.visitInsn(Opcodes.AASTORE);
264+
265+
// Decrement the size
266+
mv.visitVarInsn(Opcodes.ALOAD, 0);
267+
mv.visitFieldInsn(Opcodes.GETFIELD, "java/util/IdentityHashMap$IdentityHashMapIterator", "this$0",
268+
"Ljava/util/IdentityHashMap;");
269+
mv.visitInsn(Opcodes.DUP);
270+
mv.visitFieldInsn(Opcodes.GETFIELD, "java/util/IdentityHashMap", "size", "I");
271+
mv.visitInsn(Opcodes.ICONST_1);
272+
mv.visitInsn(Opcodes.ISUB);
273+
mv.visitFieldInsn(Opcodes.PUTFIELD, "java/util/IdentityHashMap", "size", "I");
274+
275+
// Inlined nextKeyIndex()
276+
mv.visitVarInsn(Opcodes.ILOAD, 4);
277+
mv.visitInsn(Opcodes.ICONST_2);
278+
mv.visitInsn(Opcodes.IADD);
279+
mv.visitInsn(Opcodes.DUP);
280+
mv.visitVarInsn(Opcodes.ILOAD, 3);
281+
282+
Label elseLabel = new Label();
283+
Label endLabel = new Label();
284+
mv.visitJumpInsn(Opcodes.IF_ICMPGE, elseLabel);
285+
mv.visitJumpInsn(Opcodes.GOTO, endLabel);
286+
mv.visitLabel(elseLabel);
287+
mv.visitFrame(Opcodes.F_SAME1, 0, null, 1, new Object[] { Opcodes.INTEGER });
288+
mv.visitInsn(Opcodes.POP);
289+
mv.visitInsn(Opcodes.ICONST_0);
290+
mv.visitLabel(endLabel);
291+
mv.visitFrame(Opcodes.F_SAME1, 0, null, 1, new Object[] { Opcodes.INTEGER });
292+
mv.visitVarInsn(Opcodes.ISTORE, 6); // i in local var 6
293+
294+
Label loopStart = new Label();
295+
Label loopEnd = new Label();
296+
Label loopContinue = new Label();
297+
298+
mv.visitLabel(loopStart);
299+
mv.visitFrame(Opcodes.F_APPEND, 1, new Object[] { Opcodes.INTEGER }, 0, null);
300+
301+
mv.visitVarInsn(Opcodes.ALOAD, 2);
302+
mv.visitVarInsn(Opcodes.ILOAD, 6);
303+
mv.visitInsn(Opcodes.AALOAD);
304+
mv.visitInsn(Opcodes.DUP);
305+
mv.visitVarInsn(Opcodes.ASTORE, 7); // item in local var 7
306+
307+
// Check if item==null
308+
mv.visitJumpInsn(Opcodes.IFNULL, loopEnd);
309+
310+
// Compute hash by inlining hash(item, len): ((h << 1) - (h << 8)) & (length - 1)
311+
mv.visitVarInsn(Opcodes.ALOAD, 7);
312+
mv.visitMethodInsn(Opcodes.INVOKESTATIC, "java/lang/System", "identityHashCode", "(Ljava/lang/Object;)I", false);
313+
mv.visitInsn(Opcodes.DUP);
314+
mv.visitInsn(Opcodes.ICONST_1);
315+
mv.visitInsn(Opcodes.ISHL); // h << 1
316+
mv.visitInsn(Opcodes.SWAP);
317+
mv.visitIntInsn(Opcodes.BIPUSH, 8);
318+
mv.visitInsn(Opcodes.ISHL);
319+
mv.visitInsn(Opcodes.ISUB);
320+
mv.visitVarInsn(Opcodes.ILOAD, 3);
321+
mv.visitInsn(Opcodes.ICONST_1);
322+
mv.visitInsn(Opcodes.ISUB);
323+
mv.visitInsn(Opcodes.IAND);
324+
mv.visitVarInsn(Opcodes.ISTORE, 8); // r in local var 8
325+
326+
// Complex conditional: if ((i < r && (r <= d || d <= i)) || (r <= d && d <= i))
327+
Label conditionFalse = new Label();
328+
Label conditionTrue = new Label();
329+
330+
// Check cond1: (i < r && (r <= d || d <= i))
331+
mv.visitVarInsn(Opcodes.ILOAD, 6);
332+
mv.visitVarInsn(Opcodes.ILOAD, 8);
333+
Label cond1False = new Label();
334+
mv.visitJumpInsn(Opcodes.IF_ICMPGE, cond1False);
335+
mv.visitVarInsn(Opcodes.ILOAD, 8);
336+
mv.visitVarInsn(Opcodes.ILOAD, 4);
337+
mv.visitJumpInsn(Opcodes.IF_ICMPLE, conditionTrue);
338+
mv.visitVarInsn(Opcodes.ILOAD, 4);
339+
mv.visitVarInsn(Opcodes.ILOAD, 6);
340+
mv.visitJumpInsn(Opcodes.IF_ICMPLE, conditionTrue);
341+
342+
// cond1 is false, check cond2
343+
mv.visitLabel(cond1False);
344+
mv.visitFrame(Opcodes.F_SAME, 0, null, 0, null);
345+
346+
mv.visitVarInsn(Opcodes.ILOAD, 8);
347+
mv.visitVarInsn(Opcodes.ILOAD, 4);
348+
mv.visitJumpInsn(Opcodes.IF_ICMPGT, conditionFalse);
349+
mv.visitVarInsn(Opcodes.ILOAD, 4);
350+
mv.visitVarInsn(Opcodes.ILOAD, 6);
351+
mv.visitJumpInsn(Opcodes.IF_ICMPGT, conditionFalse);
352+
353+
// Condition is true, shift the values
354+
mv.visitLabel(conditionTrue);
355+
mv.visitFrame(Opcodes.F_SAME, 0, null, 0, null);
356+
357+
// Update t[d] to item
358+
mv.visitVarInsn(Opcodes.ALOAD, 2);
359+
mv.visitVarInsn(Opcodes.ILOAD, 4);
360+
mv.visitVarInsn(Opcodes.ALOAD, 7);
361+
mv.visitInsn(Opcodes.AASTORE);
362+
363+
// Shift tab[i + 1] into tab[d + 1]
364+
mv.visitVarInsn(Opcodes.ALOAD, 2);
365+
mv.visitVarInsn(Opcodes.ILOAD, 4);
366+
mv.visitInsn(Opcodes.ICONST_1);
367+
mv.visitInsn(Opcodes.IADD);
368+
mv.visitVarInsn(Opcodes.ALOAD, 2);
369+
mv.visitVarInsn(Opcodes.ILOAD, 6);
370+
mv.visitInsn(Opcodes.ICONST_1);
371+
mv.visitInsn(Opcodes.IADD);
372+
mv.visitInsn(Opcodes.AALOAD);
373+
mv.visitInsn(Opcodes.AASTORE);
374+
375+
// Clear tab[i] to null;
376+
mv.visitVarInsn(Opcodes.ALOAD, 2);
377+
mv.visitVarInsn(Opcodes.ILOAD, 6);
378+
mv.visitInsn(Opcodes.ACONST_NULL);
379+
mv.visitInsn(Opcodes.AASTORE);
380+
381+
// Clear tab[i + 1] to null
382+
mv.visitVarInsn(Opcodes.ALOAD, 2);
383+
mv.visitVarInsn(Opcodes.ILOAD, 6);
384+
mv.visitInsn(Opcodes.ICONST_1);
385+
mv.visitInsn(Opcodes.IADD);
386+
mv.visitInsn(Opcodes.ACONST_NULL);
387+
mv.visitInsn(Opcodes.AASTORE);
388+
389+
// Set d to i
390+
mv.visitVarInsn(Opcodes.ILOAD, 6);
391+
mv.visitVarInsn(Opcodes.ISTORE, 4);
392+
393+
// Update i = nextKeyIndex(i, len), inlined
394+
mv.visitVarInsn(Opcodes.ILOAD, 6);
395+
mv.visitInsn(Opcodes.ICONST_2);
396+
mv.visitInsn(Opcodes.IADD);
397+
mv.visitInsn(Opcodes.DUP);
398+
mv.visitVarInsn(Opcodes.ILOAD, 3);
399+
400+
Label elseLabel2 = new Label();
401+
Label endLabel2 = new Label();
402+
mv.visitJumpInsn(Opcodes.IF_ICMPGE, elseLabel2);
403+
mv.visitJumpInsn(Opcodes.GOTO, endLabel2);
404+
mv.visitLabel(elseLabel2);
405+
mv.visitFrame(Opcodes.F_SAME1, 0, null, 1, new Object[] { Opcodes.INTEGER });
406+
mv.visitInsn(Opcodes.POP);
407+
mv.visitInsn(Opcodes.ICONST_0);
408+
mv.visitLabel(endLabel2);
409+
mv.visitFrame(Opcodes.F_SAME1, 0, null, 1, new Object[] { Opcodes.INTEGER });
410+
mv.visitVarInsn(Opcodes.ISTORE, 6);
411+
412+
mv.visitJumpInsn(Opcodes.GOTO, loopStart);
413+
mv.visitLabel(loopEnd);
414+
mv.visitFrame(Opcodes.F_CHOP, 1, null, 0, null);
415+
416+
mv.visitLabel(conditionFalse);
417+
mv.visitFrame(Opcodes.F_SAME, 0, null, 0, null);
418+
419+
mv.visitInsn(Opcodes.RETURN);
420+
mv.visitMaxs(4, 6);
421+
mv.visitEnd();
422+
}
423+
166424
@Override
167425
public void visitEnd() {
168426
addOrder();
169427
addKeys();
170428
addIdx();
171429
addNextIndex();
172430
addHasNext();
431+
addRemove();
173432
super.visitEnd();
174433
}
175434

@@ -181,6 +440,9 @@ public MethodVisitor visitMethod(int access, String name, String desc, String si
181440
if ("nextIndex".equals(name)) {
182441
return super.visitMethod(access, "originalNextIndex", desc, signature, exceptions);
183442
}
443+
if ("remove".equals(name)) {
444+
return super.visitMethod(access, "originalRemove", desc, signature, exceptions);
445+
}
184446
if ("<init>".equals(name) && "(Ljava/util/IdentityHashMap;)V".equals(desc)) {
185447
return new MethodVisitor(Opcodes.ASM9, super.visitMethod(access, name, desc, signature, exceptions)) {
186448
@Override

‎nondex-test/src/test/java/edu/illinois/nondex/core/IdentityHashMapTest.java‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/*
2+
The MIT License (MIT)
3+
Copyright (c) 2015 Alex Gyori
4+
Copyright (c) 2022 Kaiyao Ke
5+
Copyright (c) 2015 Owolabi Legunsen
6+
Copyright (c) 2015 Darko Marinov
7+
Copyright (c) 2015 August Shi
8+
9+
10+
Permission is hereby granted, free of charge, to any person obtaining
11+
a copy of this software and associated documentation files (the
12+
"Software"), to deal in the Software without restriction, including
13+
without limitation the rights to use, copy, modify, merge, publish,
14+
distribute, sublicense, and/or sell copies of the Software, and to
15+
permit persons to whom the Software is furnished to do so, subject to
16+
the following conditions:
17+
18+
The above copyright notice and this permission notice shall be
19+
included in all copies or substantial portions of the Software.
20+
21+
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
22+
EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
23+
MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
24+
NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
25+
LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
26+
OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
27+
WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
28+
*/
29+
30+
package edu.illinois.nondex.core;
31+
32+
import java.util.Collections;
33+
import java.util.IdentityHashMap;
34+
import java.util.Iterator;
35+
36+
import org.junit.Test;
37+
38+
public class IdentityHashMapTest {
39+
40+
// Author: Chih-Fu Lai (2025)
41+
// Added test coverage for IdentityHashMap.removeAll iterator behavior
42+
@Test
43+
public void testRemoveAllArrayIndexOutOfBounds() {
44+
for (int i = 0; i < 999; i++) {
45+
IdentityHashMap<Object, Object> map = new IdentityHashMap<>(4);
46+
47+
Object k1 = new Object();
48+
Object k2 = new Object();
49+
Object k3 = new Object();
50+
51+
map.put(k1, 1);
52+
map.put(k2, 2);
53+
map.put(k3, 3);
54+
55+
map.keySet().removeAll(Collections.singleton(k2));
56+
}
57+
}
58+
}

0 commit comments

Comments
 (0)