[10] RFR: 8184309: Buld warnings from GCC 7.1 on Fedora 26

[Vladimir Ivanov]

>> http://cr.openjdk.java.net/~ysuenaga/JDK-8184309/webrev.00/
> 
> Looks good. I've submitted a JPRT build for testing.

FTR the job finished successfully. I'll wait for second review before 
the push.

Best regards,
Vladimir Ivanov

>> 2017-07-13 3:30 GMT+09:00 Kim Barrett <kim.barrett at oracle.com>:
>>>> On Jul 11, 2017, at 9:51 PM, Yasumasa Suenaga <yasuenag at gmail.com> 
>>>> wrote:
>>>>
>>>> Hi all,
>>>>
>>>> I tried to build jdk10-hs on Fedora 26 with gcc-7.1.1-3.fc26.x86_64,
>>>> then I encountered some build warnings.
>>>> I want to fix them. Can I file it to JBS and send review request for 
>>>> 10?
>>>
>>> Yes to filing issues and submitting patches or RFRs.
>>>
>>> A couple of comments on the proposed patches.
>>>
>>>> 2-warning:
>>>> /home/ysuenaga/OpenJDK/jdk10-hs/hotspot/src/share/vm/compiler/methodMatcher.cpp: 
>>>>
>>>> In static member function 'static bool
>>>> MethodMatcher::canonicalize(char*, const char*&)':
>>>> /home/ysuenaga/OpenJDK/jdk10-hs/hotspot/src/share/vm/compiler/methodMatcher.cpp:99:22: 
>>>>
>>>> warning: comparison between pointer and zero character constant
>>>> [-Wpointer-compare]
>>>>      if (colon + 2 != '\0') {
>>>>                       ^~~~
>>>> /home/ysuenaga/OpenJDK/jdk10-hs/hotspot/src/share/vm/compiler/methodMatcher.cpp:99:22: 
>>>>
>>>> note: did you mean to dereference the pointer?
>>>>
>>>> 2-patch:
>>>> diff -r 9c54cd2cdf09 src/share/vm/compiler/methodMatcher.cpp
>>>> --- a/src/share/vm/compiler/methodMatcher.cpp   Mon Jul 10 23:28:25 
>>>> 2017 +0200
>>>> +++ b/src/share/vm/compiler/methodMatcher.cpp   Wed Jul 12 10:32:20 
>>>> 2017 +0900
>>>> @@ -96,7 +96,7 @@
>>>>    bool have_colon = (colon != NULL);
>>>>    if (have_colon) {
>>>>      // Don't allow multiple '::'
>>>> -    if (colon + 2 != '\0') {
>>>> +    if (colon[2] != '\0') {
>>>>        if (strstr(colon+2, "::")) {
>>>>          error_msg = "Method pattern only allows one '::' allowed";
>>>>          return false;
>>>
>>> Already reported in JDK-8181503.  There’s an RFR out already, though 
>>> it’s been stalled
>>> by discussion of one of the other bullet items in that CR plus 
>>> vacations.
>>>
>>>> 3-warning:
>>>> /home/ysuenaga/OpenJDK/jdk10-hs/hotspot/src/share/vm/logging/logFileOutput.cpp: 
>>>>
>>>> In static member function 'static void
>>>> LogFileOutput::set_file_name_parameters(jlong)':
>>>> /home/ysuenaga/OpenJDK/jdk10-hs/hotspot/src/share/vm/logging/logFileOutput.cpp:61:99: 
>>>>
>>>> warning: format not a string literal, format string not checked
>>>> [-Wformat-nonliteral]
>>>>    res = (int)strftime(_vm_start_time_str, sizeof(_vm_start_time_str),
>>>> TimestampFormat, &local_time);
>>>>
>>>>                             ^
>>>>
>>>> 3-patch:
>>>> diff -r 9c54cd2cdf09 src/share/vm/logging/logFileOutput.cpp
>>>> --- a/src/share/vm/logging/logFileOutput.cpp    Mon Jul 10 23:28:25 
>>>> 2017 +0200
>>>> +++ b/src/share/vm/logging/logFileOutput.cpp    Wed Jul 12 10:32:20 
>>>> 2017 +0900
>>>> @@ -51,6 +51,8 @@
>>>>    _file_name = make_file_name(name + strlen(Prefix), _pid_str,
>>>> _vm_start_time_str);
>>>> }
>>>>
>>>> +PRAGMA_DIAG_PUSH
>>>> +PRAGMA_FORMAT_NONLITERAL_IGNORED
>>>> void LogFileOutput::set_file_name_parameters(jlong vm_start_time) {
>>>>    int res = jio_snprintf(_pid_str, sizeof(_pid_str), "%d",
>>>> os::current_process_id());
>>>>    assert(res > 0, "PID buffer too small");
>>>> @@ -61,6 +63,7 @@
>>>>    res = (int)strftime(_vm_start_time_str, sizeof(_vm_start_time_str),
>>>> TimestampFormat, &local_time);
>>>>    assert(res > 0, "VM start time buffer too small.");
>>>> }
>>>> +PRAGMA_DIAG_POP
>>>>
>>>> LogFileOutput::~LogFileOutput() {
>>>>    if (_stream != NULL) {
>>>
>>> Could this one instead be fixed by changing the declaration of 
>>> TimestampFormat from
>>>
>>> static const char*  TimestampFormat;
>>>
>>> to
>>>
>>> static const char*  const TimestampFormat;
>>>
>>> with a corresponding update of the definition?  Some web searching 
>>> suggests that
>>> should work, and if it does, I’d much prefer it to the warning 
>>> suppression.
>>>
>>> If that doesn’t work, then the warning suppression should be 
>>> commented to indicate
>>> exactly what the problem is, e.g. that TimestampFormat is triggering 
>>> -Wformat-nonliteral.
>>> Otherwise, it’s not immediately obvious where the problem lies, and 
>>> the warning
>>> control scope being the entire function doesn’t help.  (I don’t 
>>> remember, can the
>>> warning scope be narrowed to just the offending statement?)
>>>