RFR(M): 8073315: Enable gcc -Wtype-limits and fix upcoming issues.

[Kim Barrett]

On Mar 2, 2015, at 7:43 AM, Lindenmaier, Goetz <goetz.lindenmaier at sap.com> wrote:
> 
> I agree not to fix this. [kab: error checking in get_core_path()]
> I uploaded a new webrev that fixes the remaining issues:
> http://cr.openjdk.java.net/~goetz/webrevs/8073315-wUnsign/webrev.04/
> 
> I changed the makefile to only set the flag in gcc 4.8,
> changed get_core_path to return -1 and removed the assertions,
> and fixed the bracket in graphKit.

Mostly looks good; just a couple more things.

------------------------------------------------------------------------------
src/share/vm/gc_implementation/g1/heapRegionSet.cpp
  59   guarantee(( is_empty() && length() == 0 && total_capacity_bytes() == 0) ||
  60             (!is_empty() && length() >= 0 && total_capacity_bytes() >= 0),
  61             hrs_ext_msg(this, "invariant"));
=>
  59   guarantee((is_empty() && length() == 0 && total_capacity_bytes() == 0) || !is_empty(),
  60             hrs_ext_msg(this, "invariant"));

[Sorry I didn't notice this earlier.]

Might it be that the real defect here is that the !is_empty() bounds
should be > rather than >= ?  I haven't done the analysis to verify
that, just positing based on "obvious" reading of the code.

If not, then it would be simpler as

  guarantee(!is_empty() || (length() == 0 && total_capacity_bytes() == 0), ...);

or even better (since this is a guarantee)

  if (is_empty()) {
    guarantee(length() == 0 && total_capacity_bytes() == 0, ...);
  }

------------------------------------------------------------------------------ 
src/share/vm/runtime/dtraceJSDT.hpp

New file?

------------------------------------------------------------------------------

I'd still like to see a doc comment update for the os::get_core_path()
declaration in runtime/os.hpp that mentions the -1 return to indicate
an error.

------------------------------------------------------------------------------

I've filed the following CRs for things we agreed not to address in
this change set.

https://bugs.openjdk.java.net/browse/JDK-8074136
Many missing error checks in get_core_path variants

https://bugs.openjdk.java.net/browse/JDK-8074142
Simplify use of G1StressConcRegionFreeingDelayMillis