RFR(S): 8252407: Build failure with gcc-8+ and asan

[Thomas Schatzl]

Hi,

On 01.09.20 11:27, Kim Barrett wrote:
>> On Sep 1, 2020, at 4:57 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
>>
>> Hi,
>>
>> On 01.09.20 10:48, serguei.spitsyn at oracle.com wrote:
>>> Hi Eric,
>>> It looks good to me.
>>> Thank you for taking care about it.
>>> Thanks,
>>> Serguei
>>> On 9/1/20 01:15, Eric Liu wrote:
>>>> Hi all,
>>>>
>>>> Please review this simple change to fix some compile warnings.
>>>>
>>>> The newer gcc (gcc-8 or higher) would warn for calls to bounded string
>>>> manipulation functions such as 'strncpy' that may either truncate the
>>>> copied string or leave the destination unchanged.
>>>>
>>>> This patch fixed stringop-truncation warnings reported by gcc, some of
>>>> them only appear when compiled with "--enable-asan".
>>>>
>>>> [TESTS]
>>>> Jtreg: hotspot::hotspot_all_no_apps, jdk::jdk_core and langtools::tier1.
>>>> No new failure found.
>>>>
>>>> http://cr.openjdk.java.net/~qfeng/ericliu/jdk/stringop_trunc/webrev.00/
>>>> https://bugs.openjdk.java.net/browse/JDK-8252407
>>>>
>>
>> Looks good.
>>
>> Maybe the comment in compileBroker.hpp could be improved by specifying a gcc version and/or gcc bug number (not sure if done elsewhere too) - so that it can be verified/removed at some point.
>>
>> 66       // This code can incorrectly cause a "stringop-truncation" warning with gcc
> 
> We don’t really know what gcc versions are relevant, and there’s a whole pile of
> gcc bugs for -Wstringop-truncation (currently 22 open bugs).  Figuring out which
> one or ones it is doesn’t seem like a good use of time.

Okay, fine.

> 
>>
>> Or, alternatively by specifying a type in the enum (we are on c++14 now...), the cast to size_t and could go away and with it the error? (If it is the cause of the gcc error).
> 
> Or don’t use an enum, but change it to an appropriately typed static constant.
> I don’t know if the cast has anything to do with -Wstringop-truncation being confused, but
> as I had the same thought, that seems worth a try.
> 

+1.

In any case using a typed static constant is better than the single enum 
value, even if it does not fix the problem.

Thanks,
   Thomas