Clean up JVMTI DDMS extension function.
We change it to not return failure (and print a warning) if the chunk
handler returns an empty chunk. This is a surprisingly common result
in real-world code and was causing significant log-spam.
Bug: 70559172
Test: ./test.py --host -j50
Change-Id: I1ba0f43cb2e834b09f51db75ec9100d97e916b62
diff --git a/openjdkjvmti/ti_ddms.cc b/openjdkjvmti/ti_ddms.cc
index 500a453..0b4906d 100644
--- a/openjdkjvmti/ti_ddms.cc
+++ b/openjdkjvmti/ti_ddms.cc
@@ -49,14 +49,16 @@
/*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 @@
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 afd0723..79a8cd6 100644
--- a/openjdkjvmti/ti_extension.cc
+++ b/openjdkjvmti/ti_extension.cc
@@ -216,7 +216,7 @@
{
{ "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 13029fb..b8f15af 100644
--- a/runtime/debugger.cc
+++ b/runtime/debugger.cc
@@ -4376,15 +4376,12 @@
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()));
+
return true;
}
@@ -4418,7 +4415,7 @@
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/expected.txt b/test/1940-ddms-ext/expected.txt
index 62d3b7b..f12593f 100644
--- a/test/1940-ddms-ext/expected.txt
+++ b/test/1940-ddms-ext/expected.txt
@@ -3,8 +3,16 @@
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: [])
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 9f79eae..55c40f4 100644
--- a/test/1940-ddms-ext/src-art/art/Test1940.java
+++ b/test/1940-ddms-ext/src-art/art/Test1940.java
@@ -30,6 +30,7 @@
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 boolean PRINT_ALL_CHUNKS = false;
@@ -58,19 +59,24 @@
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 {
+ throw new TestError("Unknown ddm request type: " + req.type);
+ }
}
}
@@ -113,18 +119,31 @@
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.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 thread chunks are sent.
final boolean[] types_seen = new boolean[] { false, false, false };
CURRENT_HANDLER = (type, cdata) -> {