This repository has been archived by the owner on Sep 2, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 56
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
8258077: Using -Xcheck:jni can lead to a double-free after JDK-8193234
8259446: runtime/jni/checked/TestCheckedReleaseArrayElements.java fails with stderr not empty Reviewed-by: coleenp Backport-of: 712014c5956cf74982531d212b03460843e4e5b6
- Loading branch information
Harold Seigel
committed
Feb 25, 2021
1 parent
9fa0e03
commit b7b2f61
Showing
5 changed files
with
319 additions
and
19 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
125 changes: 125 additions & 0 deletions
125
test/hotspot/jtreg/runtime/jni/checked/TestCheckedReleaseArrayElements.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,125 @@ | ||
| /* | ||
| * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. | ||
| * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. | ||
| * | ||
| * This code is free software; you can redistribute it and/or modify it | ||
| * under the terms of the GNU General Public License version 2 only, as | ||
| * published by the Free Software Foundation. | ||
| * | ||
| * This code is distributed in the hope that it will be useful, but WITHOUT | ||
| * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or | ||
| * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License | ||
| * version 2 for more details (a copy is included in the LICENSE file that | ||
| * accompanied this code). | ||
| * | ||
| * You should have received a copy of the GNU General Public License version | ||
| * 2 along with this work; if not, write to the Free Software Foundation, | ||
| * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. | ||
| * | ||
| * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA | ||
| * or visit www.oracle.com if you need additional information or have any | ||
| * questions. | ||
| */ | ||
|
|
||
| /* @test | ||
| * @bug 8258077 | ||
| * @summary verify multiple release calls on a copied array work when checked | ||
| * @library /test/lib | ||
| * @run main/native TestCheckedReleaseArrayElements launch | ||
| */ | ||
|
|
||
| import java.util.Arrays; | ||
| import jdk.test.lib.process.ProcessTools; | ||
| import jdk.test.lib.process.OutputAnalyzer; | ||
| import jdk.test.lib.Utils; | ||
| import jtreg.SkippedException; | ||
|
|
||
| public class TestCheckedReleaseArrayElements { | ||
|
|
||
| static { | ||
| System.loadLibrary("TestCheckedReleaseArrayElements"); | ||
| } | ||
|
|
||
| public static void main(String[] args) throws Throwable { | ||
| if (args == null || args.length == 0) { | ||
| test(); | ||
| } else { | ||
| // Uses executeProcess() instead of executeTestJvm() to avoid passing options | ||
| // that might generate output on stderr (which should be empty for this test). | ||
| ProcessBuilder pb = | ||
| ProcessTools.createJavaProcessBuilder("-Xcheck:jni", | ||
| "-Djava.library.path=" + Utils.TEST_NATIVE_PATH, | ||
| "TestCheckedReleaseArrayElements"); | ||
| OutputAnalyzer output = ProcessTools.executeProcess(pb); | ||
| output.shouldHaveExitValue(0); | ||
| output.stderrShouldBeEmpty(); | ||
| output.stdoutShouldNotBeEmpty(); | ||
| } | ||
| } | ||
|
|
||
| /* | ||
| * If GetIntArrayElements returns a copy, we update the array in slices | ||
| * calling ReleaseIntArrayElements with JNI_COMMIT to write-back the | ||
| * updates, which are then checked. Finally we use JNI_ABORT to free | ||
| * the copy. | ||
| */ | ||
| public static void test() { | ||
| final int slices = 3; | ||
| final int sliceLength = 3; | ||
| int[] arr = new int[slices * sliceLength]; | ||
|
|
||
| if (!init(arr)) { | ||
| throw new SkippedException("Test skipped as GetIntArrayElements did not make a copy"); | ||
| } | ||
|
|
||
| System.out.println("Array before: " + Arrays.toString(arr)); | ||
|
|
||
| // We fill the array in slices so that arr[i] = i | ||
| for (int i = 0; i < slices; i++) { | ||
| int start = i * sliceLength; | ||
| fill(arr, start, sliceLength); | ||
| System.out.println("Array during: " + Arrays.toString(arr)); | ||
| check(arr, (i + 1) * sliceLength); | ||
| } | ||
| System.out.println("Array after: " + Arrays.toString(arr)); | ||
| cleanup(arr); | ||
| } | ||
|
|
||
| /* | ||
| * Calls GetIntArrayElements and stashes the native pointer for | ||
| * use by fill() if a copy was made. | ||
| * Returns true if a copy was made else false. | ||
| */ | ||
| static native boolean init(int[] arr); | ||
|
|
||
| /* | ||
| * Fills in target[start] to target[start+count-1], so that | ||
| * target[i] == i. The update is done natively using the raw | ||
| * pointer into the array. | ||
| */ | ||
| static native void fill(int[] target, int start, int count); | ||
|
|
||
| /* | ||
| * Properly release the copied array | ||
| */ | ||
| static native void cleanup(int[] target); | ||
|
|
||
|
|
||
| static void check(int[] source, int count) { | ||
| for (int i = 0; i < count; i++) { | ||
| if (source[i] != i) { | ||
| System.out.println("Failing source array: " + Arrays.toString(source)); | ||
| throw new RuntimeException("Expected source[" + i + "] == " + | ||
| i + " but got " + source[i]); | ||
| } | ||
| } | ||
| for (int i = count; i < source.length; i++) { | ||
| if (source[i] != 0) { | ||
| System.out.println("Failing source array: " + Arrays.toString(source)); | ||
| throw new RuntimeException("Expected source[" + i + | ||
| "] == 0 but got " + source[i]); | ||
| } | ||
| } | ||
|
|
||
| } | ||
| } |
60 changes: 60 additions & 0 deletions
60
test/hotspot/jtreg/runtime/jni/checked/TestCheckedReleaseCriticalArray.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| /* | ||
| * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. | ||
| * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. | ||
| * | ||
| * This code is free software; you can redistribute it and/or modify it | ||
| * under the terms of the GNU General Public License version 2 only, as | ||
| * published by the Free Software Foundation. | ||
| * | ||
| * This code is distributed in the hope that it will be useful, but WITHOUT | ||
| * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or | ||
| * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License | ||
| * version 2 for more details (a copy is included in the LICENSE file that | ||
| * accompanied this code). | ||
| * | ||
| * You should have received a copy of the GNU General Public License version | ||
| * 2 along with this work; if not, write to the Free Software Foundation, | ||
| * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. | ||
| * | ||
| * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA | ||
| * or visit www.oracle.com if you need additional information or have any | ||
| * questions. | ||
| */ | ||
|
|
||
| /* @test | ||
| * @bug 8193234 8258077 | ||
| * @summary Check ReleasePrimitiveArrayCritical doesn't leak memory with | ||
| * Xcheck:jni and JNI_COMMIT mode | ||
| * @comment This is a manual test as you need to verify memory usage externally. | ||
| * @library /test/lib | ||
| * @run main/othervm/native/manual -Xms4m -Xmx4m -Xcheck:jni TestCheckedReleaseCriticalArray | ||
| */ | ||
| import jdk.test.lib.process.ProcessTools; | ||
| import jdk.test.lib.process.OutputAnalyzer; | ||
| import jtreg.SkippedException; | ||
|
|
||
| public class TestCheckedReleaseCriticalArray { | ||
|
|
||
| static { | ||
| System.loadLibrary("TestCheckedReleaseCriticalArray"); | ||
| } | ||
|
|
||
| /* | ||
| * We repeatedly modify an array via the JNI critical functions, using | ||
| * JNI_COMMIT mode. No memory leak should be observed on a VM that | ||
| * provides direct array access. | ||
| */ | ||
| public static void main(String[] args) { | ||
| int[] array = new int[] { 1, 2, 3 }; | ||
| if (!modifyArray(array)) { | ||
| // If the VM makes copies then we will leak them if we only ever use | ||
| // JNI_COMMIT mode. | ||
| throw new SkippedException("Test skipped as GetPrimitiveArrayCritical made a copy"); | ||
| } | ||
| while (true) { | ||
| modifyArray(array); | ||
| } | ||
| } | ||
|
|
||
| private static native boolean modifyArray(int[] array); | ||
| } |
64 changes: 64 additions & 0 deletions
64
test/hotspot/jtreg/runtime/jni/checked/libTestCheckedReleaseArrayElements.c
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| /* | ||
| * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. | ||
| * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. | ||
| * | ||
| * This code is free software; you can redistribute it and/or modify it | ||
| * under the terms of the GNU General Public License version 2 only, as | ||
| * published by the Free Software Foundation. | ||
| * | ||
| * This code is distributed in the hope that it will be useful, but WITHOUT | ||
| * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or | ||
| * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License | ||
| * version 2 for more details (a copy is included in the LICENSE file that | ||
| * accompanied this code). | ||
| * | ||
| * You should have received a copy of the GNU General Public License version | ||
| * 2 along with this work; if not, write to the Free Software Foundation, | ||
| * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. | ||
| * | ||
| * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA | ||
| * or visit www.oracle.com if you need additional information or have any | ||
| * questions. | ||
| */ | ||
|
|
||
| #include <jni.h> | ||
|
|
||
| static jint* arr; | ||
|
|
||
| JNIEXPORT jboolean JNICALL | ||
| Java_TestCheckedReleaseArrayElements_init(JNIEnv *env, | ||
| jclass* clazz, | ||
| jintArray target) { | ||
| jboolean isCopy; | ||
| arr = (*env)->GetIntArrayElements(env, target, &isCopy); | ||
| if (arr == NULL) { | ||
| (*env)->FatalError(env, "Unexpected NULL return from GetIntArrayElements"); | ||
| } | ||
| if (isCopy == JNI_FALSE) { | ||
| (*env)->ReleaseIntArrayElements(env, target, arr, JNI_ABORT); | ||
| } | ||
| return isCopy; | ||
| } | ||
|
|
||
| JNIEXPORT void JNICALL | ||
| Java_TestCheckedReleaseArrayElements_cleanup(JNIEnv *env, | ||
| jclass* clazz, | ||
| jintArray target) { | ||
| (*env)->ReleaseIntArrayElements(env, target, arr, JNI_ABORT); | ||
| } | ||
|
|
||
|
|
||
| JNIEXPORT void JNICALL | ||
| Java_TestCheckedReleaseArrayElements_fill(JNIEnv *env, | ||
| jclass* clazz, | ||
| jintArray target, | ||
| jint start, | ||
| jint count) { | ||
| // Update a slice of the raw array | ||
| int i; | ||
| for (i = start; count > 0; i++, count--) { | ||
| arr[i] = i; | ||
| } | ||
| // Write the results back to target, leaving it usable for future updates | ||
| (*env)->ReleaseIntArrayElements(env, target, arr, JNI_COMMIT); | ||
| } |
46 changes: 46 additions & 0 deletions
46
test/hotspot/jtreg/runtime/jni/checked/libTestCheckedReleaseCriticalArray.c
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| /* | ||
| * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. | ||
| * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. | ||
| * | ||
| * This code is free software; you can redistribute it and/or modify it | ||
| * under the terms of the GNU General Public License version 2 only, as | ||
| * published by the Free Software Foundation. | ||
| * | ||
| * This code is distributed in the hope that it will be useful, but WITHOUT | ||
| * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or | ||
| * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License | ||
| * version 2 for more details (a copy is included in the LICENSE file that | ||
| * accompanied this code). | ||
| * | ||
| * You should have received a copy of the GNU General Public License version | ||
| * 2 along with this work; if not, write to the Free Software Foundation, | ||
| * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. | ||
| * | ||
| * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA | ||
| * or visit www.oracle.com if you need additional information or have any | ||
| * questions. | ||
| */ | ||
|
|
||
| #include <jni.h> | ||
|
|
||
| JNIEXPORT jboolean JNICALL | ||
| Java_TestCheckedReleaseCriticalArray_modifyArray(JNIEnv *env, | ||
| jclass clazz, | ||
| jintArray iarr) { | ||
| jboolean isCopy; | ||
| jint* arr = (jint *)(*env)->GetPrimitiveArrayCritical(env, iarr, &isCopy); | ||
| if (arr == NULL) { | ||
| (*env)->FatalError(env, "Unexpected NULL return from GetPrimitiveArrayCritical"); | ||
| } | ||
| if (isCopy == JNI_FALSE) { | ||
| jint len = (*env)->GetArrayLength(env, iarr); | ||
| // make arbitrary changes to the array | ||
| for (int i = 0; i < len; i++) { | ||
| arr[i] *= 2; | ||
| } | ||
| // write-back using JNI_COMMIT to test for memory leak | ||
| (*env)->ReleasePrimitiveArrayCritical(env, iarr, arr, JNI_COMMIT); | ||
| } | ||
| // we skip the test if the VM makes a copy - as it will definitely leak | ||
| return !isCopy; | ||
| } |
b7b2f61There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review
Issues