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

[Lindenmaier, Goetz]

Hi Jesper, 

thanks for looking at this!

You are right, at those places I should check for underflow. Also removed the
line feed.
See new webrev:
http://cr.openjdk.java.net/~goetz/webrevs/8073315-wUnsign/webrev.02/

Best regards
  Goetz

-----Original Message-----
From: Jesper Wilhelmsson [mailto:jesper.wilhelmsson at oracle.com] 
Sent: Dienstag, 17. Februar 2015 22:57
To: Lindenmaier, Goetz; hotspot-dev Source Developers
Subject: Re: RFR(M): 8073315: Enable gcc -Wtype-limits and fix upcoming issues.

Hi Goetz,

Thanks for doing this work!

Overall I think it looks great! I only have a few minor comments.

* In a many places you have removed the >=0 check with no further action. In 
most places this is clearly the right thing to do, but there are a few places 
where I'm thinking that the check may express a concern from the author of the 
code that some expression might overflow/underflow and the check is needed. 
Since the check is pointless as such the function of the code is unchanged when 
removing this check, but the author's gut feeling that something might go wrong 
is lost. I might be paranoid but I would prefer if we add asserts in these 
places to make sure the expressions are checked properly. The places where I 
think this could be motivated are in os_linux.cpp and parCardTableModRefBS.cpp

* In heapRegionSet.cpp I would prefer if the condition was on one line now when 
the second part is so short.

Thanks,
/Jesper


Lindenmaier, Goetz skrev den 17/2/15 12:42:
> Hi,
>
> We fixed all the warnings showing with -Wtype-limits and gcc in our VM.
> I would like to contribute these small changes.
>
> This warning detects unnecessary compares for >= 0 of unsigned values.
>
> At least one warning unveils a real bug. In memory/blockOffsetTable.cpp,
> BlockOffsetArrayContigSpace::last_active_index(), the compare to >=0
> tries to prevent an underflow, but the value is unsigned.
>
> Further useless compares in real code are in os_linux.cpp,
> opto/graphKit.cpp, prims/jvmtiRedefineClasses.cpp
>
> In some places debug code with constant debug flags caused
> warnings in the opt build (g1/concurrentMark.cpp, classfile/systemDictionary.cpp)
>
> The remaining warnings occur in asserts, which is also the biggest part.
>
> The change also removes an empty #ifdef ASSERT .
>
> Please review this change.  I please need a sponsor.
> https://bugs.openjdk.java.net/browse/JDK-8073315
> http://cr.openjdk.java.net/~goetz/webrevs/8073315-wUnsign/webrev.01/
>
> I tested this on linuxx86_64 with gcc 4.1.2, 4.3 and 4.8.  Further I ran the patch
> with our nightly builds and tests of jdk9,see
> http://cr.openjdk.java.net/~simonis/ppc-aix-port/index.html
>
> Best regards,
>    Goetz.
>