Warnings Cleanup in Hotspot

[Jeremy Manson]

Okay, I turned on -Wunused-value in the gcc makefile for Linux.  I also ran
this on a more recent build of HS (I was using something very, very old),
and I got a few more warnings (including a couple that actually looked as
if they would impact functionality).  Thoughts on this?

diff --git a/make/linux/makefiles/gcc.make b/make/linux/makefiles/gcc.make
--- a/make/linux/makefiles/gcc.make
+++ b/make/linux/makefiles/gcc.make
@@ -126,7 +126,7 @@
 # Compiler warnings are treated as errors
 WARNINGS_ARE_ERRORS = -Werror

-WARNING_FLAGS = -Wpointer-arith -Wsign-compare -Wundef -Wunused-function
+WARNING_FLAGS = -Wpointer-arith -Wsign-compare -Wundef -Wunused-function
-Wunused-value

 # Since GCC 4.3, -Wconversion has changed its meanings to warn these
implicit
 # conversions which might affect the values. Only enable it in earlier
versions.
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/memory/cardTableModRefBS.cpp
b/src/share/vm/memory/cardTableModRefBS.cpp
--- a/src/share/vm/memory/cardTableModRefBS.cpp
+++ b/src/share/vm/memory/cardTableModRefBS.cpp
@@ -395,7 +395,7 @@
   }
   // Touch the last card of the covered region to show that it
   // is committed (or SEGV).
-  debug_only(*byte_for(_covered[ind].last());)
+  debug_only((void) (*byte_for(_covered[ind].last()));)
   debug_only(verify_guard();)
 }

diff --git a/src/share/vm/memory/universe.cpp
b/src/share/vm/memory/universe.cpp
--- a/src/share/vm/memory/universe.cpp
+++ b/src/share/vm/memory/universe.cpp
@@ -532,7 +532,9 @@
   if (vt) vt->initialize_vtable(false, CHECK);
   if (ko->oop_is_instance()) {
     InstanceKlass* ik = (InstanceKlass*)ko;
-    for (KlassHandle s_h(THREAD, ik->subklass()); s_h() != NULL; s_h =
(THREAD, s_h()->next_sibling())) {
+    for (KlassHandle s_h(THREAD, ik->subklass());
+         s_h() != NULL;
+         s_h = KlassHandle(THREAD, s_h()->next_sibling())) {
       reinitialize_vtable_of(s_h, CHECK);
     }
   }
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

diff --git a/src/share/vm/services/diagnosticArgument.cpp
b/src/share/vm/services/diagnosticArgument.cpp
--- a/src/share/vm/services/diagnosticArgument.cpp
+++ b/src/share/vm/services/diagnosticArgument.cpp
@@ -232,7 +232,7 @@
   } else {
     _value._time = 0;
     _value._nanotime = 0;
-    strcmp(_value._unit, "ns");
+    strcpy(_value._unit, "ns");
   }
 }

diff --git a/src/share/vm/utilities/taskqueue.hpp
b/src/share/vm/utilities/taskqueue.hpp
--- a/src/share/vm/utilities/taskqueue.hpp
+++ b/src/share/vm/utilities/taskqueue.hpp
@@ -340,8 +340,12 @@
   if (dirty_n_elems == N - 1) {
     // Actually means 0, so do the push.
     uint localBot = _bottom;
-    // g++ complains if the volatile result of the assignment is unused.
-    const_cast<E&>(_elems[localBot] = t);
+    // g++ complains if the volatile result of the assignment is
+    // unused, so we cast the volatile away.  We cannot cast directly
+    // to void, because gcc treats that as not using the result of the
+    // assignment.  However, casting to E& means that we trigger an
+    // unused-value warning.  So, we cast the E& to void.
+    (void) const_cast<E&>(_elems[localBot] = t);
     OrderAccess::release_store(&_bottom, increment_index(localBot));
     TASKQUEUE_STATS_ONLY(stats.record_push());
     return true;
@@ -397,7 +401,12 @@
     return false;
   }

-  const_cast<E&>(t = _elems[oldAge.top()]);
+  // g++ complains if the volatile result of the assignment is
+  // unused, so we cast the volatile away.  We cannot cast directly
+  // to void, because gcc treats that as not using the result of the
+  // assignment.  However, casting to E& means that we trigger an
+  // unused-value warning.  So, we cast the E& to void.
+  (void) const_cast<E&>(t = _elems[oldAge.top()]);
   Age newAge(oldAge);
   newAge.increment();
   Age resAge = _age.cmpxchg(newAge, oldAge);
@@ -640,8 +649,12 @@
   uint dirty_n_elems = dirty_size(localBot, top);
   assert(dirty_n_elems < N, "n_elems out of range.");
   if (dirty_n_elems < max_elems()) {
-    // g++ complains if the volatile result of the assignment is unused.
-    const_cast<E&>(_elems[localBot] = t);
+    // g++ complains if the volatile result of the assignment is
+    // unused, so we cast the volatile away.  We cannot cast directly
+    // to void, because gcc treats that as not using the result of the
+    // assignment.  However, casting to E& means that we trigger an
+    // unused-value warning.  So, we cast the E& to void.
+    (void) const_cast<E&>(_elems[localBot] = t);
     OrderAccess::release_store(&_bottom, increment_index(localBot));
     TASKQUEUE_STATS_ONLY(stats.record_push());
     return true;
@@ -665,7 +678,12 @@
   // This is necessary to prevent any read below from being reordered
   // before the store just above.
   OrderAccess::fence();
-  const_cast<E&>(t = _elems[localBot]);
+  // g++ complains if the volatile result of the assignment is
+  // unused, so we cast the volatile away.  We cannot cast directly
+  // to void, because gcc treats that as not using the result of the
+  // assignment.  However, casting to E& means that we trigger an
+  // unused-value warning.  So, we cast the E& to void.
+  (void) const_cast<E&>(t = _elems[localBot]);
   // This is a second read of "age"; the "size()" above is the first.
   // If there's still at least one element in the queue, based on the
   // "_bottom" and "age" we've read, then there can be no interference with
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130510/81df0e93/attachment-0001.html