RFR(S) 8154820 - JDWP: -agentlib:jdwp=help assertion error

[David Holmes]

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.

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