RFR: 8256256: UL should not use heap allocation for output string [v4]

[Thomas Stuefe]

On Thu, 19 Nov 2020 19:27:20 GMT, Yumin Qi <minqi at openjdk.org> wrote:

>> Hi, Please review
>>   Unified Logging uses 512 bytes buffer on stack for printing out message, above that will allocate heap for extra space needed. This may potentially may cause a circulation when we log the heap allocation and the log message is over 512. The max logging buffer size now is increased to 4096 and above that, output will be truncated.
>> 
>> Tests: tier1,tier4
>
> Yumin Qi has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix Copyright year

src/hotspot/share/logging/logTagSet.cpp line 127:

> 125:   }
> 126:   assert(ret >= 0, "Log message buffer issue");
> 127:   size_t newbuf_len = (size_t)ret + prefix_len + 1; // total bytes needed including prefix.

After the assert, I would return if ret < 0 to capture the release path.
Otherwise you could end up malloc'ing a buffer which is too short by one char.

src/hotspot/share/logging/logTagSet.cpp line 128:

> 126:   assert(ret >= 0, "Log message buffer issue");
> 127:   size_t newbuf_len = (size_t)ret + prefix_len + 1; // total bytes needed including prefix.
> 128:   if (newbuf_len <= sizeof(buf)) {

Hey, I just noted that your patch fixes a subtle bug here if we print with a large prefix `prefix_len > vwrite_buffer_size` but a small message `ret < sizeof(buf)`:

Before, the code would have printed just the message without the prefix. Now, we correctly go into the malloc path.

src/hotspot/share/logging/logTagSet.cpp line 146:

> 144:       prefix_len = _write_prefix(buf, sizeof(buf));
> 145:       if (prefix_len < sizeof(buf)) {
> 146:         ret = os::vsnprintf(buf + prefix_len, sizeof(buf) - prefix_len, fmt, args);

Don't you need to use saved_args here? Original args pointer is already invalid.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1246