Prevent ConcurrentModificationException in TraceCmpHooks
The hooked map can be modified concurrently and could cause concurrent
modification exceptions as seen during startup of Spring Boot
applications.
diff --git a/agent/src/main/java/com/code_intelligence/jazzer/runtime/TraceCmpHooks.java b/agent/src/main/java/com/code_intelligence/jazzer/runtime/TraceCmpHooks.java
index e6d7418..37e8eae 100644
--- a/agent/src/main/java/com/code_intelligence/jazzer/runtime/TraceCmpHooks.java
+++ b/agent/src/main/java/com/code_intelligence/jazzer/runtime/TraceCmpHooks.java
@@ -18,6 +18,7 @@
import com.code_intelligence.jazzer.api.MethodHook;
import java.lang.invoke.MethodHandle;
import java.util.Arrays;
+import java.util.ConcurrentModificationException;
import java.util.Map;
import java.util.TreeMap;
@@ -291,42 +292,47 @@
// https://github.com/llvm/llvm-project/blob/318942de229beb3b2587df09e776a50327b5cef0/compiler-rt/lib/fuzzer/FuzzerTracePC.cpp#L564
Object lowerBoundKey = null;
Object upperBoundKey = null;
- if (map instanceof TreeMap) {
- final TreeMap treeMap = (TreeMap) map;
- try {
- lowerBoundKey = treeMap.floorKey(currentKey);
- upperBoundKey = treeMap.ceilingKey(currentKey);
- } catch (ClassCastException ignored) {
- // Can be thrown by floorKey and ceilingKey if currentKey is of a type that can't be
- // compared to the maps keys.
- }
- } else if (currentKey instanceof Comparable) {
- final Comparable comparableCurrentKey = (Comparable) currentKey;
- // Find two keys that bracket currentKey.
- // Note: This is not deterministic if map.size() > MAX_NUM_KEYS_TO_ENUMERATE.
- int enumeratedKeys = 0;
- for (Object validKey : map.keySet()) {
- if (!(validKey instanceof Comparable))
- continue;
- final Comparable comparableValidKey = (Comparable) validKey;
- // If the key sorts lower than the non-existing key, but higher than the current lower
- // bound, update the lower bound and vice versa for the upper bound.
+ try {
+ if (map instanceof TreeMap) {
+ final TreeMap treeMap = (TreeMap) map;
try {
- if (comparableValidKey.compareTo(comparableCurrentKey) < 0
- && (lowerBoundKey == null || comparableValidKey.compareTo(lowerBoundKey) > 0)) {
- lowerBoundKey = validKey;
- }
- if (comparableValidKey.compareTo(comparableCurrentKey) > 0
- && (upperBoundKey == null || comparableValidKey.compareTo(upperBoundKey) < 0)) {
- upperBoundKey = validKey;
- }
+ lowerBoundKey = treeMap.floorKey(currentKey);
+ upperBoundKey = treeMap.ceilingKey(currentKey);
} catch (ClassCastException ignored) {
// Can be thrown by floorKey and ceilingKey if currentKey is of a type that can't be
// compared to the maps keys.
}
- if (enumeratedKeys++ > MAX_NUM_KEYS_TO_ENUMERATE)
- break;
+ } else if (currentKey instanceof Comparable) {
+ final Comparable comparableCurrentKey = (Comparable) currentKey;
+ // Find two keys that bracket currentKey.
+ // Note: This is not deterministic if map.size() > MAX_NUM_KEYS_TO_ENUMERATE.
+ int enumeratedKeys = 0;
+ for (Object validKey : map.keySet()) {
+ if (!(validKey instanceof Comparable))
+ continue;
+ final Comparable comparableValidKey = (Comparable) validKey;
+ // If the key sorts lower than the non-existing key, but higher than the current lower
+ // bound, update the lower bound and vice versa for the upper bound.
+ try {
+ if (comparableValidKey.compareTo(comparableCurrentKey) < 0
+ && (lowerBoundKey == null || comparableValidKey.compareTo(lowerBoundKey) > 0)) {
+ lowerBoundKey = validKey;
+ }
+ if (comparableValidKey.compareTo(comparableCurrentKey) > 0
+ && (upperBoundKey == null || comparableValidKey.compareTo(upperBoundKey) < 0)) {
+ upperBoundKey = validKey;
+ }
+ } catch (ClassCastException ignored) {
+ // Can be thrown by floorKey and ceilingKey if currentKey is of a type that can't be
+ // compared to the maps keys.
+ }
+ if (enumeratedKeys++ > MAX_NUM_KEYS_TO_ENUMERATE)
+ break;
+ }
}
+ } catch (ConcurrentModificationException ignored) {
+ // map was modified by another thread, skip this invocation
+ return;
}
// Modify the hook ID so that compares against distinct valid keys are traced separately.
if (lowerBoundKey != null) {
diff --git a/agent/src/test/java/com/code_intelligence/jazzer/runtime/BUILD.bazel b/agent/src/test/java/com/code_intelligence/jazzer/runtime/BUILD.bazel
index 360c451..ad7ddb0 100644
--- a/agent/src/test/java/com/code_intelligence/jazzer/runtime/BUILD.bazel
+++ b/agent/src/test/java/com/code_intelligence/jazzer/runtime/BUILD.bazel
@@ -1,3 +1,5 @@
+load("//bazel:compat.bzl", "SKIP_ON_WINDOWS")
+
java_test(
name = "RecordingFuzzedDataProviderTest",
srcs = [
@@ -10,3 +12,16 @@
"@maven//:junit_junit",
],
)
+
+java_test(
+ name = "TraceCmpHooksTest",
+ srcs = [
+ "TraceCmpHooksTest.java",
+ ],
+ target_compatible_with = SKIP_ON_WINDOWS,
+ deps = [
+ "//agent/src/main/java/com/code_intelligence/jazzer/runtime",
+ "//agent/src/test/java/com/code_intelligence/jazzer:MockDriver",
+ "@maven//:junit_junit",
+ ],
+)
diff --git a/agent/src/test/java/com/code_intelligence/jazzer/runtime/TraceCmpHooksTest.java b/agent/src/test/java/com/code_intelligence/jazzer/runtime/TraceCmpHooksTest.java
new file mode 100644
index 0000000..0e83838
--- /dev/null
+++ b/agent/src/test/java/com/code_intelligence/jazzer/runtime/TraceCmpHooksTest.java
@@ -0,0 +1,60 @@
+/*
+ * Copyright 2022 Code Intelligence GmbH
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.code_intelligence.jazzer.runtime;
+
+import com.code_intelligence.jazzer.MockDriver;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Function;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+public class TraceCmpHooksTest {
+ private static final ExecutorService ES = Executors.newFixedThreadPool(5);
+
+ @BeforeClass
+ public static void setup() {
+ MockDriver.load();
+ }
+
+ @Test
+ public void cmpHookShouldHandleConcurrentModifications() throws InterruptedException {
+ String arg = "test";
+ Map<String, Object> map = new HashMap<>();
+ map.put(arg, arg);
+
+ // Add elements to map asynchronously
+ Function<Integer, Runnable> put = (final Integer num) -> () -> {
+ map.put(String.valueOf(num), num);
+ };
+ for (int i = 0; i < 1_000_000; i++) {
+ ES.submit(put.apply(i));
+ }
+
+ // Call hook
+ for (int i = 0; i < 1_000; i++) {
+ TraceCmpHooks.mapGet(null, map, new Object[] {arg}, 1, null);
+ }
+
+ ES.shutdown();
+ // noinspection ResultOfMethodCallIgnored
+ ES.awaitTermination(5, TimeUnit.SECONDS);
+ }
+}
