[10] Buld warnings from GCC 7.1 on Fedora 26

[Kim Barrett]

> 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?)