Skip to content

Commit 90f3d5e

Browse files
lycoris106lycoris106
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 90f3d5e

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)