On 01/21/2016 01:50 PM, Omair Majid wrote:
Hi,

* Phil Race <philip.r...@oracle.com> [2016-01-21 14:15]:
I don't think this is a problem in practice as we also have this test ..
if (currGlyph < 0 || currGlyph >= glyphStorage.getGlyphCount()) {

But the compiler won't optimize this one away completely, will it? Both
parts of this check can be true or false. The checks that it is warning
about will never be false (because, to the compiler, signed overflow
doesn't exist).
I think my email wasn't clear in that the 'test' above is one that
is earlier, separate and valid and is sufficient to ensure that in
practice the *other* tests aren't needed.

i.e. when I wrote below :
Arguably the test could be removed but I would prefer to leave it in
^^^^^^^^^^^^^^^^^^^^^

I meant the other tests you pointed to. Not the one I cited.

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.
-fwrapv is another option and seems to be supported by gcc and clang, at
least.

That said, do you think some sort of add-and-check-for-overflow test
would be suitable here? I can take a shot at implementing it, if you
like.

Sure ! (to the code change option).
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 ?
On my box, I see the same warnings on jdk9. Please see the attached
partial build log (from `make all LOG=trace`).

Right, I did not really doubt that the  underlying issue is there,
I just don't see a warning in our build logs and it could be due to the vagaries
of each compiler version.

-phil.


Thanks,
Omair


Reply via email to