RFR (trivial): 8235819: -Wformat-overflow is reported from GCC 9

[Yasumasa Suenaga]

Hi Thomas,

I uploaded new webrev:

   http://cr.openjdk.java.net/~ysuenaga/JDK-8235819/webrev.01/

I guess static analyzer in GCC cannot trace each condition deeply.
However, G1 might have a bug to access out of Indents[] in future. It is the motivation of this fix.

This webrev introduces indent() to generate indent string.
indent() has assert() to check indent level, but it would not affect in release build (we can see same error).
So I extends Indents[] in this webrev.

`indent` is already used in some place. So I renamed it to avoid confusion.


Thanks,

Yasumasa


On 2019/12/16 19:19, Thomas Schatzl wrote:
> Hi,
> 
> On 15.12.19 07:08, Yasumasa Suenaga wrote:
>> Hi Thomas, Leo,
>>
>> How about this change?
>>
>>    http://hg.openjdk.java.net/jdk/submit/rev/10816e67561a
>>
>> It generates indent string via alloca().
>> So get_indent() should be inlined.
> 
> (typical Hotspot style is without the "get_" prefix).
> 
>>
>> However it was failed at TestGCLogMessages on macOS only on submit repo (mach5-one-ysuenaga-JDK-8235819-1-20191215-0359-7545712).
>>
>> If you are generally ok to this change, could you share details of this failure?
>> I will fix it.
> 
> The alloca-allocated buffer will be freed at the exit of the get_indent() method, so the new code effectively accesses freed memory as get_indent() returns a pointer to it.
> 
> The intent of our comments has basically been:
> 
> - remove the "5" (or "4") in the Indents static.
> - add a static indent() method that does something like:
> 
> static const char* indent(uint level) {
>    assert(level < ARRAY_SIZE(Indents), "Too high indent level %u", level);
>    return Indents[level];
> }
> 
> And replace the direct access to the Indents array in the code with a call to  the indent() method.
> 
> If gcc still complains, add that additional indent level you added in webrev.00. I do not like easily avoidable pragmas in the code, and the additional level does not hurt, but others might disagree.
> 
> Probably also file a bug with gcc.
> 
> Thanks,
>    Thomas