RFR(S): 8214854: JDWP: Unforseen output truncation in logging

[Chris Plummer]

On 2/28/19 5:07 PM, David Holmes wrote:
> Hi Dmitry,
>
> On 1/03/2019 6:15 am, Dmitry Chuyko wrote:
>> Hello,
>>
>> Please review a small fix for GCC 8.x warning in log_messages.c. 
>> Buffer sizes for parts of timestamp string can be adjusted to not 
>> exceed maximum buffer size when combined, while being able to hold 
>> necessary information.
>>
>> webrev: http://cr.openjdk.java.net/~dchuyko/8214854/webrev.00/
>> bug: https://bugs.openjdk.java.net/browse/JDK-8214854
>> testing: after the fix GCC 8 compiles working OpenJDK on Linux. 
>> Dev-submit job started.
>
> This looks good.
>
> One query:
>
>  51 typedef char timestamp_buffer[TIMESTAMP_SIZE];
>
> Is this needed so that the size of the buffer is known by gcc inside 
> get_time_stamp? (Generally I prefer to see explicitly when an array is 
> being stack allocated, not have it hidden behind a typedef.)
This also caught my attention and then got me thinking on what basis is 
gcc producing the warning in the first place. The warning is:

  src/jdk.jdwp.agent/share/native/libjdwp/log_messages.c:75:24: error: 
'%.3d' directive output may be truncated writing between 3 and 11 bytes 
into a region of size between 0 and 80 [-Werror=format-truncation=]
                     "%s.%.3d %s", timestamp_prefix,
                         ^~~~

I think it deduced this by seeing that the call to get_time_stamp() is 
passing in a char[80] for the tbuf argument. It then also sees that the 
argument for the first %s is also a char[80] array, so is pointing out 
that this first %s might fill (or nearly fill) tbuf, not leaving room 
for the %.3d argument. If this is the case, there is no need for the 
timestamp_buffer typedef. The real fix was making timestamp_date_time 
and timestamp_timezone smaller so gcc doesn't think they can overflow tbuf.

Chris


>
> Thanks,
> David
>
>
>> -Dmitry
>>