Warnings Cleanup in Hotspot

[Jeremy Manson]

Okay.  Is there anything you want me to do?

Jeremy


On Wed, May 15, 2013 at 2:37 PM, Calvin Cheung <calvin.cheung at oracle.com>wrote:

>  Jeremy,
>
> We'll fix it in hs25 first and then backport it to hs24 if the fix is
> applicable there.
>
> Calvin
>
>
> On 5/15/2013 2:14 PM, Jeremy Manson wrote:
>
> Thanks, Calvin.  What does followup look like?
>
>  Jeremy
>
>
> On Mon, May 13, 2013 at 9:59 AM, Calvin Cheung <calvin.cheung at oracle.com>wrote:
>
>> Hi Jeremy,
>>
>> I've created 8014431 (cleanup warnings indicated by the -Wunused-value
>> compiler option on linux) to track this. It'll take a few hours for it to
>> show up in the bugs.sun.com
>>     http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8014431
>>
>> Calvin
>>
>>
>> On 5/10/2013 11:38 AM, Jeremy Manson wrote:
>>
>>> 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/20130516/2dff47e8/attachment-0001.html