8205608: Fix 'frames()' in ThreadReferenceImpl.c to prevent quadratic runtime behavior
Reviewed-by: sspitsyn, cjplummer
--- a/src/jdk.jdwp.agent/share/native/libjdwp/ThreadReferenceImpl.c Thu Jul 26 14:15:24 2018 +0530
+++ b/src/jdk.jdwp.agent/share/native/libjdwp/ThreadReferenceImpl.c Mon Jul 23 15:17:14 2018 +0200
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1998, 2008, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2018, 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
@@ -214,12 +214,14 @@
frames(PacketInputStream *in, PacketOutputStream *out)
{
jvmtiError error;
- FrameNumber fnum;
+ FrameNumber index;
jint count;
+ jint filledIn;
JNIEnv *env;
jthread thread;
jint startIndex;
jint length;
+ jvmtiFrameInfo* frames;
env = getEnv();
@@ -273,37 +275,37 @@
(void)outStream_writeInt(out, length);
- for(fnum = startIndex ; fnum < startIndex+length ; fnum++ ) {
+ frames = jvmtiAllocate(sizeof(jvmtiFrameInfo) * length);
- WITH_LOCAL_REFS(env, 1) {
+ if (frames == NULL) {
+ outStream_setError(out, JDWP_ERROR(OUT_OF_MEMORY));
+ return JNI_TRUE;
+ }
- jclass clazz;
- jmethodID method;
- jlocation location;
+ error = JVMTI_FUNC_PTR(gdata->jvmti, GetStackTrace)
+ (gdata->jvmti, thread, startIndex, length, frames,
+ &filledIn);
+
+ /* Should not happen. */
+ if (error == JVMTI_ERROR_NONE && length != filledIn) {
+ error = JVMTI_ERROR_INTERNAL;
+ }
- /* Get location info */
- error = JVMTI_FUNC_PTR(gdata->jvmti,GetFrameLocation)
- (gdata->jvmti, thread, fnum, &method, &location);
- if (error == JVMTI_ERROR_OPAQUE_FRAME) {
- clazz = NULL;
- location = -1L;
- error = JVMTI_ERROR_NONE;
- } else if ( error == JVMTI_ERROR_NONE ) {
- error = methodClass(method, &clazz);
- if ( error == JVMTI_ERROR_NONE ) {
- FrameID frame;
- frame = createFrameID(thread, fnum);
- (void)outStream_writeFrameID(out, frame);
- writeCodeLocation(out, clazz, method, location);
- }
+ for (index = 0; index < filledIn && error == JVMTI_ERROR_NONE; ++index) {
+ WITH_LOCAL_REFS(env, 1) {
+ jclass clazz;
+ error = methodClass(frames[index].method, &clazz);
+
+ if (error == JVMTI_ERROR_NONE) {
+ FrameID frame = createFrameID(thread, index + startIndex);
+ outStream_writeFrameID(out, frame);
+ writeCodeLocation(out, clazz, frames[index].method,
+ frames[index].location);
}
-
} END_WITH_LOCAL_REFS(env);
+ }
- if (error != JVMTI_ERROR_NONE)
- break;
-
- }
+ jvmtiDeallocate(frames);
if (error != JVMTI_ERROR_NONE) {
outStream_setError(out, map2jdwpError(error));
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/com/sun/jdi/Frames2Test.java Mon Jul 23 15:17:14 2018 +0200
@@ -0,0 +1,150 @@
+/*
+ * Copyright (c) 2018, 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 8205608
+ * @summary Test that getting the stack trace for a very large stack does
+ * not take too long
+ *
+ * @author Ralf Schmelter
+ *
+ * @run build TestScaffold VMConnection TargetListener TargetAdapter
+ * @run compile -g Frames2Test.java
+ * @run driver Frames2Test -Xss4M
+ */
+import com.sun.jdi.*;
+import com.sun.jdi.event.*;
+import com.sun.jdi.request.*;
+
+import java.util.*;
+import java.util.stream.*;
+
+
+ /********** target program **********/
+
+class Frames2Targ {
+
+ public static void main(String[] args) {
+ recurse(1_000_000);
+ }
+
+ static void notifyRecursionEnded() {
+ // We moved this to a method instead of doing it directly in the
+ // exception handler for the StackOverflowError, since we don't
+ // need to know the line number (which might change later) when
+ // requesting the breakpoint. Additionally this method is
+ // used as a marker method to check for the correct top of stack
+ // in the stack trace queried by JDI.
+ System.out.println("SOE occurred as expected");
+ }
+
+ static int recurse(int depth) {
+ if (depth == 0) {
+ // Should have seen a stack overflow by now.
+ System.out.println("Exited without creating SOE");
+ System.exit(0);
+ }
+
+ try {
+ int newDepth = recurse(depth - 1);
+
+ if (newDepth == -1_000) {
+ // Pop some frames so there is room on the stack for the
+ // call (including println()).
+ notifyRecursionEnded();
+ }
+
+ return newDepth - 1;
+ } catch (StackOverflowError e) {
+ // Use negative depth to indicate the recursion has ended.
+ return -1;
+ }
+ }
+}
+
+ /********** test program **********/
+
+public class Frames2Test extends TestScaffold {
+
+ public Frames2Test(String args[]) {
+ super(args);
+ }
+
+ public static void main(String[] args) throws Exception {
+ new Frames2Test(args).startTests();
+ }
+
+
+ /********** test core **********/
+
+ protected void runTests() throws Exception {
+ BreakpointEvent bpe = startToMain("Frames2Targ");
+ List<Method> frames1 = bpe.thread().frames().stream().map(
+ StackFrame::location).map(Location::method).
+ collect(Collectors.toList());
+
+ bpe = resumeTo("Frames2Targ", "notifyRecursionEnded", "()V");
+ List<Method> frames2 = bpe.thread().frames().stream().map(
+ StackFrame::location).map(Location::method).
+ collect(Collectors.toList());
+ System.out.println("Got stack of " + frames2.size() + " frames");
+
+ // Check that the stack looks as follows:
+ // notifyRecursionEnded()
+ // recurse()
+ // ....
+ // recurse()
+ // main()
+ // <whatever the VM likes to add>
+
+ // Check the bottom of the stack.
+ for (int i = 0; i < frames1.size(); ++i) {
+ int i2 = frames2.size() - frames1.size() + i;
+ if (!frames1.get(i).equals(frames2.get(i2))) {
+ failure("Bottom methods do not match: " +
+ frames1.get(i) + " vs " + frames2.get(i2));
+ }
+ }
+
+ // Check the recurse() calls on the stack.
+ for (int i = 1; i < frames2.size() - frames1.size(); ++i) {
+ if (!frames2.get(i).name().equals("recurse")) {
+ failure("Expected recurse() but got " + frames2.get(i));
+ }
+ }
+
+ // Check the top method of the stack.
+ if (!frames2.get(0).name().equals("notifyRecursionEnded")) {
+ failure("Expected notifyRecursionEnded() but got " + frames2.get(0));
+ }
+
+ listenUntilVMDisconnect();
+
+ if (!testFailed) {
+ println("Frames2Test: passed");
+ } else {
+ throw new Exception("Frames2Test: failed");
+ }
+ }
+}