Warnings Cleanup in Hotspot

[Eric McCorkle]

Clang is generally stricter than GCC.  This causes some applications to
fail to compile.  FreeBSD has had a good bit of experience with this, as
it has moved the base system and has been moving ports over to clang
over the past few years.

On 05/02/13 12:55, Jeremy Manson wrote:
> These were warnings from a relatively recent build of Clang.  Not sure
> what the current status of Clang support is (I had to make some other
> Clang compatibility hacks for it to compile at all).
> 
> -Wunsequenced gave the warning in nmethod.cpp. (I believe this is
> Clang-only)
> -Wunused-value gave the warnings in c1_IR.cpp and in sharedRuntime.cpp.
> (g++ seems to have this)
> 
> I could conceivably add a patch to the makefiles to compile with
> unused-value.
> 
> Jeremy
> 
> 
> 
> On Thu, May 2, 2013 at 12:53 AM, Mikael Gerdin <mikael.gerdin at oracle.com
> <mailto:mikael.gerdin at oracle.com>> wrote:
> 
>     Jeremy,
> 
> 
>     On 2013-05-01 22:31, Jeremy Manson wrote:
> 
>         Hi folks,
> 
>         On a lark, I decided to turn a bunch of warnings on in my
>         compiler and
>         see what happens to Hotspot.  Unsurprisingly, a lot of warnings
>         came out.
> 
>         The largest class of real errors (which I chose to ignore for the
>         moment, under the assumption that it would be tricky to convince
>         people
>         to accept a patch) came from having a class with a non-virtual
>         destructor, but with virtual methods.  If people are interested
>         in the
>         results of that, I can push on it some more.
> 
>         I ended up finding a handful of "real" errors, none of which are
>         likely
>         to be real problems, but all of which are probably worth
>         checking in to
>         improve general code hygiene.
> 
> 
>     +1 for general code hygiene, but I'm afraid that if we don't enable
>     the warnings which found these problems in the build we will simply
>     re-introduce these subtle bugs.
> 
>     What warnings did you enable to find these?
> 
>     Another problem with warnings is that since we compile with -Werror
>     we should aim to have roughly the same warnings across compilers,
>     otherwise we will waste people's time when they do multi-platform
>     builds and run into warnings that didn't show up on their
>     development platform of choice.
> 
>     In general I'm all for enabling more warnings and cleaning up the code.
> 
>     /Mikael
> 
> 
> 
>         The following patch fixes the five most trivially obviously wrong
>         things.  None of them are serious in the slightest.  Three of
>         them are
>         statements that were probably supposed to be print statements;
>         one of
>         them is a bug where someone assumed that arguments to printf were
>         evaluated left-to-right (instead of in an implementation-specific
>         order); and the final one is a pointer dereference that doesn't do
>         anything.  Here it is; you can feel free to tell me that you have no
>         interest:
> 
>         diff --git a/src/share/vm/c1/c1_IR.cpp b/src/share/vm/c1/c1_IR.cpp
>         --- a/src/share/vm/c1/c1_IR.cpp
>         +++ b/src/share/vm/c1/c1_IR.cpp
>         @@ -506,7 +506,7 @@
>             _loop_map(0, 0),          // initialized later with correct size
>             _compilation(c)
>           {
>         -  TRACE_LINEAR_SCAN(2, "***** computing linear-scan block order");
>         +  TRACE_LINEAR_SCAN(2, tty->print_cr("***** computing
>         linear-scan block
>         order"));
>             init_visited();
>             count_edges(start_block, NULL);
>         @@ -683,7 +683,7 @@
>           }
>           void ComputeLinearScanOrder::__assign_loop_depth(BlockBegin*
>         start_block) {
>         -  TRACE_LINEAR_SCAN(3, "----- computing loop-depth and weight");
>         +  TRACE_LINEAR_SCAN(3, tty->print_cr("----- computing
>         loop-depth and
>         weight"));
>             init_visited();
>             assert(_work_list.is_empty(), "work list must be empty before
>         processing");
>         @@ -868,7 +868,7 @@
>           }
>           void ComputeLinearScanOrder::__compute_order(BlockBegin*
>         start_block) {
>         -  TRACE_LINEAR_SCAN(3, "----- computing final block order");
>         +  TRACE_LINEAR_SCAN(3, tty->print_cr("----- computing final
>         block order"));
>             // the start block is always the first block in the linear
>         scan order
>             _linear_scan_order = new BlockList(_num_blocks);
>         diff --git a/src/share/vm/code/nmethod.__cpp
>         b/src/share/vm/code/nmethod.__cpp
>         --- a/src/share/vm/code/nmethod.__cpp
>         +++ b/src/share/vm/code/nmethod.__cpp
>         @@ -2602,7 +2602,8 @@
>                                 relocation_begin()-1+ip[1]);
>                 for (; ip < index_end; ip++)
>                   tty->print_cr("  (%d ?)", ip[0]);
>         -      tty->print_cr("          @" INTPTR_FORMAT ":
>         index_size=%d", ip,
>         *ip++);
>         +      tty->print_cr("          @" INTPTR_FORMAT ":
>         index_size=%d", ip,
>         *ip);
>         +      ip++;
>                 tty->print_cr("reloc_end @" INTPTR_FORMAT ":", ip);
>               }
>             }
>         diff --git a/src/share/vm/runtime/__sharedRuntime.cpp
>         b/src/share/vm/runtime/__sharedRuntime.cpp
>         --- a/src/share/vm/runtime/__sharedRuntime.cpp
>         +++ b/src/share/vm/runtime/__sharedRuntime.cpp
>         @@ -2733,7 +2733,7 @@
>             // ResourceObject, so do not put any ResourceMarks in here.
>             char *s = sig->as_C_string();
>             int len = (int)strlen(s);
>         -  *s++; len--;                  // Skip opening paren
>         +  s++; len--;                   // Skip opening paren
>             char *t = s+len;
>             while( *(--t) != ')' ) ;      // Find close paren
> 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: eric_mccorkle.vcf
Type: text/x-vcard
Size: 303 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130507/d6245607/eric_mccorkle.vcf