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

[Dmitry Samersoff]

Ioi,

As soon as you touched this code, few minor nits.

error_messages.c:

54:
  MAXPATHLEN*8 has no sence for me. It might be better to just
  use a constant with an appropriate comment.

68:
 We should either add 1 when allocating buffer or substact 1 when
 setting termination zero.

    But now we are adding 1 at error_messages.c:70 then
 substracting it at utf_util.c:365


 So please, change 68, 70 to use MAX_MESSAGE_LEN
 And 74 to use MAX_MESSAGE_LEN - 1

 Changes at ll. 78 is not necessary.


utf_util.c:

364:
  It might be better to change the order of asserts -

   UTF_ASSERT(utf8);
   UTF_ASSERT(len >= 0);
  ...

365.
   it might be better to just use outputMaxLen -1 at ll. 374

-Dmitry

On 2016-06-16 01:06, 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/
> 
> 
> 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


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.