Merge changes Ic67db951,I1ba0f43c

* changes:
  Ensure that DDM processing doesn't leave unhandled exceptions
  Clean up JVMTI DDMS extension function.

6 files changed, 103 insertions, 28 deletions

diff --git a/openjdkjvmti/ti_ddms.cc b/openjdkjvmti/ti_ddms.cc
index 500a453f78..0b4906d798 100644
--- a/openjdkjvmti/ti_ddms.cc
+++ b/openjdkjvmti/ti_ddms.cc
@@ -49,14 +49,16 @@ jvmtiError DDMSUtil::HandleChunk(jvmtiEnv* env,
                                  /*out*/jint* type_out,
                                  /*out*/jint* data_length_out,
                                  /*out*/jbyte** data_out) {
-  constexpr uint32_t kDdmHeaderSize = sizeof(uint32_t) * 2;
-  if (env == nullptr || data_in == nullptr || data_out == nullptr || data_length_out == nullptr) {
+  if (env == nullptr || type_out == nullptr || data_out == nullptr || data_length_out == nullptr) {
     return ERR(NULL_POINTER);
-  } else if (length_in < static_cast<jint>(kDdmHeaderSize)) {
-    // need to get type and length at least.
+  } else if (data_in == nullptr && length_in != 0) {
+    // Data-in shouldn't be null if we have data.
     return ERR(ILLEGAL_ARGUMENT);
   }
 
+  *data_length_out = 0;
+  *data_out = nullptr;
+
   art::Thread* self = art::Thread::Current();
   art::ScopedThreadStateChange(self, art::ThreadState::kNative);
 
@@ -71,13 +73,15 @@ jvmtiError DDMSUtil::HandleChunk(jvmtiEnv* env,
     return ERR(INTERNAL);
   } else {
     jvmtiError error = OK;
-    JvmtiUniquePtr<jbyte[]> ret = AllocJvmtiUniquePtr<jbyte[]>(env, out_data.size(), &error);
-    if (error != OK) {
-      return error;
+    if (!out_data.empty()) {
+      JvmtiUniquePtr<jbyte[]> ret = AllocJvmtiUniquePtr<jbyte[]>(env, out_data.size(), &error);
+      if (error != OK) {
+        return error;
+      }
+      memcpy(ret.get(), out_data.data(), out_data.size());
+      *data_out = ret.release();
+      *data_length_out = static_cast<jint>(out_data.size());
     }
-    memcpy(ret.get(), out_data.data(), out_data.size());
-    *data_out = ret.release();
-    *data_length_out = static_cast<jint>(out_data.size());
     return OK;
   }
 }
diff --git a/openjdkjvmti/ti_extension.cc b/openjdkjvmti/ti_extension.cc
index afd0723d0f..79a8cd6304 100644
--- a/openjdkjvmti/ti_extension.cc
+++ b/openjdkjvmti/ti_extension.cc
@@ -216,7 +216,7 @@ jvmtiError ExtensionUtil::GetExtensionFunctions(jvmtiEnv* env,
       {
         { "type_in", JVMTI_KIND_IN, JVMTI_TYPE_JINT, false },
         { "length_in", JVMTI_KIND_IN, JVMTI_TYPE_JINT, false },
-        { "data_in", JVMTI_KIND_IN_BUF, JVMTI_TYPE_JBYTE, false },
+        { "data_in", JVMTI_KIND_IN_BUF, JVMTI_TYPE_JBYTE, true },
         { "type_out", JVMTI_KIND_OUT, JVMTI_TYPE_JINT, false },
         { "data_len_out", JVMTI_KIND_OUT, JVMTI_TYPE_JINT, false },
         { "data_out", JVMTI_KIND_ALLOC_BUF, JVMTI_TYPE_JBYTE, false }
diff --git a/runtime/debugger.cc b/runtime/debugger.cc
index 541bd1da81..b5ae09f701 100644
--- a/runtime/debugger.cc
+++ b/runtime/debugger.cc
@@ -4379,15 +4379,20 @@ bool Dbg::DdmHandleChunk(JNIEnv* env,
                              replyData.get(),
                              offset,
                              length);
-  if (length == 0 || replyData.get() == nullptr) {
-    return false;
-  }
-
   out_data->resize(length);
   env->GetByteArrayRegion(replyData.get(),
                           offset,
                           length,
                           reinterpret_cast<jbyte*>(out_data->data()));
+
+  if (env->ExceptionCheck()) {
+    LOG(INFO) << StringPrintf("Exception thrown when reading response data from dispatcher 0x%08x",
+                              type);
+    env->ExceptionDescribe();
+    env->ExceptionClear();
+    return false;
+  }
+
   return true;
 }
 
@@ -4421,7 +4426,7 @@ bool Dbg::DdmHandlePacket(JDWP::Request* request, uint8_t** pReplyBuf, int* pRep
   std::vector<uint8_t> out_data;
   uint32_t out_type = 0;
   request->Skip(request_length);
-  if (!DdmHandleChunk(env, type, data, &out_type, &out_data)) {
+  if (!DdmHandleChunk(env, type, data, &out_type, &out_data) || out_data.empty()) {
     return false;
   }
   const uint32_t kDdmHeaderSize = 8;
diff --git a/test/1940-ddms-ext/check b/test/1940-ddms-ext/check
new file mode 100755
index 0000000000..d2c03841fc
--- /dev/null
+++ b/test/1940-ddms-ext/check
@@ -0,0 +1,21 @@
+#!/bin/bash
+#
+# Copyright (C) 2017 The Android Open Source Project
+#
+# 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.
+
+# Need to pull out the describeException ouput since that won't be there on
+# device.
+sed -e '/\t.*$/d' "$2" | sed -e '/java.lang.ArrayIndexOutOfBoundsException:.*$/d' > "$2.tmp"
+
+./default-check "$1" "$2.tmp"
diff --git a/test/1940-ddms-ext/expected.txt b/test/1940-ddms-ext/expected.txt
index 62d3b7bd4c..1a457a01a5 100644
--- a/test/1940-ddms-ext/expected.txt
+++ b/test/1940-ddms-ext/expected.txt
@@ -3,8 +3,19 @@ MyDdmHandler: Chunk received: Chunk(Type: 0xDEADBEEF, Len: 8, data: [1, 2, 3, 4,
 MyDdmHandler: Putting value 0x800025
 MyDdmHandler: Chunk returned: Chunk(Type: 0xFADE7357, Len: 8, data: [0, 0, 0, 0, 0, -128, 0, 37])
 JVMTI returned chunk: Chunk(Type: 0xFADE7357, Len: 8, data: [0, 0, 0, 0, 0, -128, 0, 37])
+Sending empty data array
+MyDdmHandler: Chunk received: Chunk(Type: 0xDEADBEEF, Len: 0, data: [])
+MyDdmHandler: Putting value 0x1
+MyDdmHandler: Chunk returned: Chunk(Type: 0xFADE7357, Len: 8, data: [0, 0, 0, 0, 0, 0, 0, 1])
+JVMTI returned chunk: Chunk(Type: 0xFADE7357, Len: 8, data: [0, 0, 0, 0, 0, 0, 0, 1])
 Sending chunk: Chunk(Type: 0xDEADBEEF, Len: 8, data: [9, 10, 11, 12, 13, 14, 15, 16])
 Chunk published: Chunk(Type: 0xDEADBEEF, Len: 8, data: [9, 10, 11, 12, 13, 14, 15, 16])
+Sending data [1] to chunk handler -1412567295
+MyDdmHandler: Chunk received: Chunk(Type: 0xABCDEF01, Len: 1, data: [1])
+JVMTI returned chunk: Chunk(Type: 0xFADE7357, Len: 0, data: [])
+Sending data [1] to chunk handler 305419896
+MyDdmHandler: Chunk received: Chunk(Type: 0x12345678, Len: 1, data: [1])
+Got error: JVMTI_ERROR_INTERNAL
 Saw expected thread events.
 Expected chunk type published: 1213221190
 Expected chunk type published: 1297109829
diff --git a/test/1940-ddms-ext/src-art/art/Test1940.java b/test/1940-ddms-ext/src-art/art/Test1940.java
index 9f79eaebba..226fe350bd 100644
--- a/test/1940-ddms-ext/src-art/art/Test1940.java
+++ b/test/1940-ddms-ext/src-art/art/Test1940.java
@@ -30,6 +30,8 @@ public class Test1940 {
   public static final int DDMS_HEADER_LENGTH = 8;
   public static final int MY_DDMS_TYPE = 0xDEADBEEF;
   public static final int MY_DDMS_RESPONSE_TYPE = 0xFADE7357;
+  public static final int MY_EMPTY_DDMS_TYPE = 0xABCDEF01;
+  public static final int MY_INVALID_DDMS_TYPE = 0x12345678;
 
   public static final boolean PRINT_ALL_CHUNKS = false;
 
@@ -58,19 +60,27 @@ public class Test1940 {
     public void connected() {}
     public void disconnected() {}
     public Chunk handleChunk(Chunk req) {
-      // For this test we will simply calculate the checksum
-      checkEq(req.type, MY_DDMS_TYPE);
       System.out.println("MyDdmHandler: Chunk received: " + printChunk(req));
-      ByteBuffer b = ByteBuffer.wrap(new byte[8]);
-      Adler32 a = new Adler32();
-      a.update(req.data, req.offset, req.length);
-      b.order(ByteOrder.BIG_ENDIAN);
-      long val = a.getValue();
-      b.putLong(val);
-      System.out.printf("MyDdmHandler: Putting value 0x%X\n", val);
-      Chunk ret = new Chunk(MY_DDMS_RESPONSE_TYPE, b.array(), 0, 8);
-      System.out.println("MyDdmHandler: Chunk returned: " + printChunk(ret));
-      return ret;
+      if (req.type == MY_DDMS_TYPE) {
+        // For this test we will simply calculate the checksum
+        ByteBuffer b = ByteBuffer.wrap(new byte[8]);
+        Adler32 a = new Adler32();
+        a.update(req.data, req.offset, req.length);
+        b.order(ByteOrder.BIG_ENDIAN);
+        long val = a.getValue();
+        b.putLong(val);
+        System.out.printf("MyDdmHandler: Putting value 0x%X\n", val);
+        Chunk ret = new Chunk(MY_DDMS_RESPONSE_TYPE, b.array(), 0, 8);
+        System.out.println("MyDdmHandler: Chunk returned: " + printChunk(ret));
+        return ret;
+      } else if (req.type == MY_EMPTY_DDMS_TYPE) {
+        return new Chunk(MY_DDMS_RESPONSE_TYPE, new byte[0], 0, 0);
+      } else if (req.type == MY_INVALID_DDMS_TYPE) {
+        // This is a very invalid chunk.
+        return new Chunk(MY_DDMS_RESPONSE_TYPE, new byte[] { 0 }, /*offset*/ 12, /*length*/ 55);
+      } else {
+        throw new TestError("Unknown ddm request type: " + req.type);
+      }
     }
   }
 
@@ -113,18 +123,42 @@ public class Test1940 {
         Test1940.class.getDeclaredMethod("HandlePublish", Integer.TYPE, new byte[0].getClass()));
     // Test sending chunk directly.
     DdmServer.registerHandler(MY_DDMS_TYPE, SINGLE_HANDLER);
+    DdmServer.registerHandler(MY_EMPTY_DDMS_TYPE, SINGLE_HANDLER);
+    DdmServer.registerHandler(MY_INVALID_DDMS_TYPE, SINGLE_HANDLER);
     DdmServer.registrationComplete();
     byte[] data = new byte[] { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08 };
     System.out.println("Sending data " + Arrays.toString(data));
     Chunk res = processChunk(data);
     System.out.println("JVMTI returned chunk: " + printChunk(res));
 
+    // Test sending an empty chunk.
+    System.out.println("Sending empty data array");
+    res = processChunk(new byte[0]);
+    System.out.println("JVMTI returned chunk: " + printChunk(res));
+
     // Test sending chunk through DdmServer#sendChunk
     Chunk c = new Chunk(
         MY_DDMS_TYPE, new byte[] { 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10 }, 0, 8);
     System.out.println("Sending chunk: " + printChunk(c));
     DdmServer.sendChunk(c);
 
+    // Test getting back an empty chunk.
+    data = new byte[] { 0x1 };
+    System.out.println(
+        "Sending data " + Arrays.toString(data) + " to chunk handler " + MY_EMPTY_DDMS_TYPE);
+    res = processChunk(new Chunk(MY_EMPTY_DDMS_TYPE, data, 0, 1));
+    System.out.println("JVMTI returned chunk: " + printChunk(res));
+
+    // Test getting back an invalid chunk.
+    System.out.println(
+        "Sending data " + Arrays.toString(data) + " to chunk handler " + MY_INVALID_DDMS_TYPE);
+    try {
+      res = processChunk(new Chunk(MY_INVALID_DDMS_TYPE, data, 0, 1));
+      System.out.println("JVMTI returned chunk: " + printChunk(res));
+    } catch (RuntimeException e) {
+      System.out.println("Got error: " + e.getMessage());
+    }
+
     // Test thread chunks are sent.
     final boolean[] types_seen = new boolean[] { false, false, false };
     CURRENT_HANDLER = (type, cdata) -> {