RFR: 8232084: HotSpot build failed with GCC 9.2.1

[Yasumasa Suenaga]

On 2019/10/10 16:42, David Holmes wrote:
> On 10/10/2019 5:25 pm, serguei.spitsyn at oracle.com wrote:
>> On 10/9/19 23:39, Chris Plummer wrote:
>>> I think the +1 gets rid of the warning, but it also means there is no need for the following line that sets buf[len].
>>
>> Yes, it was my guess.
>> Then I agree this statement must be removed: buf[len] = '\0';
> 
> I don't think so!
> 
> Given this description:
> 
> template <> void DCmdArgument<bool>::parse_value(const char* str,
>                                                   size_t len, TRAPS) {
>    // len is the length of the current token starting at str
> 
> it is not evident to me that we are even dealing with a NUL-terminated str. Rather str is the start of the next token, with length len, after which may be another token and so forth. So we may be passing in:
> 
> str = "token1 token2"
> len = 6
> 
> so we still need the explicit buf[len]='\0'

I missed it, thanks!
I think we can fix it as below.

-------------
diff -r b4f2e13d20ea src/hotspot/share/services/diagnosticArgument.cpp
--- a/src/hotspot/share/services/diagnosticArgument.cpp Wed Oct 09 19:38:11 2019 -0700
+++ b/src/hotspot/share/services/diagnosticArgument.cpp Thu Oct 10 17:04:09 2019 +0900
@@ -151,7 +151,7 @@
ResourceMark rm;

        char* buf = NEW_RESOURCE_ARRAY(char, len + 1);
-      strncpy(buf, str, len);
memcpy(buf, str, len);
buf[len] = '\0';
        Exceptions::fthrow(THREAD_AND_LOCATION, vmSymbols::java_lang_IllegalArgumentException(),
          "Boolean parsing error in command argument '%s'. Could not parse: %s.\n", _name, buf);
-------------

I will upload webrev if it is ok.


Thanks,

Yasumasa


> David
> 
>> Thanks,
>> Serguei
>>
>>
>>>
>>> Chris
>>>
>>> On 10/9/19 11:31 PM, serguei.spitsyn at oracle.com wrote:
>>>> Hi Yasumasa,
>>>>
>>>> The fix in src/hotspot/cpu/x86/macroAssembler_x86.hpp looks Okay.
>>>>
>>>> But I don't understand why you need this fix in src/hotspot/share/services/diagnosticArgument.cpp ?:
>>>>         char* buf = NEW_RESOURCE_ARRAY(char, len + 1);
>>>> - strncpy(buf, str, len);
>>>> + strncpy(buf, str, len + 1);
>>>>         buf[len] = '\0';
>>>> The buf[len] is set to '\0' anyway.
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>>
>>>> On 10/9/19 22:55, Yasumasa Suenaga wrote:
>>>>> Hi all,
>>>>>
>>>>> Please review this change:
>>>>>
>>>>>   JBS: https://bugs.openjdk.java.net/browse/JDK-8232084
>>>>>   webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8232084/webrev.00/
>>>>>
>>>>>
>>>>> I tried to build OpenJDK on Fedora 30 x64 with GCC 9.2.1, but it was failed in
>>>>> macroAssembler_x86.hpp and diagnosticArgument.cpp .
>>>>> (Please see JBS for details)
>>>>>
>>>>> This change has passed tests on submit repo.
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa (ysuenaga)
>>>>
>>>
>>>
>>