Warnings Cleanup in Hotspot

[Jeremy Manson]

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<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/20130515/d1faf5f9/attachment.html