[OpenJDK 2D-Dev] New warnings in layout code

[Phil Race]

I don't think this is a problem in practice as we also have this test ..
 > if (currGlyph < 0 || currGlyph >= glyphStorage.getGlyphCount()) {

Arguably the test could be removed but I would prefer to leave it in
and change the code it so it is actually tested.

-fno-strict-overflow is just too compiler-specific and too far removed
from the code to be something I would want to rely upon long term.

Interestingly I see the warning in the 8u71 logs using gcc 4.3.0,
but don't see the warning in RE's JDK 9 build log using gcc 4.9.2
I don't see it directly suppressed either but perhaps -Wno-type-limits 
masks it ?


-phil.


On 01/21/2016 09:52 AM, Omair Majid wrote:
> Hi,
>
> With a recent change to layout code in OpenJDK [1], I have started to get new
> build warnings in layout code when building with gcc (5.3.1) on linux:
>
>> /home/omajid/devel/jdk8u-jdk8u/jdk/src/share/native/sun/font/layout/IndicRearrangementProcessor.cpp: In member function ‘void IndicRearrangementP      rocessor::doRearrangementAction(LEGlyphStorage&, IndicRearrangementVerb, LEErrorCode&) const’:
>> /home/omajid/devel/jdk8u-jdk8u/jdk/src/share/native/sun/font/layout/IndicRearrangementProcessor.cpp:119:30: warning: assuming signed overflow does not occur when assuming that (X + c) < X is always false [-Wstrict-overflow]
>>           if (firstGlyph + 1 < firstGlyph) {
>>                                ^
>> /home/omajid/devel/jdk8u-jdk8u/jdk/src/share/native/sun/font/layout/IndicRearrangementProcessor.cpp:140:29: warning: assuming signed overflow does not occur when assuming that (X - c) > X is always false [-Wstrict-overflow]
>>           if (lastGlyph - 1 > lastGlyph) {
>>                               ^
>> /home/omajid/devel/jdk8u-jdk8u/jdk/src/share/native/sun/font/layout/IndicRearrangementProcessor.cpp:172:31: warning: assuming signed overflow does not occur when assuming that (X + c) < X is always false [-Wstrict-overflow]
>>           if ((firstGlyph + 2 < firstGlyph) ||
>>                                 ^
>> /home/omajid/devel/jdk8u-jdk8u/jdk/src/share/native/sun/font/layout/IndicRearrangementProcessor.cpp:198:31: warning: assuming signed overflow does not occur when assuming that (X + c) < X is always false [-Wstrict-overflow]
>>           if ((firstGlyph + 2 < firstGlyph) ||
>>                                 ^
>> /home/omajid/devel/jdk8u-jdk8u/jdk/src/share/native/sun/font/layout/IndicRearrangementProcessor.cpp:224:30: warning: assuming signed overflow does not occur when assuming that (X - c) > X is always false [-Wstrict-overflow]
>>           if ((lastGlyph - 2 > lastGlyph) ||
>>                                ^
>> /home/omajid/devel/jdk8u-jdk8u/jdk/src/share/native/sun/font/layout/IndicRearrangementProcessor.cpp:250:30: warning: assuming signed overflow does not occur when assuming that (X - c) > X is always false [-Wstrict-overflow]
>>           if ((lastGlyph - 2 > lastGlyph) ||
>>                                ^
>> /home/omajid/devel/jdk8u-jdk8u/jdk/src/share/native/sun/font/layout/IndicRearrangementProcessor.cpp:276:30: warning: assuming signed overflow does not occur when assuming that (X - c) > X is always false [-Wstrict-overflow]
>>           if ((lastGlyph - 2 > lastGlyph) ||
> These warnings seem to be checking for overflow of signed integers, which is
> undefined. These warnings were added when the variable was unsigned [2], in
> which case the overflow as defined. But with the recent change [1] the variable
> was made signed and this operation is no longer defined.
>
> Are these warnings worth worrying about? Can the error condition that the
> checks are supposed to be preventing still happen? Should -fno-strict-overflow
> be added to the build flags of libfontmanager?
>
> Thanks,
> Omair
>
> [1] http://hg.openjdk.java.net/jdk8u/jdk8u/jdk/rev/f556d4c82ef1
> [2] http://hg.openjdk.java.net/jdk8u/jdk8u/jdk/rev/dbb4e2bdfa9e
>