On 6/15/16 7:59 PM, David Holmes wrote:
Hi Ioi,
On 16/06/2016 8:14 AM, Ioi Lam wrote:
Oops. Please ignore my last mail. This one uses the correct mailing
lists.
- Ioi
=============================================
On 6/15/16 3:06 PM, Ioi Lam wrote:
Please review a small fix:
Bug: https://bugs.openjdk.java.net/browse/JDK-8154820
Desc: JDWP: -agentlib:jdwp=help assertion error
Fix: http://cr.openjdk.java.net/~iklam/jdk9/8154820_jdwp_help_exit.v02/
Changes look good!
I find this code generally seems to confuse lengths and indices. There
is another usage in
./jdk.jdwp.agent/share/native/libjdwp/transport.c
that seems suspect:
/* Convert this string to UTF8 */
len = (int)strlen(msg);
maxlen = len+len/2+2; /* Should allow for plenty of room */
utf8msg = (jbyte*)jvmtiAllocate(maxlen+1);
if (utf8msg != NULL) {
(void)utf8FromPlatform(msg, len, utf8msg, maxlen);
utf8msg[maxlen] = 0;
}
This should be passing in maxlen+1 as the actual buffer length.
Hi David,
Thanks for pointing that out. I've added the following change and I'm
rerunning the tests now.
$ hg diff transport.c
diff -r dfcdb0a45822 src/jdk.jdwp.agent/share/native/libjdwp/transport.c
--- a/src/jdk.jdwp.agent/share/native/libjdwp/transport.c Mon Jun 13
09:03:32 2016 -0400
+++ b/src/jdk.jdwp.agent/share/native/libjdwp/transport.c Thu Jun 16
00:31:19 2016 -0700
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1998, 2015, Oracle and/or its affiliates. All rights
reserved.
+ * Copyright (c) 1998, 2016, 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
@@ -67,7 +67,7 @@
maxlen = len+len/2+2; /* Should allow for plenty of room */
utf8msg = (jbyte*)jvmtiAllocate(maxlen+1);
if (utf8msg != NULL) {
- (void)utf8FromPlatform(msg, len, utf8msg, maxlen);
+ (void)utf8FromPlatform(msg, len, utf8msg, maxlen+1);
utf8msg[maxlen] = 0;
}
}
Thanks,
David
The were two issues:
[1] MAXPATHLEN is really small on Windows (~256), so MAX_MESSAGE_LEN
was around 1024, but the jdwp help message is about 1600 bytes.
The fix would expand it to abut 2500 bytes which is more than
enough.
[2] The meaning of the "outputMaxLen" parameter to the utf8ToPlatform()
and utf8FromPlatform() functions was ambiguous. The assert
UTF_ASSERT(outputMaxLen > len);
suggest that the trailing zero is included in the buffer size, so
at most outputMaxLenbytes are written.
However, the implementation actually would write outputMaxLen+1
bytes
in the worst case.
Luckily, this does not seem to be a problem as all calls to
utf8ToPlatform/utf8FromPlatform allocate way more space than
needed. The only exception was vprint_message, which chose to pass
outputMaxLen := sizeof(buf) -1
So in the worst case, the VM would exit due to the above UTF_ASSERT,
but
we would not silently write one byte past the end of the buffer.
The fix is:
[a] Clarify the meaning of the outputMaxLen parameter
[b] Make sure we don't write past the end of the buffer
[c] Fix vprint_message() to pass in the correct size of the
buffer to include space for the trailing zero.
Thanks
Ioi
