RFR(M): 8073315: Enable gcc -Wtype-limits and fix upcoming issues.
goetz.lindenmaier at sap.com
Wed Feb 18 10:52:30 UTC 2015
thanks for looking at this!
You are right, at those places I should check for underflow. Also removed the
See new webrev:
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.
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.
Lindenmaier, Goetz skrev den 17/2/15 12:42:
> 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.
> 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
> Best regards,
More information about the hotspot-dev